Re: [PATCH] dbus: Message header was not initialized

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2269 bytes --]

Hi Jukka,

On 02/19/2015 03:00 AM, Jukka Rissanen wrote:
> Saw this valgrind report about the issue
>
>   ==30891== Syscall param sendmsg(msg.msg_iov[0]) points to uninitialised byte(s)
>   ==30891==    at 0x3B8FF01850: __sendmsg_nocancel (syscall-template.S:81)
>   ==30891==    by 0x410602: classic_send_message (dbus.c:561)
>   ==30891==    by 0x40F4A1: message_write_handler (dbus.c:173)
>   ==30891==    by 0x419D56: io_callback (io.c:138)
>   ==30891==    by 0x40BBC2: l_main_run (main.c:346)
>   ==30891==    by 0x401ECF: main (main.c:160)
>   ==30891==  Address 0x4c59304 is 4 bytes inside a block of size 12 alloc'd
>   ==30891==    at 0x4A06BCF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>   ==30891==    by 0x4A08A9D: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>   ==30891==    by 0x409A7D: l_realloc (util.c:92)
>   ==30891==    by 0x413A3E: message_new_common (dbus-message.c:201)
>   ==30891==    by 0x414B73: _dbus_message_new_method_call (dbus-message.c:221)
>   ==30891==    by 0x408A8B: send_request (agent.c:102)
>   ==30891==    by 0x408D28: agent_finalize_pending (agent.c:157)
>   ==30891==    by 0x408F9D: request_timeout (agent.c:241)
>   ==30891==    by 0x40C349: timeout_callback (timeout.c:78)
>   ==30891==    by 0x40BBC2: l_main_run (main.c:346)
>   ==30891==    by 0x401ECF: main (main.c:160)
>   ==30891==
> ---
>   ell/dbus-message.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/ell/dbus-message.c b/ell/dbus-message.c
> index 159df82..8743d66 100644
> --- a/ell/dbus-message.c
> +++ b/ell/dbus-message.c
> @@ -200,6 +200,7 @@ static struct l_dbus_message *message_new_common(uint8_t type, uint8_t flags,
>   	 */
>   	message->header = l_realloc(NULL, 12);
>   	message->header_size = 12;
> +	memset(message->header, 0, 12);
>

This seems bogus.  We set all the parts of the header along the way, so 
there should never be anything uninitialized.  Given the contents of the 
header, this should quickly result in some weird behavior fast.

Valgrind doesn't complain to me on the various examples, so how exactly 
did you trigger this?

>   	hdr = message->header;
>   	hdr->endian = DBUS_NATIVE_ENDIAN;
>

Regards,
-Denis