git: fc76e24e583d - main - sound: Fix lock order reversals in mseq_open()
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sat, 06 Jul 2024 18:24:06 UTC
The branch main has been updated by christos:
URL: https://cgit.FreeBSD.org/src/commit/?id=fc76e24e583d45a3a59fd7ad4e603c0679eaf572
commit fc76e24e583d45a3a59fd7ad4e603c0679eaf572
Author: Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2024-07-06 18:22:21 +0000
Commit: Christos Margiolis <christos@FreeBSD.org>
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);