Message ID | 20180710233229.9311-1-jonasschievink@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Jonas, You forgot to notify the maintainers, see https://wiki.qemu.org/Contribute/SubmitAPatch#CC_the_relevant_maintainer : ./scripts/get_maintainer.pl -f linux-user/syscall.c Riku Voipio <riku.voipio@iki.fi> (maintainer:Linux user) Laurent Vivier <laurent@vivier.eu> (reviewer:Linux user) qemu-devel@nongnu.org (open list:All patches CC here) On 07/10/2018 08:32 PM, Jonas Schievink wrote: > (Apparently I messed up my git config for the last email so it didn't > send the correct name - please bear with me, this is my first time > submitting a patch to a mailing list. I've also added a link to the > upstream bug in the commit description.) > > 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). This misses your Signed-off-by: https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_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..77ce173b27 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -3845,6 +3845,8 @@ static abi_long do_sendrecvmsg_locked(int fd, struct target_msghdr *msgp, > msg.msg_control = alloca(msg.msg_controllen); > msg.msg_flags = tswap32(msgp->msg_flags); > > + memset(msg.msg_control, 0, msg.msg_controllen); > + > count = tswapal(msgp->msg_iovlen); > target_vec = tswapal(msgp->msg_iov); Do you mind swapping: > msg.msg_control = alloca(msg.msg_controllen); > + memset(msg.msg_control, 0, msg.msg_controllen); > + > msg.msg_flags = tswap32(msgp->msg_flags); With your Signed-off-by: Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Regards, Phil.
diff --git a/linux-user/syscall.c b/linux-user/syscall.c index e4b1b7d7da..77ce173b27 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -3845,6 +3845,8 @@ static abi_long do_sendrecvmsg_locked(int fd, struct target_msghdr *msgp, msg.msg_control = alloca(msg.msg_controllen); msg.msg_flags = tswap32(msgp->msg_flags); + memset(msg.msg_control, 0, msg.msg_controllen); + count = tswapal(msgp->msg_iovlen); target_vec = tswapal(msgp->msg_iov);