Page MenuHomeFreeBSD

OSS: allow unplug sound cards without apps close devices
Needs ReviewPublic

Authored by rozhuk.im-gmail.com on Jun 10 2017, 8:50 PM.

Details

Reviewers
hselasky
Summary

This patch fix use case: user unplug USB sound card before all apps close /dev/mixer and /dev/dsp and then USB driver hang/wait until all apps closed, no old devices can be removed, no new devices can be added.

dmesg show for mixer:
pcm4: unregister: mixer busy
pcm4: Waiting for sound application to exit!
And almost same for dsp, but also prints app pid.

Test Plan
  • apply patch
  • rebuild kernel
  • install kernel and reboot
  • start playing sound/music then unplug USB sound card
  • connect sound card and restart player

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Hi,

Did you check that no memory is used after free?

Did you build a kernel with WITNESS and memory debugging turned on?

--HPS

No.
How I can check mem use after free and how turn on mem debug?

Hi,

I'm afraid that pending read/write/ioctls might use freed structures. I'll try to analyze your patch carefully next week. Currently at bsdcan2017 .

--HPS

Hi,

Make a test app like this:

fd = open("/dev/dsp" ...);
while (1) {
if (ioctl(fd, SND_XXXX, XXX) != 0)
break;
}
close(fd);

And run it while you yank the USB audio device. Also try reading(), writing() and mmap() in a loop when detaching.

--HPS

Thank you.

Im not sure about race condition in case: app read/write/ioctl/close and in same time unplug code started in another thread.
May be destroy_dev() handle this.
If yes then I'm sure that mixer OK, but dsp code match more complicated, so I hope that some one with expert knowledge in OSS and dev subsystem will rewiew/help.

destroy_dev() will make sure all system calls have returned. In your patch are you destroying the device first?

--HPS

I think you approach may work for the mixer case, assuming no IOCTLs are sleeping. Please make sure that destroy_dev() will flush all pending system calls.

sys/dev/sound/usb/uaudio.c
1239

Is this change needed. I think mixer_uninit() will now always succeed?

grahamperrin retitled this revision from OSS: allow unplug soundcars without apps close devices to OSS: allow unplug sound cards without apps close devices.Oct 31 2021, 9:42 AM
grahamperrin added a subscriber: grahamperrin.