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

From: Laurent Vivier <laurent@vivier.eu>
To: "Jonas Schievink" <jonasschievink@gmail.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>
Cc: Riku Voipio <riku.voipio@iki.fi>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2] Zero out the host's `msg_control` buffer
Date: Thu, 12 Jul 2018 18:05:50 +0200	[thread overview]
Message-ID: <5701803b-7f38-2539-fd08-6706d624271d@vivier.eu> (raw)
In-Reply-To: <20180711221244.31869-1-jonasschievink@gmail.com>

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 <jonasschievink@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