[PATCH] lib: Fix uninitialized read in msghdr_copy - causes LOCAL-MESSAGING-FDPASS2 to hang

[Jeremy Allison]

On Tue, Jun 07, 2016 at 12:55:51PM -0700, Jeremy Allison wrote:
> On Tue, Jun 07, 2016 at 12:14:20PM -0700, Jeremy Allison wrote:
> > I think the bug here is having the msg_controllen field
> > nulled out... still investigating.
> 
> Found it. msghdr_copy() can be called with a first argument
> of msg == NULL, when you're trying to find the size.
> 
> Then we call
> 
> fd_len = msghdr_prep_fds(&msg->msg, msg->buf, bufsize, fds, num_fds);
> 
> without checking that msg == NULL. We seem to get away with this
> as inside msghdr_prep_fds() we check for msg == NULL before
> indirecting msg, although I would have thought the compiler
> would cause a crash due to the &msg->msg, msg->buf parameter,
> but apparently not :-).
> 
> When msghdr_copy() is called with non-null then msghdr_prep_fds()
> calculates the correct size and sets up msg->msg_controllen for
> the fd array - but that gets zero'ed out with the original
> 
> msg->msg = (struct msghdr) {};
> 
> change. Fixed version attached.
> 
> Please confirm and push if happy.

Here's the bug report I'll add to the commit message,
as I think we need a back-port.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11955

This one might crash on other platforms (on Linux we
get away with it as the compiler just passes NULL->buf
as an invalid pointer buf=0xc8 <error: Cannot access memory at address 0xc8>
but as we never indirect through it we get away with it.

Jeremy.

> From 9cadfb54a6899b7d704be7a022b95a3bec31c43f Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Mon, 12 Jan 2015 17:47:19 +0100
> Subject: [PATCH] lib: Fix uninitialized read in msghdr_copy
> 
> Signed-off-by: Volker Lendecke <vl at samba.org>
> Reviewed-by: Amitay Isaacs <amitay at gmail.com>
> Reviewed-by: Jeremy Allison <jra at samba.org>
> ---
>  lib/util/msghdr.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/util/msghdr.c b/lib/util/msghdr.c
> index 1aeadfc..a0bb98c 100644
> --- a/lib/util/msghdr.c
> +++ b/lib/util/msghdr.c
> @@ -204,7 +204,21 @@ ssize_t msghdr_copy(struct msghdr_buf *msg, size_t msgsize,
>  	bufsize = (msgsize > offsetof(struct msghdr_buf, buf)) ?
>  		msgsize - offsetof(struct msghdr_buf, buf) : 0;
>  
> -	fd_len = msghdr_prep_fds(&msg->msg, msg->buf, bufsize, fds, num_fds);
> +	if (msg != NULL) {
> +		msg->msg = (struct msghdr) {};
> +
> +		fd_len = msghdr_prep_fds(&msg->msg,
> +					msg->buf,
> +					bufsize,
> +					fds,
> +					num_fds);
> +	} else {
> +		fd_len = msghdr_prep_fds(NULL,
> +					NULL,
> +					bufsize,
> +					fds,
> +					num_fds);
> +	}
>  
>  	if (fd_len == -1) {
>  		return -1;
> -- 
> 2.8.0.rc3.226.g39d4020
>