Re: [Qemu-devel] [PATCH v2] Zero out the host's `msg_control` buffer

[Jonas Schievink]

Yes, I do. See
https://gist.github.com/jonas-schievink/cb6e6584a055539d2113f22d91068e2d

The problem is that glibc's CMSG_NXTHDR macro will access the header of the
*next* message which isn't yet overwritten by QEMU, so it still contains
garbage at that point. In particular, it will access the length field of
the header to check if the message fits inside the msg_control buffer. If
the length contains garbage, it may think that it doesn't fit and returns
NULL. This is also mentioned in the glibc bug report I linked in the
previous mail (https://sourceware.org/bugzilla/show_bug.cgi?id=13500), and
the memset workaround is mentioned there as well.

Regards,
Jonas

On Thu, Jul 12, 2018 at 6:05 PM Laurent Vivier <address@hidden> wrote:

> Le 12/07/2018 à 00:12, Jonas Schievink a écrit :
> > If this is not done, qemu would drop any control message after the first
> > one.
> >
> > This is because glibc's `CMSG_NXTHDR` macro accesses the uninitialized
> > cmsghdr's length field in order to find out if the message fits into the
> > `msg_control` buffer, wrongly assuming that it doesn't because the
> > length field contains garbage. Accessing the length field is fine for
> > completed messages we receive from the kernel, but is - as far as I know
> > - not needed since the kernel won't return such an invalid cmsghdr in
> > the first place.
> >
> > This is tracked as this glibc bug:
> > https://sourceware.org/bugzilla/show_bug.cgi?id=13500
> >
> > It's probably also a good idea to bail with an error if `CMSG_NXTHDR`
> > returns NULL but `TARGET_CMSG_NXTHDR` doesn't (ie. we still expect
> > cmsgs).
> >
> > Signed-off-by: Jonas Schievink <address@hidden>
> > ---
> > Changes in v2:
> > - put the memset right after the msg_control alloca
> > - added missing Signed-off-by line
> >
> >  linux-user/syscall.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index e4b1b7d7da..3c427500ef 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -3843,6 +3843,8 @@ static abi_long do_sendrecvmsg_locked(int fd,
> struct target_msghdr *msgp,
> >      }
> >      msg.msg_controllen = 2 * tswapal(msgp->msg_controllen);
> >      msg.msg_control = alloca(msg.msg_controllen);
> > +    memset(msg.msg_control, 0, msg.msg_controllen);
> > +
>
> I'm not sure it is needed as the content of msg.control will be
> overwritten by target_to_host_cmsg() from the content of msgp->control.
>
> Do you have a test case revealing the bug?
>
> Thanks,
> Laurent
>