[alsa-devel] snd_seq_parse_address accesses uninitialised data

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

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

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

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

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%40...
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.

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%40...
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
participants (3)
-
Clemens Ladisch
-
Henning Thielemann
-
Takashi Iwai