[PATCH] connection: Don't add uninitialized memory as 4 byte alignment padding

[Derek Foreman]

On 10/02/16 09:35 AM, Jonas Ådahl wrote:
> When we are adding padding bytes making our wl_buffer buffer content 4
> byte aligned, we are just moving the pointer. Since the buffer is
> allocated using plain malloc(), this means our padding bytes are
> effectively uninitialized data, which could be anything previously
> allocated in the server process. As we'll be sharing this buffer
> content with arbitrary clients, we are effectively sharing private
> memory with every client, and even though a well behaving client will
> discard any such memory, a malicious client may not.
> 
> Therefor, to avoid any potential missuse of the uninitialized padding
> memory shared between the server and client, initialize the buffer
> content to 0, making the padding bytes always 0.

Cool - is this the cause of the valgrind complaints I've never bothered
to track down? :)

> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
>  src/connection.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/connection.c b/src/connection.c
> index 65b64e9..c0e322f 100644
> --- a/src/connection.c
> +++ b/src/connection.c
> @@ -1137,7 +1137,7 @@ wl_closure_send(struct wl_closure *closure, struct wl_connection *connection)
>  		return -1;
>  
>  	buffer_size = buffer_size_for_closure(closure);
> -	buffer = malloc(buffer_size * sizeof buffer[0]);
> +	buffer = zalloc(buffer_size * sizeof buffer[0]);

Oh, took me a couple of reads to realize the padding is between certain
potential members in the closure. (and not trivially at the start or end
of the buffer)

I guess we could zero just the pad bytes in serialize_closure, I don't
know whether that falls under "premature optimization" or not, though.
Looks pretty easy to do...

In any case,
Reviewed-by: Derek Foreman <derekf at osg.samsung.com>

Thanks,
Derek

>  	if (buffer == NULL)
>  		return -1;
>  
>