New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 701542 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

NULL deference in cras

Project Member Reported by abodenha@chromium.org, Mar 14 2017

Issue description

Thread 0 CRASHED [SIGSEGV @ 0x00000000 ] MAGIC SIGNATURE THREAD
Stack Quality87%Show frame trust levels
0x00007f54fb510006	(libc-2.23.so + 0x00085006 )	strlen
0x00007f54fc3b49ec	(cras -cras_alsa_io.c:1164 )	new_input
0x00007f54fc3b4cc0	(cras -cras_alsa_io.c:1391 )	jack_input_plug_event
0x00007f54fc3b77b2	(cras -cras_alsa_jack.c:335 )	hctl_jack_cb
0x00007f54fc23b6fd	(libasound.so.2.0.0 -hcontrol.c:213 )	snd_hctl_free
0x00007f54fc23b6ac	(libasound.so.2.0.0 -hcontrol.c:117 )	snd_hctl_close
0x00007f54fc3b11de	(cras -cras_alsa_card.c:597 )	cras_alsa_card_destroy
0x00007f54fc3cebc3	(cras -cras_system_state.c:361 )	cras_system_remove_alsa_card
0x00007f54fc3cfcf4	(cras -cras_udev.c:307 )	udev_sound_subsystem_callback
0x00007f54fc3cdf50	(cras -cras_server.c:551 )	cras_server_run
0x00007f54fc395186	(cras -cras.c:118 )	main
0x00007f54fb4ab795	(libc-2.23.so -libc-start.c:289 )	__libc_start_main
0x00007f54fc395208	(cras + 0x00009208 )	_start
0x00007ffed0da8617		
0x00007f54fc3951df	(cras + 0x000091df )	
 

Comment 1 by roy...@google.com, Mar 14 2017

Labels: Hotlist-Enterprise
Components: -Internals>Media OS>Kernel>Audio
Labels: Stability-Crash
Owner: dgreid@chromium.org
Status: Assigned (was: Untriaged)
https://crash.corp.google.com/browse?q=ReportID%3D%2703251d6fc0000000

Initially reported in  bug 701315 
Cc: royans@chromium.org

Comment 5 by dgreid@chromium.org, Mar 14 2017

Owner: hychao@chromium.org
Owner: cychiang@chromium.org
From the stack the scenario looks like:
1. User unplug USB card.
2. hctl is removed
3. CRAS gets USB unplugged event, and tries to create a node for it. But mixer is destroyed, so it could not get name from mixer.

I think we should check the mask value SNDRV_CTL_EVENT_MASK_REMOVE.
If we receive this mask value we should just ignore it.

Thanks!



Status: Started (was: Assigned)
I see what problem we have in using the hctl callback.
CRAS does not know the mask value so it can't use it.
I see why we did not see this issue before.
Before the iodev and mixer refactoring, hctl was maintained in jacklist, so it is closed in jack_list destroy, which is in iodev destroy.
After the refactoring, we moved hctl to be maintained in card.
In cras_alsa_card_destroy, hctl is closed after iodev is destroyed. So when jack callback is triggered by hctl close, it will face all kinds of troubles because iodev is destroyed earlier and the callback arg interpreted as the iodev becomes a dangling pointer.
I think we should close hctl first in cras_alsa_card_destroy.




Cleaned the callback on jack destroy seems cleaner. CL posted. Thanks!
https://chromium-review.googlesource.com/455618

Comment 9 by hychao@chromium.org, Mar 15 2017

Issue 699513 has been merged into this issue.
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 17 2017

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

commit 53efd2763992b84f701d771a70065a939efa3b8b
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Fri Mar 17 18:06:53 2017

CRAS: alsa_jack - Remove callback of hctl element on destroy

When a hctl jack is to be freed, its callback should be removed too.
Otherwise, callback will be called when snd_hctl_close is called, and at
that time, iodev and jack might already be destroyed.

BUG= chromium:701542 
TEST=make check

Change-Id: If08f98f5323aef75cb45dfe81bf5980da891e2f7
Reviewed-on: https://chromium-review.googlesource.com/455618
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/53efd2763992b84f701d771a70065a939efa3b8b/cras/src/server/cras_alsa_jack.c
[modify] https://crrev.com/53efd2763992b84f701d771a70065a939efa3b8b/cras/src/tests/alsa_jack_unittest.cc

Cc: bhthompson@chromium.org
Labels: Merge-Request-58 Merge-Request-57
Add merge request for fix in #10 to R57 and R58.
Thanks!
Labels: -Merge-Request-58 Merge-Approved-58
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 21 2017

Labels: merge-merged-release-R58-9334.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/495ca06cd0c2f24e21686254c34be5cdf9cf1fc4

commit 495ca06cd0c2f24e21686254c34be5cdf9cf1fc4
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Tue Mar 21 06:55:08 2017

CRAS: alsa_jack - Remove callback of hctl element on destroy

When a hctl jack is to be freed, its callback should be removed too.
Otherwise, callback will be called when snd_hctl_close is called, and at
that time, iodev and jack might already be destroyed.

BUG= chromium:701542 
TEST=make check

Change-Id: If08f98f5323aef75cb45dfe81bf5980da891e2f7
Previous-Reviewed-on: https://chromium-review.googlesource.com/455618
(cherry picked from commit 1e2521f301180f066626d7c85c5236d678ddffc2)
Reviewed-on: https://chromium-review.googlesource.com/456893
Reviewed-by: Cheng-Yi Chiang <cychiang@chromium.org>
Commit-Queue: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>

[modify] https://crrev.com/495ca06cd0c2f24e21686254c34be5cdf9cf1fc4/cras/src/server/cras_alsa_jack.c
[modify] https://crrev.com/495ca06cd0c2f24e21686254c34be5cdf9cf1fc4/cras/src/tests/alsa_jack_unittest.cc

Project Member

Comment 14 by sheriffbot@chromium.org, Mar 24 2017

Cc: bhthompson@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-58
The fix was merged to R58, but not on R57 yet.
Status: Fixed (was: Started)
Mark as fixed since we are not going to merge to R57.
Labels: -Merge-Request-57

Comment 18 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61
Status: Verified (was: Fixed)
Verified in Chrome OS 9765.32.0, 61.0.3163.53.

Sign in to add a comment