segfault in namehint API (valgrind aplay -L prints scary warnings)

Bug #1008600 reported by Julien Pommier
32
This bug affects 6 people
Affects Status Importance Assigned to Milestone
alsa-lib (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

valgrind reports a lot of scary errors when run on aplay -L , it looks like the alsa snd_device_name_hint function is doing some dangerous stuff:

==30818== Memcheck, a memory error detector
==30818== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==30818== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==30818== Command: aplay -L
==30818==
==30818== Invalid read of size 8
==30818== at 0x50653F0: snd_config_iterator_next (conf.c:3885)
==30818== by 0x5070732: snd_device_name_hint (namehint.c:506)
==30818== by 0x403DE8: ??? (in /usr/bin/aplay)
==30818== by 0x4094A8: ??? (in /usr/bin/aplay)
==30818== by 0x556576C: (below main) (libc-start.c:226)
==30818== Address 0x5e0c8f8 is 40 bytes inside a block of size 72 free'd
==30818== at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30818== by 0x5065E94: snd_config_delete (conf.c:1850)
==30818== by 0x5066425: parse_defs (conf.c:1200)
==30818== by 0x50667E5: snd_config_load1 (conf.c:1661)
==30818== by 0x5066A0C: config_file_open (conf.c:3403)
==30818== by 0x506827D: snd_config_hook_load (conf.c:3528)
==30818== by 0x64C8ACC: ???
==30818== by 0x5068EBC: snd_config_hooks.constprop.26 (conf.c:3326)
==30818== by 0x50694C3: snd_config_searcha_hooks (conf.c:3127)
==30818== by 0x5069599: snd_config_searchva_hooks (conf.c:3164)
==30818== by 0x5069675: snd1_config_search_alias_hooks (conf.c:3194)
==30818== by 0x50687A1: snd_config_search_definition (conf.c:4782)
==30818==
==30818== Invalid read of size 8
==30818== at 0x506470E: snd_config_get_id (conf.c:1578)
==30818== by 0x50706F7: snd_device_name_hint (namehint.c:508)
==30818== by 0x403DE8: ??? (in /usr/bin/aplay)
==30818== by 0x4094A8: ??? (in /usr/bin/aplay)
==30818== by 0x556576C: (below main) (libc-start.c:226)
==30818== Address 0x5e0c8d0 is 0 bytes inside a block of size 72 free'd
==30818== at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30818== by 0x5065E94: snd_config_delete (conf.c:1850)
==30818== by 0x5066425: parse_defs (conf.c:1200)
==30818== by 0x50667E5: snd_config_load1 (conf.c:1661)
==30818== by 0x5066A0C: config_file_open (conf.c:3403)
==30818== by 0x506827D: snd_config_hook_load (conf.c:3528)
==30818== by 0x64C8ACC: ???
==30818== by 0x5068EBC: snd_config_hooks.constprop.26 (conf.c:3326)
==30818== by 0x50694C3: snd_config_searcha_hooks (conf.c:3127)
==30818== by 0x5069599: snd_config_searchva_hooks (conf.c:3164)
==30818== by 0x5069675: snd1_config_search_alias_hooks (conf.c:3194)
==30818== by 0x50687A1: snd_config_search_definition (conf.c:4782)
==30818==
==30818== Invalid read of size 1
==30818== at 0x558DDBA: vfprintf (vfprintf.c:1624)
==30818== by 0x564B403: __vsprintf_chk (vsprintf_chk.c:86)
==30818== by 0x564B34C: __sprintf_chk (sprintf_chk.c:33)
==30818== by 0x506F50F: try_config (stdio2.h:34)
==30818== by 0x5070722: snd_device_name_hint (namehint.c:512)
==30818== by 0x403DE8: ??? (in /usr/bin/aplay)
==30818== by 0x4094A8: ??? (in /usr/bin/aplay)
==30818== by 0x556576C: (below main) (libc-start.c:226)
==30818== Address 0x5e0c820 is 0 bytes inside a block of size 8 free'd
==30818== at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30818== by 0x5065E8C: snd_config_delete (conf.c:1849)
==30818== by 0x5066425: parse_defs (conf.c:1200)
==30818== by 0x50667E5: snd_config_load1 (conf.c:1661)
==30818== by 0x5066A0C: config_file_open (conf.c:3403)
==30818== by 0x506827D: snd_config_hook_load (conf.c:3528)
==30818== by 0x64C8ACC: ???
==30818== by 0x5068EBC: snd_config_hooks.constprop.26 (conf.c:3326)
==30818== by 0x50694C3: snd_config_searcha_hooks (conf.c:3127)
==30818== by 0x5069599: snd_config_searchva_hooks (conf.c:3164)
==30818== by 0x5069675: snd1_config_search_alias_hooks (conf.c:3194)
==30818== by 0x50687A1: snd_config_search_definition (conf.c:4782)
==30818==
==30818== Invalid read of size 1
==30818== at 0x55BFB98: _IO_default_xsputn (genops.c:480)
==30818== by 0x558DBED: vfprintf (vfprintf.c:1624)
==30818== by 0x564B403: __vsprintf_chk (vsprintf_chk.c:86)
==30818== by 0x564B34C: __sprintf_chk (sprintf_chk.c:33)
==30818== by 0x506F50F: try_config (stdio2.h:34)
==30818== by 0x5070722: snd_device_name_hint (namehint.c:512)
==30818== by 0x403DE8: ??? (in /usr/bin/aplay)
==30818== by 0x4094A8: ??? (in /usr/bin/aplay)
==30818== by 0x556576C: (below main) (libc-start.c:226)
==30818== Address 0x5e0c820 is 0 bytes inside a block of size 8 free'd
==30818== at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30818== by 0x5065E8C: snd_config_delete (conf.c:1849)
==30818== by 0x5066425: parse_defs (conf.c:1200)
==30818== by 0x50667E5: snd_config_load1 (conf.c:1661)
==30818== by 0x5066A0C: config_file_open (conf.c:3403)
==30818== by 0x506827D: snd_config_hook_load (conf.c:3528)
==30818== by 0x64C8ACC: ???
==30818== by 0x5068EBC: snd_config_hooks.constprop.26 (conf.c:3326)
==30818== by 0x50694C3: snd_config_searcha_hooks (conf.c:3127)
==30818== by 0x5069599: snd_config_searchva_hooks (conf.c:3164)
==30818== by 0x5069675: snd1_config_search_alias_hooks (conf.c:3194)
==30818== by 0x50687A1: snd_config_search_definition (conf.c:4782)
==30818==
==30818== Invalid read of size 1
==30818== at 0x55BFBA7: _IO_default_xsputn (genops.c:479)
==30818== by 0x558DBED: vfprintf (vfprintf.c:1624)
==30818== by 0x564B403: __vsprintf_chk (vsprintf_chk.c:86)
==30818== by 0x564B34C: __sprintf_chk (sprintf_chk.c:33)
==30818== by 0x506F50F: try_config (stdio2.h:34)
==30818== by 0x5070722: snd_device_name_hint (namehint.c:512)
==30818== by 0x403DE8: ??? (in /usr/bin/aplay)
==30818== by 0x4094A8: ??? (in /usr/bin/aplay)
==30818== by 0x556576C: (below main) (libc-start.c:226)
==30818== Address 0x5e0c822 is 2 bytes inside a block of size 8 free'd
==30818== at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30818== by 0x5065E8C: snd_config_delete (conf.c:1849)
==30818== by 0x5066425: parse_defs (conf.c:1200)
==30818== by 0x50667E5: snd_config_load1 (conf.c:1661)
==30818== by 0x5066A0C: config_file_open (conf.c:3403)
==30818== by 0x506827D: snd_config_hook_load (conf.c:3528)
==30818== by 0x64C8ACC: ???
==30818== by 0x5068EBC: snd_config_hooks.constprop.26 (conf.c:3326)
==30818== by 0x50694C3: snd_config_searcha_hooks (conf.c:3127)
==30818== by 0x5069599: snd_config_searchva_hooks (conf.c:3164)
==30818== by 0x5069675: snd1_config_search_alias_hooks (conf.c:3194)
==30818== by 0x50687A1: snd_config_search_definition (conf.c:4782)
==30818==
==30818== Invalid read of size 1
==30818== at 0x4C2E439: __strcpy_chk (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30818== by 0x506F6BF: try_config (string3.h:105)
==30818== by 0x5070722: snd_device_name_hint (namehint.c:512)
==30818== by 0x403DE8: ??? (in /usr/bin/aplay)
==30818== by 0x4094A8: ??? (in /usr/bin/aplay)
==30818== by 0x556576C: (below main) (libc-start.c:226)
==30818== Address 0x5e0c820 is 0 bytes inside a block of size 8 free'd
==30818== at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30818== by 0x5065E8C: snd_config_delete (conf.c:1849)
==30818== by 0x5066425: parse_defs (conf.c:1200)
==30818== by 0x50667E5: snd_config_load1 (conf.c:1661)
==30818== by 0x5066A0C: config_file_open (conf.c:3403)
==30818== by 0x506827D: snd_config_hook_load (conf.c:3528)
==30818== by 0x64C8ACC: ???
==30818== by 0x5068EBC: snd_config_hooks.constprop.26 (conf.c:3326)
==30818== by 0x50694C3: snd_config_searcha_hooks (conf.c:3127)
==30818== by 0x5069599: snd_config_searchva_hooks (conf.c:3164)
==30818== by 0x5069675: snd1_config_search_alias_hooks (conf.c:3194)
==30818== by 0x50687A1: snd_config_search_definition (conf.c:4782)
==30818==
default
    Playback/recording through the PulseAudio sound server
null
    Discard all samples (playback) or generate zero samples (capture)
pulse
    PulseAudio Sound Server
default
    Playback/recording through the PulseAudio sound server
sysdefault:CARD=I82801AAICH
    Intel 82801AA-ICH, Intel 82801AA-ICH
    Default Audio Device
front:CARD=I82801AAICH,DEV=0
    Intel 82801AA-ICH, Intel 82801AA-ICH
    Front speakers
surround40:CARD=I82801AAICH,DEV=0
    Intel 82801AA-ICH, Intel 82801AA-ICH
    4.0 Surround output to Front and Rear speakers
surround41:CARD=I82801AAICH,DEV=0
    Intel 82801AA-ICH, Intel 82801AA-ICH
    4.1 Surround output to Front, Rear and Subwoofer speakers
surround50:CARD=I82801AAICH,DEV=0
    Intel 82801AA-ICH, Intel 82801AA-ICH
    5.0 Surround output to Front, Center and Rear speakers
surround51:CARD=I82801AAICH,DEV=0
    Intel 82801AA-ICH, Intel 82801AA-ICH
    5.1 Surround output to Front, Center, Rear and Subwoofer speakers
iec958:CARD=I82801AAICH,DEV=0
    Intel 82801AA-ICH, Intel 82801AA-ICH
    IEC958 (S/PDIF) Digital Audio Output
dmix:CARD=I82801AAICH,DEV=0
    Intel 82801AA-ICH, Intel 82801AA-ICH
    Direct sample mixing device
dsnoop:CARD=I82801AAICH,DEV=0
    Intel 82801AA-ICH, Intel 82801AA-ICH
    Direct sample snooping device
hw:CARD=I82801AAICH,DEV=0
    Intel 82801AA-ICH, Intel 82801AA-ICH
    Direct hardware device without any conversions
plughw:CARD=I82801AAICH,DEV=0
    Intel 82801AA-ICH, Intel 82801AA-ICH
    Hardware device with all software conversions
==30818==
==30818== HEAP SUMMARY:
==30818== in use at exit: 32,284 bytes in 94 blocks
==30818== total heap usage: 16,469 allocs, 16,375 frees, 719,816 bytes allocated
==30818==
==30818== LEAK SUMMARY:
==30818== definitely lost: 0 bytes in 0 blocks
==30818== indirectly lost: 0 bytes in 0 blocks
==30818== possibly lost: 0 bytes in 0 blocks
==30818== still reachable: 32,284 bytes in 94 blocks
==30818== suppressed: 0 bytes in 0 blocks
==30818== Rerun with --leak-check=full to see details of leaked memory
==30818==
==30818== For counts of detected and suppressed errors, rerun with: -v
==30818== ERROR SUMMARY: 25 errors from 6 contexts (suppressed: 2 from 2)

Revision history for this message
David Henningsson (diwic) wrote :

I can confirm this error. It looks like there is some iterator running, and when snd_config_search_definition runs, it changes the config tree, because there is some hook that does this.
So the iterator's pointing to already freed memory.

The iterator is probably the one in the add_card function, because it repeatedly runs try_config.

Changed in alsa-lib (Ubuntu):
status: New → Triaged
Revision history for this message
David Henningsson (diwic) wrote : [PATCH] Fix access of freed memory in namehints
Download full text (3.9 KiB)

Sometimes a hook manipulates the config tree, which makes currently
running iterators point to freed memory. As a workaround, make two
copies, one for the iterators and another for the hooks.

BugLink: https://bugs.launchpad.net/bugs/1008600
Signed-off-by: David Henningsson <email address hidden>
---
 src/control/namehint.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/control/namehint.c b/src/control/namehint.c
index 8d5e925..28975a4 100644
--- a/src/control/namehint.c
+++ b/src/control/namehint.c
@@ -406,7 +406,7 @@ static const next_devices_t next_devices[] = {
 };
 #endif

-static int add_card(snd_config_t *config, struct hint_list *list, int card)
+static int add_card(snd_config_t *config, snd_config_t *rw_config, struct hint_list *list, int card)
 {
  int err, ok;
  snd_config_t *conf, *n;
@@ -449,7 +449,7 @@ static int add_card(snd_config_t *config, struct hint_list *list, int card)
    ok = 0;
    for (device = 0; err >= 0 && device <= max_device; device++) {
     list->device = device;
- err = try_config(config, list, list->siface, str);
+ err = try_config(rw_config, list, list->siface, str);
     if (err < 0)
      break;
     ok++;
@@ -464,7 +464,7 @@ static int add_card(snd_config_t *config, struct hint_list *list, int card)
   if (err < 0) {
    list->card = card;
    list->device = -1;
- err = try_config(config, list, list->siface, str);
+ err = try_config(rw_config, list, list->siface, str);
   }
   if (err == -ENOMEM)
    goto __error;
@@ -493,7 +493,8 @@ static int get_card_name(struct hint_list *list, int card)
  return 0;
 }

-static int add_software_devices(snd_config_t *config, struct hint_list *list)
+static int add_software_devices(snd_config_t *config, snd_config_t *rw_config,
+ struct hint_list *list)
 {
  int err;
  snd_config_t *conf, *n;
@@ -509,7 +510,7 @@ static int add_software_devices(snd_config_t *config, struct hint_list *list)
    continue;
   list->card = -1;
   list->device = -1;
- err = try_config(config, list, list->siface, str);
+ err = try_config(rw_config, list, list->siface, str);
   if (err == -ENOMEM)
    return -ENOMEM;
  }
@@ -547,7 +548,7 @@ int snd_device_name_hint(int card, const char *iface, void ***hints)
  struct hint_list list;
  char ehints[24];
  const char *str;
- snd_config_t *conf, *local_config = NULL;
+ snd_config_t *conf, *local_config = NULL, *local_config_rw = NULL;
  snd_config_update_t *local_config_update = NULL;
  snd_config_iterator_t i, next;
  int err;
@@ -557,6 +558,7 @@ int snd_device_name_hint(int card, const char *iface, void ***hints)
  err = snd_config_update_r(&local_config, &local_config_update, NULL);
  if (err < 0)
   return err;
+ err = snd_config_copy(&local_config_rw, local_config);
  list.list = NULL;
  list.count = list.allocated = 0;
  list.siface = iface;
@@ -586,9 +588,9 @@ int snd_device_name_hint(int card, const char *iface, void ***hints)
  if (card >= 0) {
   err = get_card_name(&list, card);
   if (err >= 0)
- err = add_card(local_config, &list, card);
+ err = add_card(local_config, local_config_rw, &list, card);
  } else {
- add_software_devices(local_config, &list);
...

Read more...

Changed in alsa-lib (Ubuntu):
status: Triaged → In Progress
Revision history for this message
Takashi Iwai (tiwai) wrote :
Download full text (4.2 KiB)

At Fri, 13 Sep 2013 13:21:44 -0400,
David Henningsson wrote:
>
> Sometimes a hook manipulates the config tree, which makes currently
> running iterators point to freed memory. As a workaround, make two
> copies, one for the iterators and another for the hooks.
>
> BugLink: https://bugs.launchpad.net/bugs/1008600
> Signed-off-by: David Henningsson <email address hidden>

Thanks, applied.

Takashi

> ---
> src/control/namehint.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/src/control/namehint.c b/src/control/namehint.c
> index 8d5e925..28975a4 100644
> --- a/src/control/namehint.c
> +++ b/src/control/namehint.c
> @@ -406,7 +406,7 @@ static const next_devices_t next_devices[] = {
> };
> #endif
>
> -static int add_card(snd_config_t *config, struct hint_list *list, int card)
> +static int add_card(snd_config_t *config, snd_config_t *rw_config, struct hint_list *list, int card)
> {
> int err, ok;
> snd_config_t *conf, *n;
> @@ -449,7 +449,7 @@ static int add_card(snd_config_t *config, struct hint_list *list, int card)
> ok = 0;
> for (device = 0; err >= 0 && device <= max_device; device++) {
> list->device = device;
> - err = try_config(config, list, list->siface, str);
> + err = try_config(rw_config, list, list->siface, str);
> if (err < 0)
> break;
> ok++;
> @@ -464,7 +464,7 @@ static int add_card(snd_config_t *config, struct hint_list *list, int card)
> if (err < 0) {
> list->card = card;
> list->device = -1;
> - err = try_config(config, list, list->siface, str);
> + err = try_config(rw_config, list, list->siface, str);
> }
> if (err == -ENOMEM)
> goto __error;
> @@ -493,7 +493,8 @@ static int get_card_name(struct hint_list *list, int card)
> return 0;
> }
>
> -static int add_software_devices(snd_config_t *config, struct hint_list *list)
> +static int add_software_devices(snd_config_t *config, snd_config_t *rw_config,
> + struct hint_list *list)
> {
> int err;
> snd_config_t *conf, *n;
> @@ -509,7 +510,7 @@ static int add_software_devices(snd_config_t *config, struct hint_list *list)
> continue;
> list->card = -1;
> list->device = -1;
> - err = try_config(config, list, list->siface, str);
> + err = try_config(rw_config, list, list->siface, str);
> if (err == -ENOMEM)
> return -ENOMEM;
> }
> @@ -547,7 +548,7 @@ int snd_device_name_hint(int card, const char *iface, void ***hints)
> struct hint_list list;
> char ehints[24];
> const char *str;
> - snd_config_t *conf, *local_config = NULL;
> + snd_config_t *conf, *local_config = NULL, *local_config_rw = NULL;
> snd_config_update_t *local_config_update = NULL;
> snd_config_iterator_t i, next;
> int err;
> @@ -557,6 +558,7 @@ int snd_device_name_hint(int card, const char *iface, void ***hints)
> err = snd_config_update_r(&local_config, &local_config_update, NULL);
> if (err < 0)
> return err;
> + err = snd_config_copy(&local_config_rw, local_config);
> list.list = NULL;
> list.count = list.allocated = 0;
> list.siface = iface;
> @@ -586,9 +588,9 @@ int snd_device_name_hint(int card, const char *iface, void **...

Read more...

Changed in alsa-lib (Ubuntu):
status: In Progress → Fix Committed
Revision history for this message
sz gy (gyszabolcs) wrote : Re: valgrind aplay -L prints scary warnings

It seems to affect firefox webrtc to: http://support.mozilla.org/hu/questions/967768

Revision history for this message
Randell Jesup (randell1) wrote :

This would also affect any other users of the webrtc.org codebase

Two questions:
1) when will this be available in Alsa in Ubuntu (and elsewhere)? And how do we know?
2) How can we workaround/avoid this error

Related: how far back does this error go?

summary: - valgrind aplay -L prints scary warnings
+ segfault in namehint API (valgrind aplay -L prints scary warnings)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package alsa-lib - 1.0.27.2-3ubuntu2

---------------
alsa-lib (1.0.27.2-3ubuntu2) trusty; urgency=low

  * Fix-access-of-freed-memory-in-namehints.patch:
    Some applications using the namehints API might
    occasionally crash (LP: #1008600)
 -- David Henningsson <email address hidden> Fri, 07 Feb 2014 08:33:55 +0100

Changed in alsa-lib (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Sven (noya) wrote :

Thank you for the patch and for applying the fix to Trusty! Would anyone consider nominating this bug also for Saucy?

Revision history for this message
Jean Deruelle (jean-deruelle) wrote :

it makes WebRTC completely unusable on Ubuntu 13.10 current stable release. Can anyone backport it to Saucy ?

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.