dgreid pointed out that this is likely a use-after-free.

cras_iodev_list.c:close_dev() will not close an iodev with a pinned stream. This means when the card is being torn down (i.e. we unplugged USB) we'll end up leaving the iodev intact but with dangling pointers to other things that were maintained by the card. See cras_alsa_io.c:alsa_iodev_destroy() and cras_alsa_card.c:cras_alsa_card_destroy(). The mixer pointer, for example, has definitely been reused:

p *((struct alsa_io *)devs[1]->iodevs->next)->mixer                                                                                                                                                  
$257 = {
  mixer = 0x5d13defef840,
  main_volume_controls = 0x100000001, # not a valid pointer
  output_controls = 0x5d13df1ecc20,
  playback_switch = 0x5d13df1a4c00,
  main_capture_controls = 0x0,
  input_controls = 0x21,              # not a valid pointer
  capture_switch = 0x65727574706143,  # C string "Capture"
  max_volume_dB = 138896501050328,    # pointer
  min_volume_dB = 32                  # unlikely
}

Should we just close the iodev anyway? If I unplug my USB speaker/mic, it doesn't matter if the stream is pinned - the thing's got to be closed. +audio people

Hi Steve, thank you for tracking down this bug.

The function close_dev was used by two cases:

1. Close the device anyway.
2. Possibly close the device, still keep it for pinned stream.

cras_iodev_list is complicated since these functions

cras_iodev_list_disable_dev
disable_device
close_dev
possibly_close_enabled_devs

do not let user show clear intention whether it is case 1 or case 2.

The consistent way to reproduce this issue is to use

cras_test_client --playback_f /dev/zero --pin_device X:Y
where X:Y is USB device index.

Unplug USB

ctrl+c to stop playback, and see CRAS crashed.

I will check if there is any quick fix for this particular case.
But I really doubt there can be a quick fix.
The cras_iodev_list needs to be refactored somehow so pinned stream can be managed cleanly.

Hsinyu is working on hotword fallback device recently which will further complicate cras_iodev_list.

We need to sort this out.

https://chromium-review.googlesource.com/636770
maybe?

Hm, I'm having trouble with the repro in #3 with my USB speaker. I added a few extra logs and it looks like I'm never actually getting a stream pinned properly on the device, so the bad codepath is never triggered. So no luck verifying #4.

Hi Steve, you can play some tone like

cras_test_client --playback_f /usr/local/autotest/client/cros/audio/fix_440_16.raw --pin_device X:Y
and check tone is from USB speaker.

Then, select active node to internal speaker from UI, and check whether sine tone is still coming from USB speaker. That way you can make sure stream is pinned to USB device.

Anyway I can verify the fix since it seems pretty easy to trigger the crash for me.

As a side note, we did not expect a pinned stream can be used on a device that can be unplugged. As I understand the pinned stream is used in hotword only as for now. Maybe there is something else causing the crash in #0.

Maybe the crash has something to do with loopback device or casting?

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

commit 105baf9f2e68dd411f6c8b84b663f81ae45b40c7
Author: Dylan Reid <dgreid@chromium.org>
Date: Thu Aug 31 09:37:09 2017

CRAS: iodev_list - Allow device to be force closed

When a device is removed we have to close it, we can't leave it open if
there is a pinned stream.

BUG= chromium:756830 
TEST=cras_test_client --playback_f /dev/zero --pin_device X:Y, where X:Y
is USB device. Unplug USB device and see CRAS does not crash.

Change-Id: I7ac5172162b9dcc66b6318117685e144aee40473
Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/636770
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Cheng-Yi Chiang <cychiang@chromium.org>

[modify] https://crrev.com/105baf9f2e68dd411f6c8b84b663f81ae45b40c7/cras/src/server/cras_iodev_list.c
[modify] https://crrev.com/105baf9f2e68dd411f6c8b84b663f81ae45b40c7/cras/src/tests/iodev_list_unittest.cc
[modify] https://crrev.com/105baf9f2e68dd411f6c8b84b663f81ae45b40c7/cras/src/server/cras_bt_device.c
[modify] https://crrev.com/105baf9f2e68dd411f6c8b84b663f81ae45b40c7/cras/src/server/cras_iodev_list.h
[modify] https://crrev.com/105baf9f2e68dd411f6c8b84b663f81ae45b40c7/cras/src/server/cras_device_monitor.c
[modify] https://crrev.com/105baf9f2e68dd411f6c8b84b663f81ae45b40c7/cras/src/server/cras_iodev.c
[modify] https://crrev.com/105baf9f2e68dd411f6c8b84b663f81ae45b40c7/cras/src/tests/device_monitor_unittest.cc

Fix landed in master, should we cherry pick to 61 after a couple days on canary?

#9, yes, please :)

Matching priority on b/65196100 and requesting merge to 61.

This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Approving merge to M61.

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

commit b2af0b6ba525a8a76dbef69cf79a092f589bc5f6
Author: Dylan Reid <dgreid@chromium.org>
Date: Fri Sep 08 19:31:03 2017

CRAS: iodev_list - Allow device to be force closed

When a device is removed we have to close it, we can't leave it open if
there is a pinned stream.

BUG= chromium:756830 
TEST=cras_test_client --playback_f /dev/zero --pin_device X:Y, where X:Y
is USB device. Unplug USB device and see CRAS does not crash.

Change-Id: I7ac5172162b9dcc66b6318117685e144aee40473
Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/636770
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Cheng-Yi Chiang <cychiang@chromium.org>
(cherry picked from commit 105baf9f2e68dd411f6c8b84b663f81ae45b40c7)
Reviewed-on: https://chromium-review.googlesource.com/657302
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Commit-Queue: Stephen Barber <smbarber@chromium.org>
Tested-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/b2af0b6ba525a8a76dbef69cf79a092f589bc5f6/cras/src/server/cras_iodev_list.c
[modify] https://crrev.com/b2af0b6ba525a8a76dbef69cf79a092f589bc5f6/cras/src/tests/iodev_list_unittest.cc
[modify] https://crrev.com/b2af0b6ba525a8a76dbef69cf79a092f589bc5f6/cras/src/server/cras_bt_device.c
[modify] https://crrev.com/b2af0b6ba525a8a76dbef69cf79a092f589bc5f6/cras/src/server/cras_iodev_list.h
[modify] https://crrev.com/b2af0b6ba525a8a76dbef69cf79a092f589bc5f6/cras/src/server/cras_device_monitor.c
[modify] https://crrev.com/b2af0b6ba525a8a76dbef69cf79a092f589bc5f6/cras/src/server/cras_iodev.c
[modify] https://crrev.com/b2af0b6ba525a8a76dbef69cf79a092f589bc5f6/cras/src/tests/device_monitor_unittest.cc