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

[Felix Fietkau]

On 2016-02-16 10:06, Eyal Birger wrote:
> Hi Felix,
> 
> Thanks again for your responses.
> 
> On Mon, Feb 15, 2016 at 2:14 PM Felix Fietkau <nbd at openwrt.org
> <mailto:nbd at openwrt.org>> wrote:
> 
>     On 2016-02-15 12:54, Eyal Birger wrote:
>     >     >     >       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?
>     Before your change, iov[0].iov_base points at ub->hdr, which is on heap
>     and is preserved across calls.
>     After your change, iov[0].iov_base points at the on-stack struct hdr,
>     which is not preserved across calls.
> 
> 
> The thing is, I don't see why the area pointed to in iov_base is
> required to be preserved between calls - the sendmsg() call never uses
> the cached values in iov. They are always re-armed with new pointers.

You're still confused, so please forget about iov for a second and only
focus on struct ubus_msghdr hdr. Think about what happens if the first
call only receives a few bytes of it, and the next call fills in the rest.

- Felix
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel