Le 12/07/2018 à 20:22, Jonas Schievink a écrit : > 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.
ok, thank you for the advanced explanation. Reviewed-by: Laurent Vivier <laur...@vivier.eu> > On Thu, Jul 12, 2018 at 6:05 PM Laurent Vivier <laur...@vivier.eu > <mailto:laur...@vivier.eu>> 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 <jonasschiev...@gmail.com > <mailto:jonasschiev...@gmail.com>> > > --- > > 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 >
