snd_seq_parse_address accesses uninitialised data

snd_seq_parse_address accesses uninitialised data

[Henning Thielemann]

I think I found a bug. The following small program simply parses an 
address:

$ cat ctest/parse-address.c
#include <alsa/asoundlib.h>

int main () {
   snd_seq_t *seq;

   snd_seq_open(&seq, "default", SND_SEQ_OPEN_DUPLEX, 0);

   snd_seq_addr_t addr = {128,0};
   const char *dest_name = "TiMidity";
   snd_seq_parse_address(seq, &addr, dest_name);
   printf("found client %s: %d:%d\n", dest_name, addr.client, addr.port);

   snd_seq_close(seq);
   return 0;
}

$ gcc -Wall -o ctest/parse-address ctest/parse-address.c `pkg-config --libs alsa`

$ valgrind ctest/parse-address
==27797== Memcheck, a memory error detector
==27797== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==27797== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==27797== Command: ctest/parse-address
==27797==
==27797== Syscall param ioctl(generic) points to uninitialised byte(s)
==27797==    at 0x5209D27: ioctl (syscall-template.S:82)
==27797==    by 0x4EC9168: snd_seq_hw_query_next_client (seq_hw.c:367)
==27797==    by 0x4ECF3E2: snd_seq_parse_address (seqmid.c:416)
==27797==    by 0x40069D: main (in /home/thielema/programming/haskell/alsa-seq/ctest/parse-address)
==27797==  Address 0x7fefffd24 is on thread 1's stack
==27797==
found client TiMidity: 129:0
==27797==
==27797== HEAP SUMMARY:
==27797==     in use at exit: 75,317 bytes in 1,409 blocks
==27797==   total heap usage: 2,054 allocs, 645 frees, 278,456 bytes allocated
==27797==
==27797== LEAK SUMMARY:
==27797==    definitely lost: 0 bytes in 0 blocks
==27797==    indirectly lost: 0 bytes in 0 blocks
==27797==      possibly lost: 42,852 bytes in 1,310 blocks
==27797==    still reachable: 32,465 bytes in 99 blocks
==27797==         suppressed: 0 bytes in 0 blocks
==27797== Rerun with --leak-check=full to see details of leaked memory
==27797==
==27797== For counts of detected and suppressed errors, rerun with: -v
==27797== Use --track-origins=yes to see where uninitialised values come from
==27797== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 2 from 2)

$ pkg-config --modversion alsa
1.0.25

Re: snd_seq_parse_address accesses uninitialised data

[Takashi Iwai]

At Sun, 23 Sep 2012 19:08:09 +0200 (CEST),
Henning Thielemann wrote:
> 
> 
> I think I found a bug. The following small program simply parses an 
> address:
> 
> $ cat ctest/parse-address.c
> #include <alsa/asoundlib.h>
> 
> int main () {
>    snd_seq_t *seq;
> 
>    snd_seq_open(&seq, "default", SND_SEQ_OPEN_DUPLEX, 0);
> 
>    snd_seq_addr_t addr = {128,0};
>    const char *dest_name = "TiMidity";
>    snd_seq_parse_address(seq, &addr, dest_name);
>    printf("found client %s: %d:%d\n", dest_name, addr.client, addr.port);
> 
>    snd_seq_close(seq);
>    return 0;
> }
> 
> $ gcc -Wall -o ctest/parse-address ctest/parse-address.c `pkg-config --libs alsa`
> 
> $ valgrind ctest/parse-address
> ==27797== Memcheck, a memory error detector
> ==27797== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
> ==27797== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
> ==27797== Command: ctest/parse-address
> ==27797==
> ==27797== Syscall param ioctl(generic) points to uninitialised byte(s)
> ==27797==    at 0x5209D27: ioctl (syscall-template.S:82)
> ==27797==    by 0x4EC9168: snd_seq_hw_query_next_client (seq_hw.c:367)
> ==27797==    by 0x4ECF3E2: snd_seq_parse_address (seqmid.c:416)
> ==27797==    by 0x40069D: main (in /home/thielema/programming/haskell/alsa-seq/ctest/parse-address)
> ==27797==  Address 0x7fefffd24 is on thread 1's stack

This is no bug.  The uninitialized fields aren't referred in the ioctl
at all.  valgrind is just too picky, so you can safely ignore it.


thanks,

Takashi

Re: snd_seq_parse_address accesses uninitialised data

[Henning Thielemann]

On Mon, 24 Sep 2012, Takashi Iwai wrote:

> At Sun, 23 Sep 2012 19:08:09 +0200 (CEST),
> Henning Thielemann wrote:
>>
>> ==27797== Syscall param ioctl(generic) points to uninitialised byte(s)
>> ==27797==    at 0x5209D27: ioctl (syscall-template.S:82)
>> ==27797==    by 0x4EC9168: snd_seq_hw_query_next_client (seq_hw.c:367)
>> ==27797==    by 0x4ECF3E2: snd_seq_parse_address (seqmid.c:416)
>> ==27797==    by 0x40069D: main (in /home/thielema/programming/haskell/alsa-seq/ctest/parse-address)
>> ==27797==  Address 0x7fefffd24 is on thread 1's stack
>
> This is no bug.  The uninitialized fields aren't referred in the ioctl
> at all.  valgrind is just too picky, so you can safely ignore it.


For me valgrind is pretty useless if I have to manually distinguish real 
problems from false alarms in a long list of valgrind messages. Are you 
sure that it is generally ok to pass pointers to uninitialized data to 
ioctl also if in this particular case the pointer is not touched by ioctl? 
If yes, then I think valgrind must be somehow teached to ignore this 
particular case.


Best,
Henning

Re: snd_seq_parse_address accesses uninitialised data

[Takashi Iwai]

At Mon, 24 Sep 2012 16:20:50 +0200 (CEST),
Henning Thielemann wrote:
> 
> 
> On Mon, 24 Sep 2012, Takashi Iwai wrote:
> 
> > At Sun, 23 Sep 2012 19:08:09 +0200 (CEST),
> > Henning Thielemann wrote:
> >>
> >> ==27797== Syscall param ioctl(generic) points to uninitialised byte(s)
> >> ==27797==    at 0x5209D27: ioctl (syscall-template.S:82)
> >> ==27797==    by 0x4EC9168: snd_seq_hw_query_next_client (seq_hw.c:367)
> >> ==27797==    by 0x4ECF3E2: snd_seq_parse_address (seqmid.c:416)
> >> ==27797==    by 0x40069D: main (in /home/thielema/programming/haskell/alsa-seq/ctest/parse-address)
> >> ==27797==  Address 0x7fefffd24 is on thread 1's stack
> >
> > This is no bug.  The uninitialized fields aren't referred in the ioctl
> > at all.  valgrind is just too picky, so you can safely ignore it.
> 
> 
> For me valgrind is pretty useless if I have to manually distinguish real 
> problems from false alarms in a long list of valgrind messages. Are you 
> sure that it is generally ok to pass pointers to uninitialized data to 
> ioctl also if in this particular case the pointer is not touched by ioctl? 

Only client field is read, and this is the field properly initialized
in alsa-lib code.  Others are fields to be filled by the kernel, so it
doesn't matter at all what's left there.

> If yes, then I think valgrind must be somehow teached to ignore this 
> particular case.

I have no idea how to teach valgrind, sorry.


Takashi

Re: snd_seq_parse_address accesses uninitialised data

[Henning Thielemann]

On Mon, 24 Sep 2012, Takashi Iwai wrote:

> At Mon, 24 Sep 2012 16:20:50 +0200 (CEST),
> Henning Thielemann wrote:
>>
>> For me valgrind is pretty useless if I have to manually distinguish real
>> problems from false alarms in a long list of valgrind messages. Are you
>> sure that it is generally ok to pass pointers to uninitialized data to
>> ioctl also if in this particular case the pointer is not touched by ioctl?
>
> Only client field is read, and this is the field properly initialized
> in alsa-lib code.  Others are fields to be filled by the kernel, so it
> doesn't matter at all what's left there.
>
>> If yes, then I think valgrind must be somehow teached to ignore this
>> particular case.
>
> I have no idea how to teach valgrind, sorry.

I also asked in the valgrind mailing list and got this answer:
    http://sourceforge.net/mailarchive/forum.php?thread_name=506089CA.5030309%40bitwagon.com&forum_name=valgrind-users

Of course, I can suppress the message for me, but I expect that more 
people will stumble on it. Thus I'd prefer a resolution of the problem in 
ALSA.

Re: snd_seq_parse_address accesses uninitialised data

[Clemens Ladisch]

Henning Thielemann wrote:
> I also asked in the valgrind mailing list and got this answer:
>    http://sourceforge.net/mailarchive/forum.php?thread_name=506089CA.5030309%40bitwagon.com&forum_name=valgrind-users
>
> Of course, I can suppress the message for me, but I expect that more
> people will stumble on it. Thus I'd prefer a resolution of the problem
> in ALSA.

If we worked around this bug in valgrind, then it would not be
possible to detect an actual uninitialized-use bug on this variable
with a (correctly-working) memory checker.


Regards,
Clemens