On 29/05/20(Fri) 15:57, Visa Hankala wrote:
> On Fri, May 29, 2020 at 04:27:46PM +0200, Alexandre Ratchov wrote:
> > On Thu, May 28, 2020 at 01:41:43PM +0100, Stuart Henderson wrote:
> > > uaudio0 at uhub7 port 2 configuration 1 interface 1 "GN Netcom GN 9350"
> > > rev 2.00/1.00 addr 7
> > > uaudio0: class v1, full-speed, sync, channels: 1 play, 1 rec, 4 ctls
> > > audio1 at uaudio0
> > > uhidev0 at uhub7 port 2 configuration 1 interface 3 "GN Netcom GN 9350"
> > > rev 2.00/1.00 addr 7
> > > uhidev0: iclass 3/0
> > > uhid0 at uhidev0: input=2, output=2, feature=0
> > > uaudio0: can't reset interface
> > > uaudio0: can't reset interface
> > > audio1 detached
> > > uaudio0 detached
> > > uhid0 detached
> > > uhidev0 detached
> > > RA\xaf\xdeRA\xaf\xdeRA\xaf\xdeRA\xaf\xdeRA\xaf\xdeRA\xaf\xdeRA\xaf\xde:
> > > can't set interface
> > > kernel: protection fault trap, code=0
> > > Stopped at uaudio_stream_close+0x8a: movzbl 0x8(%r12),%esi
> > > ddb{3}> [-- sthen@localhost attached -- Thu May 28 11:58:19 2020]
> > >
> > > ddb{3}>
> > > ddb{3}> tr
> > > uaudio_stream_close(ffff800001dfb000,1) at uaudio_stream_close+0x8a
> > > uaudio_stream_open(ffff800001dfb000,1,ffff8000001e8000,ffff8000001eaa80,2a8,ffffffff816f7630)
> > > at uaudio_stream_open+0x761
> > > uaudio_trigger_output(ffff800001dfb000,ffff8000001e8000,ffff8000001eaa80,2a8,ffffffff816f7630,ffff800001e95c00)
> > > at uaudio_trigger_output+0x47
> > > audio_start_do(ffff800001e95c00) at audio_start_do+0xb5
> > > audioioctl(2a01,20004126,ffff800035a74470,7,ffff800034fe6750) at
> > > audioioctl+0x71
> > > VOP_IOCTL(fffffd867a72e9e0,20004126,ffff800035a74470,7,fffffd84fea6f9c0,ffff800034fe6750)
> > > at VOP_IOCTL+0x55
> > > vn_ioctl(fffffd867d490f10,20004126,ffff800035a74470,ffff800034fe6750) at
> > > vn_ioctl+0x75
> > > sys_ioctl(ffff800034fe6750,ffff800035a74580,ffff800035a745e0) at
> > > sys_ioctl+0x2df
> > > syscall(ffff800035a74650) at syscall+0x389
> > > Xsyscall() at Xsyscall+0x128
> > > end of kernel
> >
> > According to dmesg, audio1 was detached, so we shouldn't enter
> > audio_start_do().
> >
> > At this point the DVF_ACTIVE flag is clear; audioioctl() calls
> > device_lookup() which is supposed to return NULL in this case, so
> > ioctl() is supposed to return ENXIO, not attempt to start playback.
>
> Lets assume that audio_start_do() started when the device was still
> attached to the system. In that case device_lookup() returned a pointer
> to a good softc. This is supported by the fact that audio_start_do() did
> not crash earlier.
>
> Did usbd_set_interface() block for a moment, letting the detachment
> happen? The trace suggests that usbd_set_interface() failed, and when
> audio_start_do() resumed, sc pointed to freed memory.
The audio(4) drivers has an unaccounted reference to uaudio(4)'s softc.
So when the USB thread responsible for detaching device kicks in to
clean up the software state of an uaudio(4), it first spins on the
KERNEL_LOCK(). If any of the threads playing/recording audio sleeps
while holding an unaccounted reference to uaudio(4)'s softc, the above
issue can happen.
A way to fix this is to use usbd_ref_incr(9) and its counterpart
usbd_ref_wait(9) in uaudio_detach().
I'm not sure if it's possible for audio(4) to increment the reference
only once. Is there a place where such increment/decrement can be put?
Otherwise every operation should do the dance.
