[OpenWrt-Devel] [PATCH v2] ubus: use network order in ubus message header fields

[Eyal Birger]

Hi Felix,

Thanks for the quick reply! (sorry for the previous top post...)

On Mon, Feb 15, 2016 at 1:24 PM Felix Fietkau <nbd at openwrt.org> wrote:

> On 2016-02-15 12:19, Eyal Birger wrote:
> > Hi Felix,
> >
> > Thanks for your review!
> >
> > If I understood the code correctly, the header fields are never accessed
> > before the header is fully received - at which point the conversion is
> done.
> > My motivation for changing the fields only as they are sent on the wire
> > is to avoid renaming the fields or changing their semantics to avoid
> > backporting messes...
> >
> > Could you elaborate on the corner cases? are there any tests I can run
> > to verify them?
> >
> > Thanks again!
> > Eyal.
> >
> >     >       if (offset < sizeof(ub->hdr)) {
> >     > -             iov[0].iov_base = ((char *) &ub->hdr) + offset;
> >     > -             iov[0].iov_len = sizeof(ub->hdr) - offset;
> >     > +             struct ubus_msghdr hdr;
> >     > +
> >     > +             hdr.version = ub->hdr.version;
> >     > +             hdr.type = ub->hdr.type;
> >     > +             hdr.seq = cpu_to_be16(ub->hdr.seq);
> >     > +             hdr.peer = cpu_to_be32(ub->hdr.peer);
> >     > +
> >     > +             iov[0].iov_base = ((char *) &hdr) + offset;
> >     > +             iov[0].iov_len = sizeof(hdr) - offset;
> The corner case is this: You changed the iov to point at stack space
> instead of ub->hdr. If the code receives a part of the header in one
> call, and another one in the next (offset > 0), the contents of hdr will
> be corrupt, as it will be a mix of uninitialized stack space + the
> received data from the last call.
>
>
Interesting... I initialize the iov_base every time to a newly created and
calculated hdr variable before the sendmsg() call, and iov is never used
otherwise - so I wonder how it could be reused in subsequent calls?



> - Felix
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20160215/7922da71/attachment.htm>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel