From nobody Sat Jul 06 18:24:06 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4WGf0k5dzsz5PMTp; Sat, 06 Jul 2024 18:24:06 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4WGf0k52Mtz53qM; Sat, 6 Jul 2024 18:24:06 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1720290246; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=bE6bobQyOTZ4VkqfaovvcYgC1iEKou8QmVUbIcBGC6Y=; b=A7GZRepFbXYCPPaeZlzXPdzbMhOCv3m0971H/MYv8CPcDqrWNK0U7A35GS0HpGgCgKtUu4 5xxptHj8XLRxTdTIVyE0vDmyZJTGp764K5jy9C9RfpuTTal6RLaTSOjHYQQkfaNUPYzCJc wD8bVWXolatIM6KEn54aWjP9mAD5B8byHeG4ofa+j6hecngFqwDSrcTIVWxtuRRdFP3QMK RCJHcyc3L9yO6YvQDZ/y7NNTnTckIHy15KYX2+Qh4HeUnIvvBgAFlRw2DM8TaDiLfbbQ5w PQ+bvz685con8iGgNDaHq2gcXmMsgF44YAv4kA1OQ7/qLJKXQ45vG8VAwEm41Q== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1720290246; a=rsa-sha256; cv=none; b=qtujmc9nm/jHNd2ntGV7GjqA8KpJfulw390niL6dCjrtZxzRWuL2xx3LG9frRGNa++Ot97 25iSCjUGlcMBEXz49z7SPjKhOP7ISDCmsCrUOjtumWD8GI6LbtOS8c2t1Llkgtc7rfE9yV r+R6A7oqFBR2eeiceuzQb0X9okiyxCY6Q1WsU7lMHCRRVg0ofgQwxE3TIrjEXymrkpHirc c1m/7fIQ7SHYsFDQGxoRERdE6XN9zKowTqwkJCyb7fglZLGvHvVwJp+YyGSIeqmOU2vYL3 Etzly6hZsTKY4erv3PtLACZnv+8zd2vEdqQHIH+xeOP53H6lwI+cnm8vw+WCag== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1720290246; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=bE6bobQyOTZ4VkqfaovvcYgC1iEKou8QmVUbIcBGC6Y=; b=L7DgYlDw2ODgCE9V0vrBgmT9xKPZVTERmm/KI291A9Hvqr2RsGIg93+gqkoZWHwt7pl1bD 10ec6NJUcKeIP8k3uHhyS7mE1VjfYfCd55jg1TnIvBcqfxNcttCyXPgSEHbQ8tePPi04Gc mynUXRKpm4cZHyKjxZeA3B/32t0GKgjGqGhwhdkata43A7FXYoLCOqq3K1DAoLw96Q140Y awztgR5eayuQm5RrBME4PQ2AByZ/nAVbZ/dMPIiS7oTvQ2dJBeV8fCfmykcpl9rBJXdWKL 9BpWnfWVdFB5uREjEsfp0ktyNNC2ynfw3nWDBsaqh5WqKirprZcdVTPsYMf2hg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4WGf0k48pczrVt; Sat, 6 Jul 2024 18:24:06 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 466IO6gS090588; Sat, 6 Jul 2024 18:24:06 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 466IO6uQ090585; Sat, 6 Jul 2024 18:24:06 GMT (envelope-from git) Date: Sat, 6 Jul 2024 18:24:06 GMT Message-Id: <202407061824.466IO6uQ090585@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Christos Margiolis Subject: git: fc76e24e583d - main - sound: Fix lock order reversals in mseq_open() List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: christos X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: fc76e24e583d45a3a59fd7ad4e603c0679eaf572 Auto-Submitted: auto-generated The branch main has been updated by christos: URL: https://cgit.FreeBSD.org/src/commit/?id=fc76e24e583d45a3a59fd7ad4e603c0679eaf572 commit fc76e24e583d45a3a59fd7ad4e603c0679eaf572 Author: Christos Margiolis AuthorDate: 2024-07-06 18:22:21 +0000 Commit: Christos Margiolis 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 #include #include +#include #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);