d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix for locking order reversal in USB audio driver, when using mmap().

Browse Source 
Locking the second lock which causes the LOR, can be skipped because
the code updating the shared variables is always executing from the
same USB thread.

lock order reversal:
  1st 0xfffff80005cc3840 pcm7:play:dsp7.p0 (pcm play channel, sleep mutex)
@ usb_transfer.c:2342
  2nd 0xfffff80005cc3860 pcm7:record:dsp7.r0 (pcm record channel, sleep mutex)
@ uaudio.c:2317

lock order pcm record channel -> pcm play channel established at:
witness_checkorder+0x461
__mtx_lock_flags+0x98
dsp_mmap_single+0x151
vm_mmap_cdev+0x65
devfs_mmap_f+0x143
kern_mmap_req+0x594
sys_mmap+0x46
amd64_syscall+0x12e
fast_syscall_common+0xf8

lock order pcm play channel -> pcm record channel attempted at:
witness_checkorder+0xd82
__mtx_lock_flags+0x98
uaudio_chan_play_callback+0xeb
usbd_callback_wrapper+0x7ec
usb_command_wrapper+0x7e
usb_callback_proc+0x8e
usb_process+0xf3
fork_exit+0x80
fork_trampoline+0xe

Found by:	Stefan Ehmann <shoesoft@gmx.net>
MFC after:	1 week
Sponsored by:	Mellanox Technologies // NVIDIA Networking
This commit is contained in:
Hans Petter Selasky 2021-02-14 20:29:16 +01:00
parent 66803b34a0
commit 12148d4300
1 changed files with 17 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
sys/dev/sound/usb/uaudio.c
View File

@ -2313,11 +2313,16 @@ uaudio_chan_play_callback(struct usb_xfer *xfer, usb_error_t error)
case USB_ST_SETUP:
tr_setup:
if (ch_rec != NULL) {
/*
* NOTE: The play and record callbacks are
* executed from the same USB thread and
* locking the record channel mutex here is
* not needed. This avoids a LOR situation.
*/
/* reset receive jitter counters */
mtx_lock(ch_rec->pcm_mtx);
ch_rec->jitter_curr = 0;
ch_rec->jitter_rem = 0;
mtx_unlock(ch_rec->pcm_mtx);
}
/* reset transmit jitter counters */
@ -2338,10 +2343,17 @@ uaudio_chan_play_callback(struct usb_xfer *xfer, usb_error_t error)
*/
if (ch_rec != NULL &&
uaudio_chan_is_async(ch, ch->cur_alt) != 0) {
mtx_lock(ch_rec->pcm_mtx);
if (ch_rec->cur_alt < ch_rec->num_alt) {
uint32_t rec_alt = ch_rec->cur_alt;
if (rec_alt < ch_rec->num_alt) {
int64_t tx_jitter;
int64_t rx_rate;
/*
* NOTE: The play and record callbacks
* are executed from the same USB
* thread and locking the record
* channel mutex here is not needed.
* This avoids a LOR situation.
*/
/* translate receive jitter into transmit jitter */
tx_jitter = ch->usb_alt[ch->cur_alt].sample_rate;
@ -2353,11 +2365,10 @@ uaudio_chan_play_callback(struct usb_xfer *xfer, usb_error_t error)
ch_rec->jitter_rem = 0;
/* compute exact number of transmit jitter samples */
rx_rate = ch_rec->usb_alt[ch_rec->cur_alt].sample_rate;
rx_rate = ch_rec->usb_alt[rec_alt].sample_rate;
ch->jitter_curr += tx_jitter / rx_rate;
ch->jitter_rem = tx_jitter % rx_rate;
}
mtx_unlock(ch_rec->pcm_mtx);
}
/* start the SYNC transfer one time per second, if any */