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 <jad...@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 <der...@osg.samsung.com> Thanks, Derek > if (buffer == NULL) > return -1; > > _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel