[alsa-devel] valgrind error in snd_tlv_get_dB_range
One program (qasmixer -- alsa mixer) on my system prints lots of valgrind errors.
The first one is this:
==25716== Conditional jump or move depends on uninitialised value(s) ==25716== at 0x4E72307: snd_tlv_get_dB_range (tlv.c:170) ==25716== by 0x4E8A746: get_dB_range (simple_none.c:1162) ==25716== by 0x4E8A7E7: get_dB_range_ops (simple_none.c:1176) ==25716== by 0x4E85590: snd_mixer_selem_get_playback_dB_range (simple.c:298) ==25716== by 0x472B2B: QSnd::Mixer_Simple_Elem::set_snd_mixer_selem_id(_snd_mixer_selem_id*) (mixer_simple_elem.cpp:161) ==25716== by 0x4724B8: QSnd::Mixer_Simple_Elem::Mixer_Simple_Elem(QObject*, _snd_mixer*, _snd_mixer_selem_id*) (mixer_simple_elem.cpp:31) ==25716== by 0x479F17: QSnd::Mixer_Simple::create_mixer_elem(_snd_mixer_elem*) (mixer_simple.cpp:318) ==25716== by 0x4797B3: QSnd::Mixer_Simple::create_mixer_elems() (mixer_simple.cpp:217) ==25716== by 0x4795DF: QSnd::Mixer_Simple::open(QString const&) (mixer_simple.cpp:178) ==25716== by 0x4D4754: Tray_Mixer::load_mixer() (tray_mixer.cpp:222) ==25716== by 0x4D4505: Tray_Mixer::set_mdev_setup(Tray_Mixer_MDev_Setup*) (tray_mixer.cpp:137) ==25716== by 0x4DB863: Desktop_Items::tray_mixer_create() (desktop_items.cpp:504) ==25716== Uninitialised value was created by a heap allocation ==25716== at 0x4C2CD7B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==25716== by 0x4E8A572: init_db_range (simple_none.c:1114) ==25716== by 0x4E8A70D: get_dB_range (simple_none.c:1159) ==25716== by 0x4E8A7E7: get_dB_range_ops (simple_none.c:1176) ==25716== by 0x4E85590: snd_mixer_selem_get_playback_dB_range (simple.c:298) ==25716== by 0x472B2B: QSnd::Mixer_Simple_Elem::set_snd_mixer_selem_id(_snd_mixer_selem_id*) (mixer_simple_elem.cpp:161) ==25716== by 0x4724B8: QSnd::Mixer_Simple_Elem::Mixer_Simple_Elem(QObject*, _snd_mixer*, _snd_mixer_selem_id*) (mixer_simple_elem.cpp:31) ==25716== by 0x479F17: QSnd::Mixer_Simple::create_mixer_elem(_snd_mixer_elem*) (mixer_simple.cpp:318) ==25716== by 0x4797B3: QSnd::Mixer_Simple::create_mixer_elems() (mixer_simple.cpp:217) ==25716== by 0x4795DF: QSnd::Mixer_Simple::open(QString const&) (mixer_simple.cpp:178) ==25716== by 0x4D4754: Tray_Mixer::load_mixer() (tray_mixer.cpp:222) ==25716== by 0x4D4505: Tray_Mixer::set_mdev_setup(Tray_Mixer_MDev_Setup*) (tray_mixer.cpp:137)
I compiled libasound2 from the latest git from git://gitorious.org/alsa/alsa-lib.git with -g -O0, so you can see line numbers.
As I understand the problem is that init_db_range initialize invalid rec->db_info, because snd_hctl_elem_tlv_read failed to fully initialize tlv:
tlv = malloc(tlv_size); // line 1114 if (! tlv) return -ENOMEM; if (snd_hctl_elem_tlv_read(ctl, tlv, tlv_size) < 0) goto error; db_size = snd_tlv_parse_dB_info(tlv, tlv_size, &dbrec); if (db_size < 0) goto error; rec->db_info = malloc(db_size); if (!rec->db_info) goto error; memcpy(rec->db_info, dbrec, db_size); free(tlv);
I tried to trace problem in snd_hctl_elem_tlv_read, but I failed to do so due to lack of experience with alsa-lib.
Could someone more experienced with alsa-lib look at the problem?
Ivan Sorokin wrote:
One program (qasmixer -- alsa mixer) on my system prints lots of valgrind errors.
Yes, these are errors in valgrind.
As I understand the problem is that init_db_range initialize invalid rec->db_info, because snd_hctl_elem_tlv_read failed to fully initialize tlv:
snd_hctl_elem_tlv_read initializes as many bytes as needed.
I'd guess valgrind doesn't notice this because this is done through an ioctl, in the kernel.
Regards, Clemens
On 19.01.2014 14:16, Ivan Sorokin wrote:
I tried to trace problem in snd_hctl_elem_tlv_read, but I failed to do so due to lack of experience with alsa-lib.
Could someone more experienced with alsa-lib look at the problem?
After a bit more investigation I've found that tlv is initialized in snd_ctl_hw_elem_tlv with memcpy in line 245. This initialization looks perfectly correct. So perhaps this is a error in valgrind (unimplemented ioctl).
P.S. I believe I found a memory leak:
switch (op_flag) { case -1: inum = SNDRV_CTL_IOCTL_TLV_COMMAND; break; case 0: inum = SNDRV_CTL_IOCTL_TLV_READ; break; case 1: inum = SNDRV_CTL_IOCTL_TLV_WRITE; break; default: return -EINVAL; } xtlv = malloc(sizeof(struct snd_ctl_tlv) + tlv_size); if (xtlv == NULL) return -ENOMEM; xtlv->numid = numid; xtlv->length = tlv_size; memcpy(xtlv->tlv, tlv, tlv_size); if (ioctl(hw->fd, inum, xtlv) < 0) { free(xtlv); return -errno; } if (op_flag == 0) { if (xtlv->tlv[1] + 2 * sizeof(unsigned int) > tlv_size)
missing free(xtlv) here
return -EFAULT; memcpy(tlv, xtlv->tlv, xtlv->tlv[1] + 2 * sizeof(unsigned int));} free(xtlv); return 0;
At Sun, 19 Jan 2014 16:22:47 +0400, Ivan Sorokin wrote:
On 19.01.2014 14:16, Ivan Sorokin wrote:
I tried to trace problem in snd_hctl_elem_tlv_read, but I failed to do so due to lack of experience with alsa-lib.
Could someone more experienced with alsa-lib look at the problem?
After a bit more investigation I've found that tlv is initialized in snd_ctl_hw_elem_tlv with memcpy in line 245. This initialization looks perfectly correct. So perhaps this is a error in valgrind (unimplemented ioctl).
P.S. I believe I found a memory leak:
switch (op_flag) { case -1: inum = SNDRV_CTL_IOCTL_TLV_COMMAND; break; case 0: inum = SNDRV_CTL_IOCTL_TLV_READ; break; case 1: inum = SNDRV_CTL_IOCTL_TLV_WRITE; break; default: return -EINVAL; } xtlv = malloc(sizeof(struct snd_ctl_tlv) + tlv_size); if (xtlv == NULL) return -ENOMEM; xtlv->numid = numid; xtlv->length = tlv_size; memcpy(xtlv->tlv, tlv, tlv_size); if (ioctl(hw->fd, inum, xtlv) < 0) { free(xtlv); return -errno; } if (op_flag == 0) { if (xtlv->tlv[1] + 2 * sizeof(unsigned int) > tlv_size)
missing free(xtlv) here
Good catch. Care to send a proper fix patch?
thanks,
Takashi
return -EFAULT; memcpy(tlv, xtlv->tlv, xtlv->tlv[1] + 2 * sizeof(unsigned int));} free(xtlv); return 0;
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (3)
-
Clemens Ladisch -
Ivan Sorokin -
Takashi Iwai