The branch main has been updated by christos: URL: https://cgit.FreeBSD.org/src/commit/?id=fc76e24e583d45a3a59fd7ad4e603c0679eaf572
commit fc76e24e583d45a3a59fd7ad4e603c0679eaf572 Author: Christos Margiolis <[email protected]> AuthorDate: 2024-07-06 18:22:21 +0000 Commit: Christos Margiolis <[email protected]> CommitDate: 2024-07-06 18:22:21 +0000 sound: Fix lock order reversals in mseq_open() Opening /dev/sequencer after a clean reboot yields: lock order reversal: (sleepable after non-sleepable) 1st 0xfffffe004a2c2c08 seqflq (seqflq, sleep mutex) @ /mnt/src/sys/dev/sound/midi/sequencer.c:754 2nd 0xffffffff84197ed8 midistat lock (midistat lock, sx) @ /mnt/src/sys/dev/sound/midi/midi.c:1478 lock order seqflq -> midistat lock attempted at: 0xffffffff811c9029 at witness_checkorder+0x12b9 0xffffffff810f18a7 at _sx_xlock+0xf7 0xffffffff8417f992 at midimapper_open+0x22 0xffffffff84182770 at mseq_open+0xf0 0xffffffff80e3380f at devfs_open+0x30f 0xffffffff81b8b4b7 at VOP_OPEN_APV+0x57 0xffffffff812da1e7 at vn_open_vnode+0x397 0xffffffff812d96b3 at vn_open_cred+0xb23 0xffffffff812c2c6b at openatfp+0x52b 0xffffffff812c2711 at sys_openat+0x81 0xffffffff84110579 at filemon_wrapper_openat+0x19 0xffffffff81a223ae at amd64_syscall+0x39e 0xffffffff819dd0eb at fast_syscall_common+0xf8 Expose midistat_lock to midi/midi.c so that we can acquire the lock from mseq_open() before we lock seq_lock, and introduce _locked variants of midimapper_open() and midimapper_fetch_synth(). Sponsored by: The FreeBSD Foundation MFC after: 2 days Reviewed by: dev_submerge.ch, emaste Differential Revision: https://reviews.freebsd.org/D45770 --- sys/dev/sound/midi/midi.c | 41 ++++++++++++++++++++++++++++++++--------- sys/dev/sound/midi/midi.h | 4 ++++ sys/dev/sound/midi/sequencer.c | 8 ++++++-- 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/sys/dev/sound/midi/midi.c b/sys/dev/sound/midi/midi.c index 81c20580f7b8..d31d6ce0fa8e 100644 --- a/sys/dev/sound/midi/midi.c +++ b/sys/dev/sound/midi/midi.c @@ -181,7 +181,8 @@ TAILQ_HEAD(, snd_midi) midi_devs; * /dev/midistat variables and declarations, protected by midistat_lock */ -static struct sx midistat_lock; +struct sx midistat_lock; + static int midistat_isopen = 0; static struct sbuf midistat_sbuf; static struct cdev *midistat_dev; @@ -1470,16 +1471,28 @@ midimapper_addseq(void *arg1, int *unit, void **cookie) } int -midimapper_open(void *arg1, void **cookie) +midimapper_open_locked(void *arg1, void **cookie) { int retval = 0; struct snd_midi *m; - sx_xlock(&midistat_lock); + sx_assert(&midistat_lock, SX_XLOCKED); TAILQ_FOREACH(m, &midi_devs, link) { retval++; } + + return retval; +} + +int +midimapper_open(void *arg1, void **cookie) +{ + int retval; + + sx_xlock(&midistat_lock); + retval = midimapper_open_locked(arg1, cookie); sx_xunlock(&midistat_lock); + return retval; } @@ -1490,22 +1503,32 @@ midimapper_close(void *arg1, void *cookie) } kobj_t -midimapper_fetch_synth(void *arg, void *cookie, int unit) +midimapper_fetch_synth_locked(void *arg, void *cookie, int unit) { struct snd_midi *m; int retval = 0; - sx_xlock(&midistat_lock); + sx_assert(&midistat_lock, SX_XLOCKED); TAILQ_FOREACH(m, &midi_devs, link) { - if (unit == retval) { - sx_xunlock(&midistat_lock); + if (unit == retval) return (kobj_t)m->synth; - } retval++; } - sx_xunlock(&midistat_lock); + return NULL; } +kobj_t +midimapper_fetch_synth(void *arg, void *cookie, int unit) +{ + kobj_t synth; + + sx_xlock(&midistat_lock); + synth = midimapper_fetch_synth_locked(arg, cookie, unit); + sx_xunlock(&midistat_lock); + + return synth; +} + DEV_MODULE(midi, midi_modevent, NULL); MODULE_VERSION(midi, 1); diff --git a/sys/dev/sound/midi/midi.h b/sys/dev/sound/midi/midi.h index 567279d1e654..b200eed9bc74 100644 --- a/sys/dev/sound/midi/midi.h +++ b/sys/dev/sound/midi/midi.h @@ -41,6 +41,8 @@ MALLOC_DECLARE(M_MIDI); #define MIDI_TYPE unsigned char +extern struct sx midistat_lock; + struct snd_midi; struct snd_midi * @@ -50,8 +52,10 @@ int midi_out(struct snd_midi *_m, MIDI_TYPE *_buf, int _size); int midi_in(struct snd_midi *_m, MIDI_TYPE *_buf, int _size); kobj_t midimapper_addseq(void *arg1, int *unit, void **cookie); +int midimapper_open_locked(void *arg1, void **cookie); int midimapper_open(void *arg1, void **cookie); int midimapper_close(void *arg1, void *cookie); +kobj_t midimapper_fetch_synth_locked(void *arg, void *cookie, int unit); kobj_t midimapper_fetch_synth(void *arg, void *cookie, int unit); #endif diff --git a/sys/dev/sound/midi/sequencer.c b/sys/dev/sound/midi/sequencer.c index 817540f1545a..68b06a4f4ca4 100644 --- a/sys/dev/sound/midi/sequencer.c +++ b/sys/dev/sound/midi/sequencer.c @@ -64,6 +64,7 @@ #include <sys/kthread.h> #include <sys/unistd.h> #include <sys/selinfo.h> +#include <sys/sx.h> #ifdef HAVE_KERNEL_OPTION_HEADERS #include "opt_snd.h" @@ -751,6 +752,7 @@ mseq_open(struct cdev *i_dev, int flags, int mode, struct thread *td) * Mark this device busy. */ + sx_xlock(&midistat_lock); mtx_lock(&scp->seq_lock); if (scp->busy) { mtx_unlock(&scp->seq_lock); @@ -768,14 +770,15 @@ mseq_open(struct cdev *i_dev, int flags, int mode, struct thread *td) * Enumerate the available midi devices */ scp->midi_number = 0; - scp->maxunits = midimapper_open(scp->mapper, &scp->mapper_cookie); + scp->maxunits = midimapper_open_locked(scp->mapper, &scp->mapper_cookie); if (scp->maxunits == 0) SEQ_DEBUG(2, printf("seq_open: no midi devices\n")); for (i = 0; i < scp->maxunits; i++) { scp->midis[scp->midi_number] = - midimapper_fetch_synth(scp->mapper, scp->mapper_cookie, i); + midimapper_fetch_synth_locked(scp->mapper, + scp->mapper_cookie, i); if (scp->midis[scp->midi_number]) { if (SYNTH_OPEN(scp->midis[scp->midi_number], scp, scp->fflags) != 0) @@ -787,6 +790,7 @@ mseq_open(struct cdev *i_dev, int flags, int mode, struct thread *td) } } } + sx_xunlock(&midistat_lock); timer_setvals(scp, 60, 100);
