New issue
Advanced search Search tips

Issue 658162 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

CRAS: fix valgrind err and warnings

Project Member Reported by hychao@chromium.org, Oct 21 2016

Issue description

When running cras with valgrind, it reports several error and warnings about memory access.

Steps:
1. emerge valgrind and deploy to DUT.
2. 'stop cras'
3. 'valgrind /usr/bin/cras' 

Example of valgrind output:

==30218== Syscall param write(buf) points to uninitialised byte(s)
==30218==    at 0x4A3321D: ??? (syscall-template.S:81)
==30218==    by 0x12AC85: audio_thread_post_message (audio_thread.c:1569)
==30218==    by 0x12AC85: audio_thread_rm_open_dev (audio_thread.c:1764)
==30218==    by 0x142DF6: close_dev (cras_iodev_list.c:319)
==30218==    by 0x142F31: disable_device (cras_iodev_list.c:661)
==30218==    by 0x143EBC: possibly_disable_fallback (cras_iodev_list.c:598)
==30218==    by 0x143EBC: cras_iodev_list_enable_dev (cras_iodev_list.c:740)
==30218==    by 0x112AE0: bt_device_switch_profile (cras_bt_device.c:1067)
==30218==    by 0x1452E9: handle_main_messages (cras_main_message.c:95)
==30218==    by 0x1482F0: cras_server_run (cras_server.c:544)
==30218==    by 0x110DA7: main (cras.c:90)
==30218==  Address 0xffefffefc is on thread 1's stack
==30218==  in frame #1, created by audio_thread_rm_open_dev (audio_thread.c:1754)
==30218==
==30218== Conditional jump or move depends on uninitialised value(s)
==30218==    at 0x1140BE: cras_bt_device_sco_mtu (cras_bt_device.c:881)
==30218==    by 0x11AA29: open_dev (cras_hfp_iodev.c:95)
==30218==    by 0x116921: open_dev (cras_bt_io.c:185)
==30218==    by 0x14153F: cras_iodev_open (cras_iodev.c:791)
==30218==    by 0x142692: init_device (cras_iodev_list.c:389)
==30218==    by 0x14282B: enable_device (cras_iodev_list.c:631)
==30218==    by 0x143E9D: cras_iodev_list_enable_dev (cras_iodev_list.c:741)
==30218==    by 0x112AE0: bt_device_switch_profile (cras_bt_device.c:1067)
==30218==    by 0x1452E9: handle_main_messages (cras_main_message.c:95)
==30218==    by 0x1482F0: cras_server_run (cras_server.c:544)
==30218==    by 0x110DA7: main (cras.c:90)


==30218== Invalid read of size 1
==30218==    at 0x121C90: SuperFastHash (sfh.c:54)
==30218==    by 0x12FE60: new_input (cras_alsa_io.c:1043)
==30218==    by 0x1301BD: new_input_by_mixer_control (cras_alsa_io.c:1079)
==30218==    by 0x135691: list_controls (cras_alsa_mixer.c:588)
==30218==    by 0x135691: cras_alsa_mixer_list_inputs (cras_alsa_mixer.c:1133)
==30218==    by 0x130D90: alsa_iodev_legacy_complete_init (cras_alsa_io.c:1734)
==30218==    by 0x12C6DF: add_controls_and_iodevs_by_matching (cras_alsa_card.c:302)
==30218==    by 0x12CB17: cras_alsa_card_create (cras_alsa_card.c:507)
==30218==    by 0x1491A5: cras_system_add_alsa_card (cras_system_state.c:416)
==30218==    by 0x14A191: device_add_alsa (cras_udev.c:291)
==30218==    by 0x14A191: change_udev_device_if_alsa_device (cras_udev.c:329)
==30218==    by 0x14A521: enumerate_devices (cras_udev.c:358)
==30218==    by 0x14A521: cras_udev_start_sound_subsystem_monitor (cras_udev.c:411)
==30218==    by 0x147E46: cras_server_run (cras_server.c:419)
==30218==    by 0x110DA7: main (cras.c:90)
==30218==  Address 0x63152a5 is 1 bytes after a block of size 4 alloc'd
==30218==    at 0x4029BE8: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==30218==    by 0x133AB4: mixer_control_create (cras_alsa_mixer.c:284)
==30218==    by 0x133C23: add_control_with_name (cras_alsa_mixer.c:554)
==30218==    by 0x134C4B: add_control (cras_alsa_mixer.c:577)
==30218==    by 0x134C4B: cras_alsa_mixer_add_controls_by_name_matching (cras_alsa_mixer.c:836)
==30218==    by 0x12C50C: add_controls_and_iodevs_by_matching (cras_alsa_card.c:244)
==30218==    by 0x12CB17: cras_alsa_card_create (cras_alsa_card.c:507)
==30218==    by 0x1491A5: cras_system_add_alsa_card (cras_system_state.c:416)
==30218==    by 0x14A191: device_add_alsa (cras_udev.c:291)
==30218==    by 0x14A191: change_udev_device_if_alsa_device (cras_udev.c:329)
==30218==    by 0x14A521: enumerate_devices (cras_udev.c:358)
==30218==    by 0x14A521: cras_udev_start_sound_subsystem_monitor (cras_udev.c:411)
==30218==    by 0x147E46: cras_server_run (cras_server.c:419)
==30218==    by 0x110DA7: main (cras.c:90)


    ==24232== Invalid read of size 8
    ==24232==    at 0x139617: insert_swap_lr_plugin (cras_dsp_ini.c:280)
    ==24232==    by 0x139617: cras_dsp_ini_create (cras_dsp_ini.c:351)
    ==24232==    by 0x1385D3: cmd_reload_ini (cras_dsp.c:114)
    ==24232==    by 0x110D99: main (cras.c:86)
    ==24232==  Address 0x5711688 is 104 bytes inside a block of size 224
    free'd
    ==24232==    at 0x402C396: realloc (in
    /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==24232==    by 0x1385D3: cmd_reload_ini (cras_dsp.c:114)
    ==24232==    by 0x110D99: main (cras.c:86)
    ==24232==


 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/8c4b4743acad43edafbe12e6126447f40a1d96b9

commit 8c4b4743acad43edafbe12e6126447f40a1d96b9
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Thu Oct 20 11:02:11 2016

CRAS: dsp_ini - Fix invalid memory access

Using ARRAY_APPEND_ZERO could realloc the ini->plugins array inside
insert_swap_lr_plugin() causing invalid access to old position of
first sink. Example of valgrind output:

==24232== Invalid read of size 8
==24232==    at 0x139617: insert_swap_lr_plugin (cras_dsp_ini.c:280)
==24232==    by 0x139617: cras_dsp_ini_create (cras_dsp_ini.c:351)
==24232==    by 0x1385D3: cmd_reload_ini (cras_dsp.c:114)
==24232==    by 0x110D99: main (cras.c:86)
==24232==  Address 0x5711688 is 104 bytes inside a block of size 224
free'd
==24232==    at 0x402C396: realloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==24232==    by 0x1385D3: cmd_reload_ini (cras_dsp.c:114)
==24232==    by 0x110D99: main (cras.c:86)
==24232==

BUG= chromium:658162 
TEST=Run cras with valgrind to verify no invalid memory access.

Change-Id: Ib1eed807d84958aeec0a0fdb42eb5d855b9b272d
Reviewed-on: https://chromium-review.googlesource.com/401145
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/8c4b4743acad43edafbe12e6126447f40a1d96b9/cras/src/server/cras_dsp_ini.c

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/81e7418b513f391d60019dd3a21ceebe9ca1a81c

commit 81e7418b513f391d60019dd3a21ceebe9ca1a81c
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Wed Aug 19 10:25:18 2015

CRAS: fix various warnings reported by valgrind

BUG= chromium:658162 
TEST=emerge valgrind and deploy to DUT. On DUT do
'stop cras' and then 'valgrind /usr/bin/cras' and
watch the output. Use cras_test_client to select
input and output node, playback and record. Connect
BT headset and test A2DP and HFP. Verify everthing
above produces no valgrind warning.

Change-Id: I8c1bd63f9b29427387dcac5388d5be129472e80c
Reviewed-on: https://chromium-review.googlesource.com/401146
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/81e7418b513f391d60019dd3a21ceebe9ca1a81c/cras/src/server/cras_rstream.c
[modify] https://crrev.com/81e7418b513f391d60019dd3a21ceebe9ca1a81c/cras/src/server/cras_server_metrics.c
[modify] https://crrev.com/81e7418b513f391d60019dd3a21ceebe9ca1a81c/cras/src/server/cras_bt_device.c
[modify] https://crrev.com/81e7418b513f391d60019dd3a21ceebe9ca1a81c/cras/src/server/cras_alsa_helpers.c
[modify] https://crrev.com/81e7418b513f391d60019dd3a21ceebe9ca1a81c/cras/src/server/audio_thread.c
[modify] https://crrev.com/81e7418b513f391d60019dd3a21ceebe9ca1a81c/cras/src/server/cras_bt_adapter.c
[modify] https://crrev.com/81e7418b513f391d60019dd3a21ceebe9ca1a81c/cras/src/server/cras_alsa_io.c

Comment 3 by hychao@chromium.org, Oct 26 2016

Labels: Merge-Request-55
Status: Started (was: Assigned)

Comment 4 by dimu@chromium.org, Oct 26 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-release-R55-8872.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/5307a7758c418390c621b6281ea5cccfbda1337d

commit 5307a7758c418390c621b6281ea5cccfbda1337d
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Thu Oct 20 11:02:11 2016

CRAS: dsp_ini - Fix invalid memory access

Using ARRAY_APPEND_ZERO could realloc the ini->plugins array inside
insert_swap_lr_plugin() causing invalid access to old position of
first sink. Example of valgrind output:

==24232== Invalid read of size 8
==24232==    at 0x139617: insert_swap_lr_plugin (cras_dsp_ini.c:280)
==24232==    by 0x139617: cras_dsp_ini_create (cras_dsp_ini.c:351)
==24232==    by 0x1385D3: cmd_reload_ini (cras_dsp.c:114)
==24232==    by 0x110D99: main (cras.c:86)
==24232==  Address 0x5711688 is 104 bytes inside a block of size 224
free'd
==24232==    at 0x402C396: realloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==24232==    by 0x1385D3: cmd_reload_ini (cras_dsp.c:114)
==24232==    by 0x110D99: main (cras.c:86)
==24232==

BUG= chromium:658162 
TEST=Run cras with valgrind to verify no invalid memory access.

Change-Id: Ib1eed807d84958aeec0a0fdb42eb5d855b9b272d
Reviewed-on: https://chromium-review.googlesource.com/401145
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
(cherry picked from commit 8c4b4743acad43edafbe12e6126447f40a1d96b9)
Reviewed-on: https://chromium-review.googlesource.com/403575
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/5307a7758c418390c621b6281ea5cccfbda1337d/cras/src/server/cras_dsp_ini.c

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/23f28abe07df532cecdded031544e8fd1a9a8b72

commit 23f28abe07df532cecdded031544e8fd1a9a8b72
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Wed Aug 19 10:25:18 2015

CRAS: fix various warnings reported by valgrind

BUG= chromium:658162 
TEST=emerge valgrind and deploy to DUT. On DUT do
'stop cras' and then 'valgrind /usr/bin/cras' and
watch the output. Use cras_test_client to select
input and output node, playback and record. Connect
BT headset and test A2DP and HFP. Verify everthing
above produces no valgrind warning.

Change-Id: I8c1bd63f9b29427387dcac5388d5be129472e80c
Reviewed-on: https://chromium-review.googlesource.com/401146
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
(cherry picked from commit 81e7418b513f391d60019dd3a21ceebe9ca1a81c)
Reviewed-on: https://chromium-review.googlesource.com/403576
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/23f28abe07df532cecdded031544e8fd1a9a8b72/cras/src/server/cras_rstream.c
[modify] https://crrev.com/23f28abe07df532cecdded031544e8fd1a9a8b72/cras/src/server/cras_server_metrics.c
[modify] https://crrev.com/23f28abe07df532cecdded031544e8fd1a9a8b72/cras/src/server/cras_bt_device.c
[modify] https://crrev.com/23f28abe07df532cecdded031544e8fd1a9a8b72/cras/src/server/cras_alsa_helpers.c
[modify] https://crrev.com/23f28abe07df532cecdded031544e8fd1a9a8b72/cras/src/server/audio_thread.c
[modify] https://crrev.com/23f28abe07df532cecdded031544e8fd1a9a8b72/cras/src/server/cras_bt_adapter.c
[modify] https://crrev.com/23f28abe07df532cecdded031544e8fd1a9a8b72/cras/src/server/cras_alsa_io.c

Comment 7 by hychao@chromium.org, Oct 27 2016

Labels: -Hotlist-Merge-Approved -Merge-Approved-55
Status: Fixed (was: Started)

Comment 8 by ka...@chromium.org, Dec 1 2016

Status: Verified (was: Fixed)
Bulk-Verify

Sign in to add a comment