Re: Sound/seq: hung task in odev_open - unuse callback sleeps under list_mutex

Re: Sound/seq: hung task in odev_open - unuse callback sleeps under list_mutex

[Takashi Iwai]

On Thu, 17 Sep 2026 04:41:31 +0200,
A Akhil wrote:
> 
> 
> To: tiwai@suse.com, perex@perex.cz
> Cc: linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
>     syzkaller-bugs@googlegroups.com
> Subject: sound/seq: hung task in odev_open - unuse callback sleeps under
> list_mutex
> 
> Hi,
> 
> I've been looking at this syzbot report:
> 
>   INFO: task hung in odev_open (5)
>   https://syzkaller.appspot.com/bug?extid=825b7e3a03dd072c187f
> 
> The hang isn't really in the OSS code where it gets reported. syzbot only
> shows the task waiting on register_mutex in odev_open(); the one actually
> holding things up is this:
> 
>   odev_release                            takes register_mutex
>    snd_seq_oss_release / snd_seq_oss_reset
>     snd_seq_oss_synth_reset
>      snd_seq_oss_midi_close               takes mdev->open_mutex
>       snd_seq_ioctl_unsubscribe_port
>        snd_seq_port_disconnect
>         __delete_and_unsubscribe_port     called with grp->list_mutex held
>          midisynth_unuse                  seq_midi.c:298
>           snd_rawmidi_drain_output        sleeps 10*HZ per substream
>            schedule_timeout
> 
> delete_and_unsubscribe_port() holds grp->list_mutex for write while the
> unuse callback runs, and midisynth_unuse() ends up in
> snd_rawmidi_drain_output(), which waits 10*HZ per substream. With ~8
> substreams I measured close() taking 82 seconds. Anything calling
> snd_seq_port_connect() blocks on the same rwsem, and in the OSS path that
> connect runs under register_mutex, so every other odev_open() piles up
> behind it and the hung task detector fires.
> 
> I tried narrowing register_mutex in odev_release, and then open_mutex in
> snd_seq_oss_midi_close. Both build, both still hang - the wait just moves
> down a level each time, ending up in snd_seq_port_connect().
> 
> Moving unsubscribe_port() out of the rwsem looked wrong to me: grp->count
> is protected by it, and the comment above subscribe_port() says open and
> close are only invoked on the 0->1 and 1->0 transitions. If close ran
> unlocked, a concurrent subscribe could call open first.
> 
> That pairing also means an opener of the same port has to wait for a close
> in progress anyway, so I don't think lock narrowing can fix this - the
> teardown just has to stop taking tens of seconds.
> 
> So the drain has to stop blocking the teardown. I can see three ways to do
> that, but all of them change when or whether pending MIDI output gets
> flushed, and that's not something I want to decide on my own:
> 
>   - drop the output instead of draining it in midisynth_unuse()
>   - move the drain to a workqueue so the unuse callback doesn't sleep
>   - keep draining but cap the total wait
> 
> Which of those would you take? I can write it and test it.
> 
> (v7.3-rc3, 9b87fdc9af2f, qemu with dummy_hcd + raw-gadget. The syz repro
> alone didn't trigger it for me - I needed a second thread opening
> /dev/sequencer2 while another one closes it. That hits the hang in under a
> minute.)

IIUC, this is no real "hang" that locks up forever but just went over
threshold in mutex?  If so, we can avoid taking too long mutex like
the (totally untested) patch below?


thanks,

Takashi

-- 8< --
diff --git a/sound/core/seq/oss/seq_oss.c b/sound/core/seq/oss/seq_oss.c
index 2835576040ed..c790b1cd451a 100644
--- a/sound/core/seq/oss/seq_oss.c
+++ b/sound/core/seq/oss/seq_oss.c
@@ -132,13 +132,19 @@ static int
 odev_release(struct inode *inode, struct file *file)
 {
 	struct seq_oss_devinfo *dp;
+	int index;
 
 	dp = file->private_data;
 	if (!dp)
 		return 0;
 
-	guard(mutex)(&register_mutex);
+	scoped_guard(mutex, &register_mutex)
+		snd_seq_oss_detach(dp);
+	index = dp->index;
 	snd_seq_oss_release(dp);
+	scoped_guard(mutex, &register_mutex)
+		snd_seq_oss_detach_done(index);
+
 	return 0;
 }
 
@@ -149,6 +155,8 @@ odev_read(struct file *file, char __user *buf, size_t count, loff_t *offset)
 	dp = file->private_data;
 	if (snd_BUG_ON(!dp))
 		return -ENXIO;
+	if (dp->closing)
+		return -EBADFD;
 	return snd_seq_oss_read(dp, buf, count);
 }
 
@@ -160,6 +168,8 @@ odev_write(struct file *file, const char __user *buf, size_t count, loff_t *offs
 	dp = file->private_data;
 	if (snd_BUG_ON(!dp))
 		return -ENXIO;
+	if (dp->closing)
+		return -EBADFD;
 	return snd_seq_oss_write(dp, buf, count, file);
 }
 
diff --git a/sound/core/seq/oss/seq_oss_device.h b/sound/core/seq/oss/seq_oss_device.h
index 935cf3df0b30..ee318ea1a096 100644
--- a/sound/core/seq/oss/seq_oss_device.h
+++ b/sound/core/seq/oss/seq_oss_device.h
@@ -72,6 +72,7 @@ struct seq_oss_devinfo {
 	int cseq;	/* sequencer client number */
 	int port;	/* sequencer port number */
 	int queue;	/* sequencer queue number */
+	bool closing;
 
 	struct snd_seq_addr addr;	/* address of this device */
 
@@ -107,7 +108,9 @@ int snd_seq_oss_delete_client(void);
 
 /* device file interface */
 int snd_seq_oss_open(struct file *file, int level);
+void snd_seq_oss_detach(struct seq_oss_devinfo *dp);
 void snd_seq_oss_release(struct seq_oss_devinfo *dp);
+void snd_seq_oss_detach_done(int index);
 int snd_seq_oss_ioctl(struct seq_oss_devinfo *dp, unsigned int cmd, unsigned long arg);
 int snd_seq_oss_read(struct seq_oss_devinfo *dev, char __user *buf, int count);
 int snd_seq_oss_write(struct seq_oss_devinfo *dp, const char __user *buf, int count, struct file *opt);
diff --git a/sound/core/seq/oss/seq_oss_init.c b/sound/core/seq/oss/seq_oss_init.c
index 6586e07431c3..3aece0d2981a 100644
--- a/sound/core/seq/oss/seq_oss_init.c
+++ b/sound/core/seq/oss/seq_oss_init.c
@@ -33,6 +33,7 @@ static int system_port __ro_after_init = -1;
 static int num_clients;
 static struct seq_oss_devinfo *client_table[SNDRV_SEQ_OSS_MAX_CLIENTS];
 
+#define SEQ_OSS_DETACHED	((struct seq_oss_devinfo *)-1)
 
 /*
  * prototypes
@@ -396,14 +397,23 @@ free_devinfo(void *private)
 /*
  * close sequencer device
  */
+void snd_seq_oss_detach(struct seq_oss_devinfo *dp)
+{
+	dp->closing = true;
+	client_table[dp->index] = SEQ_OSS_DETACHED;
+	num_clients--;
+}
+
+void snd_seq_oss_detach_done(int index)
+{
+	client_table[index] = NULL;
+}
+
 void
 snd_seq_oss_release(struct seq_oss_devinfo *dp)
 {
 	int queue;
 
-	client_table[dp->index] = NULL;
-	num_clients--;
-
 	snd_seq_oss_reset(dp);
 
 	snd_seq_oss_synth_cleanup(dp);
@@ -475,7 +485,7 @@ snd_seq_oss_system_info_read(struct snd_info_buffer *buf)
 	for (i = 0; i < num_clients; i++) {
 		snd_iprintf(buf, "\nApplication %d: ", i);
 		dp = client_table[i];
-		if (!dp) {
+		if (!dp || dp == SEQ_OSS_DETACHED) {
 			snd_iprintf(buf, "*empty*\n");
 			continue;
 		}
diff --git a/sound/core/seq/oss/seq_oss_ioctl.c b/sound/core/seq/oss/seq_oss_ioctl.c
index f1a79776773f..2b4930a56a22 100644
--- a/sound/core/seq/oss/seq_oss_ioctl.c
+++ b/sound/core/seq/oss/seq_oss_ioctl.c
@@ -66,6 +66,9 @@ snd_seq_oss_ioctl(struct seq_oss_devinfo *dp, unsigned int cmd, unsigned long ca
 	void __user *arg = (void __user *)carg;
 	int __user *p = arg;
 
+	if (dp->closing)
+		return -EBADFD;
+
 	switch (cmd) {
 	case SNDCTL_TMR_TIMEBASE:
 	case SNDCTL_TMR_TEMPO:

Re: Sound/seq: hung task in odev_open - unuse callback sleeps under list_mutex

[A Akhil]

Hi Takashi,

Thanks for the quick patch. Right, it's not a deadlock, that was my
reading of it too. Nothing locks up permanently, it's just a wait long
enough to trip the detector. That's the reason I bothered with it though.
On the syzbot run the wait was over 140 s, and for the whole of that
anything else touching the sequencer is stuck behind it.

I ran it on the setup I used to find this in the first place. One thread
opening and closing /dev/sequencer2 while two others try to open it,
hung_task_timeout_secs=30, three runs each.

  variant                       hung tasks    worst opener block
  ---------------------------------------------------------------
  unpatched                     10, 10, 10    ~130 s
  your patch                    0, 0, 7       30.7 / 30.6 / 129.9 s
  yours + open_mutex narrowed   7, 0, 0       129.9 / 41.3 / 40.9 s
  drain bounded to 1s           0, 0, 0       20.6 / 18.4 / 21.8 s
  drop instead of drain         0, 0, 0       4.0 / 2.4 / 2.4 s

Your patch builds clean and clearly helps, two of the three runs were
completely quiet. The third still hung though, and when it does it's
coming from the opening side rather than the closing side.

  task 98:  odev_open                      holds register_mutex
             snd_seq_oss_open
              snd_seq_oss_synth_setup_midi
               snd_seq_oss_midi_open+0x8e  blocked on mdev->open_mutex
  tasks 97, 99: blocked on register_mutex owned by 98 -> hung task

An opener grabs register_mutex, blocks deeper down, and sits on it while
it waits.

I tried stacking my earlier open_mutex narrowing in snd_seq_oss_midi_close
on top of yours, so the closer would be holding neither lock over the
drain. Still hung 1 in 3, and the wait had just moved further down again,
this time into snd_seq_port_connect() on grp->list_mutex.

Three attempts at moving locks around now (mine, yours, both together) and
every one of them just shifts where the wait happens. I don't think the
locking is really the problem here. It's the 10*HZ per substream.

Bounding the drain to 1*HZ in midisynth_unuse() does stop the hangs, but
the cap is per substream and there are about twenty of them, so teardown
still runs ~20 s. Capping the total across the teardown would do better,
though that's more surgery.

The one that actually worked was swapping snd_rawmidi_drain_output() for
snd_rawmidi_drop_output() in midisynth_unuse(). ~2.4 s, nothing hung in
any run. Most of what's left there isn't even the data wait, it's the
msleep(50) per substream for the Tx FIFOs.

I did wonder about throwing the data away, so I went and looked. As far as
I can tell snd_rawmidi_drain_output() already drops the buffer once the
10*HZ expires, so the bytes are gone either way and dropping early only
costs whatever the device would have taken during the timeout. For a
device that isn't draining, which is the case that gets us here, that's
nothing. Tell me if I've misread that.

So, do you want me to send the drop version as a proper patch? Or would
you rather bound the drain, in which case I can try a total cap instead of
a per substream one. I can also test a revised version of yours if you'd
prefer to keep this inside seq_oss.

Thanks,
Akhil Arul




On Thu, 17 Sept 2026 at 16:37, Takashi Iwai <tiwai@suse.de> wrote:
>
> On Thu, 17 Sep 2026 04:41:31 +0200,
> A Akhil wrote:
> >
> >
> > To: tiwai@suse.com, perex@perex.cz
> > Cc: linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org,
> >     syzkaller-bugs@googlegroups.com
> > Subject: sound/seq: hung task in odev_open - unuse callback sleeps under
> > list_mutex
> >
> > Hi,
> >
> > I've been looking at this syzbot report:
> >
> >   INFO: task hung in odev_open (5)
> >   https://syzkaller.appspot.com/bug?extid=825b7e3a03dd072c187f
> >
> > The hang isn't really in the OSS code where it gets reported. syzbot only
> > shows the task waiting on register_mutex in odev_open(); the one actually
> > holding things up is this:
> >
> >   odev_release                            takes register_mutex
> >    snd_seq_oss_release / snd_seq_oss_reset
> >     snd_seq_oss_synth_reset
> >      snd_seq_oss_midi_close               takes mdev->open_mutex
> >       snd_seq_ioctl_unsubscribe_port
> >        snd_seq_port_disconnect
> >         __delete_and_unsubscribe_port     called with grp->list_mutex held
> >          midisynth_unuse                  seq_midi.c:298
> >           snd_rawmidi_drain_output        sleeps 10*HZ per substream
> >            schedule_timeout
> >
> > delete_and_unsubscribe_port() holds grp->list_mutex for write while the
> > unuse callback runs, and midisynth_unuse() ends up in
> > snd_rawmidi_drain_output(), which waits 10*HZ per substream. With ~8
> > substreams I measured close() taking 82 seconds. Anything calling
> > snd_seq_port_connect() blocks on the same rwsem, and in the OSS path that
> > connect runs under register_mutex, so every other odev_open() piles up
> > behind it and the hung task detector fires.
> >
> > I tried narrowing register_mutex in odev_release, and then open_mutex in
> > snd_seq_oss_midi_close. Both build, both still hang - the wait just moves
> > down a level each time, ending up in snd_seq_port_connect().
> >
> > Moving unsubscribe_port() out of the rwsem looked wrong to me: grp->count
> > is protected by it, and the comment above subscribe_port() says open and
> > close are only invoked on the 0->1 and 1->0 transitions. If close ran
> > unlocked, a concurrent subscribe could call open first.
> >
> > That pairing also means an opener of the same port has to wait for a close
> > in progress anyway, so I don't think lock narrowing can fix this - the
> > teardown just has to stop taking tens of seconds.
> >
> > So the drain has to stop blocking the teardown. I can see three ways to do
> > that, but all of them change when or whether pending MIDI output gets
> > flushed, and that's not something I want to decide on my own:
> >
> >   - drop the output instead of draining it in midisynth_unuse()
> >   - move the drain to a workqueue so the unuse callback doesn't sleep
> >   - keep draining but cap the total wait
> >
> > Which of those would you take? I can write it and test it.
> >
> > (v7.3-rc3, 9b87fdc9af2f, qemu with dummy_hcd + raw-gadget. The syz repro
> > alone didn't trigger it for me - I needed a second thread opening
> > /dev/sequencer2 while another one closes it. That hits the hang in under a
> > minute.)
>
> IIUC, this is no real "hang" that locks up forever but just went over
> threshold in mutex?  If so, we can avoid taking too long mutex like
> the (totally untested) patch below?
>
>
> thanks,
>
> Takashi
>
> -- 8< --
> diff --git a/sound/core/seq/oss/seq_oss.c b/sound/core/seq/oss/seq_oss.c
> index 2835576040ed..c790b1cd451a 100644
> --- a/sound/core/seq/oss/seq_oss.c
> +++ b/sound/core/seq/oss/seq_oss.c
> @@ -132,13 +132,19 @@ static int
>  odev_release(struct inode *inode, struct file *file)
>  {
>         struct seq_oss_devinfo *dp;
> +       int index;
>
>         dp = file->private_data;
>         if (!dp)
>                 return 0;
>
> -       guard(mutex)(&register_mutex);
> +       scoped_guard(mutex, &register_mutex)
> +               snd_seq_oss_detach(dp);
> +       index = dp->index;
>         snd_seq_oss_release(dp);
> +       scoped_guard(mutex, &register_mutex)
> +               snd_seq_oss_detach_done(index);
> +
>         return 0;
>  }
>
> @@ -149,6 +155,8 @@ odev_read(struct file *file, char __user *buf, size_t count, loff_t *offset)
>         dp = file->private_data;
>         if (snd_BUG_ON(!dp))
>                 return -ENXIO;
> +       if (dp->closing)
> +               return -EBADFD;
>         return snd_seq_oss_read(dp, buf, count);
>  }
>
> @@ -160,6 +168,8 @@ odev_write(struct file *file, const char __user *buf, size_t count, loff_t *offs
>         dp = file->private_data;
>         if (snd_BUG_ON(!dp))
>                 return -ENXIO;
> +       if (dp->closing)
> +               return -EBADFD;
>         return snd_seq_oss_write(dp, buf, count, file);
>  }
>
> diff --git a/sound/core/seq/oss/seq_oss_device.h b/sound/core/seq/oss/seq_oss_device.h
> index 935cf3df0b30..ee318ea1a096 100644
> --- a/sound/core/seq/oss/seq_oss_device.h
> +++ b/sound/core/seq/oss/seq_oss_device.h
> @@ -72,6 +72,7 @@ struct seq_oss_devinfo {
>         int cseq;       /* sequencer client number */
>         int port;       /* sequencer port number */
>         int queue;      /* sequencer queue number */
> +       bool closing;
>
>         struct snd_seq_addr addr;       /* address of this device */
>
> @@ -107,7 +108,9 @@ int snd_seq_oss_delete_client(void);
>
>  /* device file interface */
>  int snd_seq_oss_open(struct file *file, int level);
> +void snd_seq_oss_detach(struct seq_oss_devinfo *dp);
>  void snd_seq_oss_release(struct seq_oss_devinfo *dp);
> +void snd_seq_oss_detach_done(int index);
>  int snd_seq_oss_ioctl(struct seq_oss_devinfo *dp, unsigned int cmd, unsigned long arg);
>  int snd_seq_oss_read(struct seq_oss_devinfo *dev, char __user *buf, int count);
>  int snd_seq_oss_write(struct seq_oss_devinfo *dp, const char __user *buf, int count, struct file *opt);
> diff --git a/sound/core/seq/oss/seq_oss_init.c b/sound/core/seq/oss/seq_oss_init.c
> index 6586e07431c3..3aece0d2981a 100644
> --- a/sound/core/seq/oss/seq_oss_init.c
> +++ b/sound/core/seq/oss/seq_oss_init.c
> @@ -33,6 +33,7 @@ static int system_port __ro_after_init = -1;
>  static int num_clients;
>  static struct seq_oss_devinfo *client_table[SNDRV_SEQ_OSS_MAX_CLIENTS];
>
> +#define SEQ_OSS_DETACHED       ((struct seq_oss_devinfo *)-1)
>
>  /*
>   * prototypes
> @@ -396,14 +397,23 @@ free_devinfo(void *private)
>  /*
>   * close sequencer device
>   */
> +void snd_seq_oss_detach(struct seq_oss_devinfo *dp)
> +{
> +       dp->closing = true;
> +       client_table[dp->index] = SEQ_OSS_DETACHED;
> +       num_clients--;
> +}
> +
> +void snd_seq_oss_detach_done(int index)
> +{
> +       client_table[index] = NULL;
> +}
> +
>  void
>  snd_seq_oss_release(struct seq_oss_devinfo *dp)
>  {
>         int queue;
>
> -       client_table[dp->index] = NULL;
> -       num_clients--;
> -
>         snd_seq_oss_reset(dp);
>
>         snd_seq_oss_synth_cleanup(dp);
> @@ -475,7 +485,7 @@ snd_seq_oss_system_info_read(struct snd_info_buffer *buf)
>         for (i = 0; i < num_clients; i++) {
>                 snd_iprintf(buf, "\nApplication %d: ", i);
>                 dp = client_table[i];
> -               if (!dp) {
> +               if (!dp || dp == SEQ_OSS_DETACHED) {
>                         snd_iprintf(buf, "*empty*\n");
>                         continue;
>                 }
> diff --git a/sound/core/seq/oss/seq_oss_ioctl.c b/sound/core/seq/oss/seq_oss_ioctl.c
> index f1a79776773f..2b4930a56a22 100644
> --- a/sound/core/seq/oss/seq_oss_ioctl.c
> +++ b/sound/core/seq/oss/seq_oss_ioctl.c
> @@ -66,6 +66,9 @@ snd_seq_oss_ioctl(struct seq_oss_devinfo *dp, unsigned int cmd, unsigned long ca
>         void __user *arg = (void __user *)carg;
>         int __user *p = arg;
>
> +       if (dp->closing)
> +               return -EBADFD;
> +
>         switch (cmd) {
>         case SNDCTL_TMR_TIMEBASE:
>         case SNDCTL_TMR_TEMPO:

Re: Sound/seq: hung task in odev_open - unuse callback sleeps under list_mutex

[Takashi Iwai]

On Thu, 17 Sep 2026 16:11:27 +0200,
A Akhil wrote:
> 
> Hi Takashi,
> 
> Thanks for the quick patch. Right, it's not a deadlock, that was my
> reading of it too. Nothing locks up permanently, it's just a wait long
> enough to trip the detector. That's the reason I bothered with it though.
> On the syzbot run the wait was over 140 s, and for the whole of that
> anything else touching the sequencer is stuck behind it.
> 
> I ran it on the setup I used to find this in the first place. One thread
> opening and closing /dev/sequencer2 while two others try to open it,
> hung_task_timeout_secs=30, three runs each.
> 
>   variant                       hung tasks    worst opener block
>   ---------------------------------------------------------------
>   unpatched                     10, 10, 10    ~130 s
>   your patch                    0, 0, 7       30.7 / 30.6 / 129.9 s
>   yours + open_mutex narrowed   7, 0, 0       129.9 / 41.3 / 40.9 s
>   drain bounded to 1s           0, 0, 0       20.6 / 18.4 / 21.8 s
>   drop instead of drain         0, 0, 0       4.0 / 2.4 / 2.4 s
> 
> Your patch builds clean and clearly helps, two of the three runs were
> completely quiet. The third still hung though, and when it does it's
> coming from the opening side rather than the closing side.
> 
>   task 98:  odev_open                      holds register_mutex
>              snd_seq_oss_open
>               snd_seq_oss_synth_setup_midi
>                snd_seq_oss_midi_open+0x8e  blocked on mdev->open_mutex
>   tasks 97, 99: blocked on register_mutex owned by 98 -> hung task
> 
> An opener grabs register_mutex, blocks deeper down, and sits on it while
> it waits.
> 
> I tried stacking my earlier open_mutex narrowing in snd_seq_oss_midi_close
> on top of yours, so the closer would be holding neither lock over the
> drain. Still hung 1 in 3, and the wait had just moved further down again,
> this time into snd_seq_port_connect() on grp->list_mutex.
> 
> Three attempts at moving locks around now (mine, yours, both together) and
> every one of them just shifts where the wait happens. I don't think the
> locking is really the problem here. It's the 10*HZ per substream.
> 
> Bounding the drain to 1*HZ in midisynth_unuse() does stop the hangs, but
> the cap is per substream and there are about twenty of them, so teardown
> still runs ~20 s. Capping the total across the teardown would do better,
> though that's more surgery.
> 
> The one that actually worked was swapping snd_rawmidi_drain_output() for
> snd_rawmidi_drop_output() in midisynth_unuse(). ~2.4 s, nothing hung in
> any run. Most of what's left there isn't even the data wait, it's the
> msleep(50) per substream for the Tx FIFOs.
> 
> I did wonder about throwing the data away, so I went and looked. As far as
> I can tell snd_rawmidi_drain_output() already drops the buffer once the
> 10*HZ expires, so the bytes are gone either way and dropping early only
> costs whatever the device would have taken during the timeout. For a
> device that isn't draining, which is the case that gets us here, that's
> nothing. Tell me if I've misread that.
> 
> So, do you want me to send the drop version as a proper patch? Or would
> you rather bound the drain, in which case I can try a total cap instead of
> a per substream one. I can also test a revised version of yours if you'd
> prefer to keep this inside seq_oss.

Hm, dropping isn't optimal, as that's a clear behavior change; there
can be pending bytes even in the real use case.

Actually, shortening the drain limit would be an easier way.
Practically seen, 1 second should be enough for the real hardware.
If this is enough for the syzkaller report, we can take it quickly.

Another option would be to offload the snd_seq_kernel_client_ctl()
calls in seq_oss_midi.c snd_seq_oss_midi_open() and close() to a work,
as you pointed out previously.  This would be relatively safe, I
suppose.

In general, I'd like to avoid touching too much outside the OSS
emulation layer.  The shortening of the drain limit would be OK, but
restructuring else isn't preferred.

In anyway, if you can pitch some good workaround, I'll happily take.

(But I'm going to be off from tomorrow, so it'll be continued after my
vacation.)


thanks,

Takashi

Re: Sound/seq: hung task in odev_open - unuse callback sleeps under list_mutex

[A Akhil]

Hi Takashi,

Fair enough on dropping, I'll leave that alone.

I'd actually already tried the 1 second limit before your mail came in, so
the short answer is yes, it holds up. Same rig as before, one thread
opening and closing /dev/sequencer2 while two others open it, three runs
each:

  unpatched              hung 10, 10, 10    worst block ~130 s
  your patch             hung  0,  0,  7    30.7 / 30.6 / 129.9 s
  drain capped at 1*HZ   hung  0,  0,  0    20.6 / 18.4 / 21.8 s
  progress detection     hung  0,  0,  0    11.0 / 11.1 / 11.6 s  <- new

I was running with hung_task_timeout_secs=30 to make the failures turn up
faster, so at the 120 s default the 1*HZ cap has plenty of room.

The last row is new, and it's the one I'd rather pitch. Rather than
picking a constant, it just checks whether the device is draining at all:
wait in HZ/5 slices and give up once avail stops changing, with 10*HZ
still there as the outer bound. A device that works drains completely as
before, nothing gets truncated, and a stalled one gets spotted in about
400 ms. Comes out at roughly half the teardown time of the 1 s cap here,
and it's about 20 lines inside snd_rawmidi_drain_output(), nothing
outside it.

One note in case you do go with the cap. MIDI is 3125 bytes/s and the
buffer is PAGE_SIZE, so a completely full 4K buffer needs 1.31 s to get
out. 1*HZ would clip that, and 10*HZ already clips a full buffer on 64K
page kernels. Unlikely to bite in practice since the buffer is rarely
full at close, but it's the reason I went looking for something that
doesn't depend on a fixed number.

Also a correction to my last mail: I said about twenty substreams, which
was wrong. It's one rawmidi device, midiC0D0, and the teardown just calls
the drain repeatedly, around 31 times per cycle.

If either of these looks fine to you I'll send it as a proper patch. And
if I find something better in the meantime I'll pitch that instead.

Enjoy your vacation.

Thanks,
Akhil Arul


On Thu, 17 Sept 2026 at 20:25, Takashi Iwai <tiwai@suse.de> wrote:
>
> On Thu, 17 Sep 2026 16:11:27 +0200,
> A Akhil wrote:
> >
> > Hi Takashi,
> >
> > Thanks for the quick patch. Right, it's not a deadlock, that was my
> > reading of it too. Nothing locks up permanently, it's just a wait long
> > enough to trip the detector. That's the reason I bothered with it though.
> > On the syzbot run the wait was over 140 s, and for the whole of that
> > anything else touching the sequencer is stuck behind it.
> >
> > I ran it on the setup I used to find this in the first place. One thread
> > opening and closing /dev/sequencer2 while two others try to open it,
> > hung_task_timeout_secs=30, three runs each.
> >
> >   variant                       hung tasks    worst opener block
> >   ---------------------------------------------------------------
> >   unpatched                     10, 10, 10    ~130 s
> >   your patch                    0, 0, 7       30.7 / 30.6 / 129.9 s
> >   yours + open_mutex narrowed   7, 0, 0       129.9 / 41.3 / 40.9 s
> >   drain bounded to 1s           0, 0, 0       20.6 / 18.4 / 21.8 s
> >   drop instead of drain         0, 0, 0       4.0 / 2.4 / 2.4 s
> >
> > Your patch builds clean and clearly helps, two of the three runs were
> > completely quiet. The third still hung though, and when it does it's
> > coming from the opening side rather than the closing side.
> >
> >   task 98:  odev_open                      holds register_mutex
> >              snd_seq_oss_open
> >               snd_seq_oss_synth_setup_midi
> >                snd_seq_oss_midi_open+0x8e  blocked on mdev->open_mutex
> >   tasks 97, 99: blocked on register_mutex owned by 98 -> hung task
> >
> > An opener grabs register_mutex, blocks deeper down, and sits on it while
> > it waits.
> >
> > I tried stacking my earlier open_mutex narrowing in snd_seq_oss_midi_close
> > on top of yours, so the closer would be holding neither lock over the
> > drain. Still hung 1 in 3, and the wait had just moved further down again,
> > this time into snd_seq_port_connect() on grp->list_mutex.
> >
> > Three attempts at moving locks around now (mine, yours, both together) and
> > every one of them just shifts where the wait happens. I don't think the
> > locking is really the problem here. It's the 10*HZ per substream.
> >
> > Bounding the drain to 1*HZ in midisynth_unuse() does stop the hangs, but
> > the cap is per substream and there are about twenty of them, so teardown
> > still runs ~20 s. Capping the total across the teardown would do better,
> > though that's more surgery.
> >
> > The one that actually worked was swapping snd_rawmidi_drain_output() for
> > snd_rawmidi_drop_output() in midisynth_unuse(). ~2.4 s, nothing hung in
> > any run. Most of what's left there isn't even the data wait, it's the
> > msleep(50) per substream for the Tx FIFOs.
> >
> > I did wonder about throwing the data away, so I went and looked. As far as
> > I can tell snd_rawmidi_drain_output() already drops the buffer once the
> > 10*HZ expires, so the bytes are gone either way and dropping early only
> > costs whatever the device would have taken during the timeout. For a
> > device that isn't draining, which is the case that gets us here, that's
> > nothing. Tell me if I've misread that.
> >
> > So, do you want me to send the drop version as a proper patch? Or would
> > you rather bound the drain, in which case I can try a total cap instead of
> > a per substream one. I can also test a revised version of yours if you'd
> > prefer to keep this inside seq_oss.
>
> Hm, dropping isn't optimal, as that's a clear behavior change; there
> can be pending bytes even in the real use case.
>
> Actually, shortening the drain limit would be an easier way.
> Practically seen, 1 second should be enough for the real hardware.
> If this is enough for the syzkaller report, we can take it quickly.
>
> Another option would be to offload the snd_seq_kernel_client_ctl()
> calls in seq_oss_midi.c snd_seq_oss_midi_open() and close() to a work,
> as you pointed out previously.  This would be relatively safe, I
> suppose.
>
> In general, I'd like to avoid touching too much outside the OSS
> emulation layer.  The shortening of the drain limit would be OK, but
> restructuring else isn't preferred.
>
> In anyway, if you can pitch some good workaround, I'll happily take.
>
> (But I'm going to be off from tomorrow, so it'll be continued after my
> vacation.)
>
>
> thanks,
>
> Takashi

Re: Sound/seq: hung task in odev_open - unuse callback sleeps under list_mutex

[Takashi Iwai]

On Thu, 17 Sep 2026 17:42:36 +0200,
A Akhil wrote:
> 
> Hi Takashi,
> 
> Fair enough on dropping, I'll leave that alone.
> 
> I'd actually already tried the 1 second limit before your mail came in, so
> the short answer is yes, it holds up. Same rig as before, one thread
> opening and closing /dev/sequencer2 while two others open it, three runs
> each:
> 
>   unpatched              hung 10, 10, 10    worst block ~130 s
>   your patch             hung  0,  0,  7    30.7 / 30.6 / 129.9 s
>   drain capped at 1*HZ   hung  0,  0,  0    20.6 / 18.4 / 21.8 s
>   progress detection     hung  0,  0,  0    11.0 / 11.1 / 11.6 s  <- new
> 
> I was running with hung_task_timeout_secs=30 to make the failures turn up
> faster, so at the 120 s default the 1*HZ cap has plenty of room.
> 
> The last row is new, and it's the one I'd rather pitch. Rather than
> picking a constant, it just checks whether the device is draining at all:
> wait in HZ/5 slices and give up once avail stops changing, with 10*HZ
> still there as the outer bound. A device that works drains completely as
> before, nothing gets truncated, and a stalled one gets spotted in about
> 400 ms. Comes out at roughly half the teardown time of the 1 s cap here,
> and it's about 20 lines inside snd_rawmidi_drain_output(), nothing
> outside it.

Sounds like a good idea.

> One note in case you do go with the cap. MIDI is 3125 bytes/s and the
> buffer is PAGE_SIZE, so a completely full 4K buffer needs 1.31 s to get
> out. 1*HZ would clip that, and 10*HZ already clips a full buffer on 64K
> page kernels. Unlikely to bite in practice since the buffer is rarely
> full at close, but it's the reason I went looking for something that
> doesn't depend on a fixed number.

Fair enough.

> Also a correction to my last mail: I said about twenty substreams, which
> was wrong. It's one rawmidi device, midiC0D0, and the teardown just calls
> the drain repeatedly, around 31 times per cycle.
> 
> If either of these looks fine to you I'll send it as a proper patch. And
> if I find something better in the meantime I'll pitch that instead.

The idea with an adaptive timeout looks promising, so I'd like to
check this primarily, unless it becomes too complex.

> Enjoy your vacation.

Thanks!


Takashi

> 
> Thanks,
> Akhil Arul
> 
> 
> On Thu, 17 Sept 2026 at 20:25, Takashi Iwai <tiwai@suse.de> wrote:
> >
> > On Thu, 17 Sep 2026 16:11:27 +0200,
> > A Akhil wrote:
> > >
> > > Hi Takashi,
> > >
> > > Thanks for the quick patch. Right, it's not a deadlock, that was my
> > > reading of it too. Nothing locks up permanently, it's just a wait long
> > > enough to trip the detector. That's the reason I bothered with it though.
> > > On the syzbot run the wait was over 140 s, and for the whole of that
> > > anything else touching the sequencer is stuck behind it.
> > >
> > > I ran it on the setup I used to find this in the first place. One thread
> > > opening and closing /dev/sequencer2 while two others try to open it,
> > > hung_task_timeout_secs=30, three runs each.
> > >
> > >   variant                       hung tasks    worst opener block
> > >   ---------------------------------------------------------------
> > >   unpatched                     10, 10, 10    ~130 s
> > >   your patch                    0, 0, 7       30.7 / 30.6 / 129.9 s
> > >   yours + open_mutex narrowed   7, 0, 0       129.9 / 41.3 / 40.9 s
> > >   drain bounded to 1s           0, 0, 0       20.6 / 18.4 / 21.8 s
> > >   drop instead of drain         0, 0, 0       4.0 / 2.4 / 2.4 s
> > >
> > > Your patch builds clean and clearly helps, two of the three runs were
> > > completely quiet. The third still hung though, and when it does it's
> > > coming from the opening side rather than the closing side.
> > >
> > >   task 98:  odev_open                      holds register_mutex
> > >              snd_seq_oss_open
> > >               snd_seq_oss_synth_setup_midi
> > >                snd_seq_oss_midi_open+0x8e  blocked on mdev->open_mutex
> > >   tasks 97, 99: blocked on register_mutex owned by 98 -> hung task
> > >
> > > An opener grabs register_mutex, blocks deeper down, and sits on it while
> > > it waits.
> > >
> > > I tried stacking my earlier open_mutex narrowing in snd_seq_oss_midi_close
> > > on top of yours, so the closer would be holding neither lock over the
> > > drain. Still hung 1 in 3, and the wait had just moved further down again,
> > > this time into snd_seq_port_connect() on grp->list_mutex.
> > >
> > > Three attempts at moving locks around now (mine, yours, both together) and
> > > every one of them just shifts where the wait happens. I don't think the
> > > locking is really the problem here. It's the 10*HZ per substream.
> > >
> > > Bounding the drain to 1*HZ in midisynth_unuse() does stop the hangs, but
> > > the cap is per substream and there are about twenty of them, so teardown
> > > still runs ~20 s. Capping the total across the teardown would do better,
> > > though that's more surgery.
> > >
> > > The one that actually worked was swapping snd_rawmidi_drain_output() for
> > > snd_rawmidi_drop_output() in midisynth_unuse(). ~2.4 s, nothing hung in
> > > any run. Most of what's left there isn't even the data wait, it's the
> > > msleep(50) per substream for the Tx FIFOs.
> > >
> > > I did wonder about throwing the data away, so I went and looked. As far as
> > > I can tell snd_rawmidi_drain_output() already drops the buffer once the
> > > 10*HZ expires, so the bytes are gone either way and dropping early only
> > > costs whatever the device would have taken during the timeout. For a
> > > device that isn't draining, which is the case that gets us here, that's
> > > nothing. Tell me if I've misread that.
> > >
> > > So, do you want me to send the drop version as a proper patch? Or would
> > > you rather bound the drain, in which case I can try a total cap instead of
> > > a per substream one. I can also test a revised version of yours if you'd
> > > prefer to keep this inside seq_oss.
> >
> > Hm, dropping isn't optimal, as that's a clear behavior change; there
> > can be pending bytes even in the real use case.
> >
> > Actually, shortening the drain limit would be an easier way.
> > Practically seen, 1 second should be enough for the real hardware.
> > If this is enough for the syzkaller report, we can take it quickly.
> >
> > Another option would be to offload the snd_seq_kernel_client_ctl()
> > calls in seq_oss_midi.c snd_seq_oss_midi_open() and close() to a work,
> > as you pointed out previously.  This would be relatively safe, I
> > suppose.
> >
> > In general, I'd like to avoid touching too much outside the OSS
> > emulation layer.  The shortening of the drain limit would be OK, but
> > restructuring else isn't preferred.
> >
> > In anyway, if you can pitch some good workaround, I'll happily take.
> >
> > (But I'm going to be off from tomorrow, so it'll be continued after my
> > vacation.)
> >
> >
> > thanks,
> >
> > Takashi

[PATCH] ALSA: rawmidi: give up draining output when the device stops draining

[Akhil Arul]

snd_rawmidi_drain_output() waits up to 10 seconds for the output buffer
to empty.  The wait is unconditional, so a substream that has stopped
being consumed costs the full timeout on every call.

The sequencer OSS emulation makes that expensive.  midisynth_unuse() runs
as the port unuse callback with grp->list_mutex held for write, and calls
snd_rawmidi_drain_output().  A single teardown closes every OSS midi port
of the device, so an unresponsive device exposing many ports holds the
rwsem for minutes.  snd_seq_port_connect() needs the same rwsem, and in
the OSS path it runs under register_mutex, so every other odev_open()
queues up behind it until the hung task detector fires:

  INFO: task syz.4.21:6176 blocked for more than 143 seconds.
    __mutex_lock
    odev_open
    chrdev_open
    vfs_open
    path_openat

Detect a stalled drain by sampling the free space instead of always
sleeping for the whole timeout, and give up once it has not increased for
a second.  A substream that is still making progress is given as long as
it needs, within the same overall 10 second limit as before, and each
wait is clipped to the remaining time so that limit is not overshot.

The warning is rate limited because a stalled drain is now detected much
more often than once per 10 seconds.

Closing one such device in qemu, with the syzkaller reproducer supplying
the gadget, took 72-103 seconds before and 9-11 seconds after.

Reported-by: syzbot+825b7e3a03dd072c187f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=825b7e3a03dd072c187f
Signed-off-by: Akhil Arul <akhilarul324@gmail.com>
---
Went with the adaptive one in the end.

I tried the fixed cap first and couldn't make it stand up. buffer_size is
PAGE_SIZE, so 16K or 64K on arm64, and PARAMS goes to 1MiB, so any constant
I pick is really a guess about the buffer. Test device consuming at the
MIDI-1 wire rate:

  buffer    unpatched      2*HZ cap        adaptive
   4096     ok 1.42s       ok 1.44s        ok 1.43s
  16384     ok 5.60s       -EIO 2.06s      ok 5.63s
  65536     -EIO 10.06s    -EIO 2.06s      -EIO 10.06s

The middle row is a healthy device at full rate failing because the kernel
has 16K pages. That killed it for me.

The adaptive version keeps the 10s ceiling and only bails out when avail
hasn't moved for a second, so those rows stay as they are. It also happens
to fix the hang better than the cap did. Closing one /dev/sequencer2 with
the syzkaller gadget attached, three runs each:

  unpatched   72.0s   102.7s   102.3s
  adaptive    10.7s     8.7s    11.6s
  2*HZ cap    18.4s    12.4s    16.5s

The teardown still calls the drain the same ~31 times either way, 9 to 15
of which time out; only the cost of each timeout moves. Hung task detector
at the default 120s stays quiet over three runs of about four minutes, with
the repro plus two threads opening /dev/sequencer2.

The cost is that a substream which goes quiet for a second with data still
queued now gets -EIO. Same device, pausing once and then resuming:

  pause    unpatched     patched
   950ms   ok 3.53s      ok 3.53s
  1050ms   ok 3.83s      ok 3.84s
  1200ms   ok 4.28s      -EIO 1.30s
  2000ms   ok 6.68s      -EIO 1.31s

So it's 1.05-1.2s rather than exactly a second. The poll grid is 200ms and
the cutoff drifts depending on where the pause lands against it.

I said 400ms in my last mail. That turned out to cut off a device pausing
for half a second, which a real one might do, so I widened it to a second.

Worth saying that avail isn't monotonic here - drain doesn't gate writers,
it only forces the wakeup in snd_rawmidi_transmit_ack(). That's why the
test is whether avail increased rather than whether it reached buffer_size.
I checked a shared append substream with a second writer refilling it, in
case that read as a stall. It doesn't: avail jitters instead of sitting
flat, the counter keeps resetting and the drain runs the full deadline.
With the refill matched to the consumption rate, unpatched gives -EIO at
10.07s and patched at 10.06s.

I couldn't find a way to bound the teardown without putting a cutoff
somewhere. Whether a second of silence is enough to call a device stopped
is your call.

No Fixes: tag, the 10s wait predates git.

 sound/core/rawmidi.c | 67 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 58 insertions(+), 9 deletions(-)

diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c
index 2617bb5b4..3df9366d7 100644
--- a/sound/core/rawmidi.c
+++ b/sound/core/rawmidi.c
@@ -36,6 +36,13 @@ module_param_array(amidi_map, int, NULL, 0444);
 MODULE_PARM_DESC(amidi_map, "Raw MIDI device number assigned to 2nd OSS device.");
 #endif /* CONFIG_SND_OSSEMUL */
 
+/* upper bound for draining the output buffer */
+#define SNDRV_RAWMIDI_DRAIN_TIMEOUT	(10 * HZ)
+/* interval at which drain progress is re-checked */
+#define SNDRV_RAWMIDI_DRAIN_POLL	(HZ / 5)
+/* polls without progress before the drain is considered stalled */
+#define SNDRV_RAWMIDI_DRAIN_STALLS	5
+
 static int snd_rawmidi_dev_free(struct snd_device *device);
 static int snd_rawmidi_dev_register(struct snd_device *device);
 static int snd_rawmidi_dev_disconnect(struct snd_device *device);
@@ -246,11 +253,20 @@ int snd_rawmidi_drop_output(struct snd_rawmidi_substream *substream)
 }
 EXPORT_SYMBOL(snd_rawmidi_drop_output);
 
+static bool output_drained(struct snd_rawmidi_runtime *runtime)
+{
+	return runtime->avail >= runtime->buffer_size;
+}
+
 int snd_rawmidi_drain_output(struct snd_rawmidi_substream *substream)
 {
-	int err = 0;
-	long timeout;
 	struct snd_rawmidi_runtime *runtime;
+	size_t avail, prev_avail;
+	unsigned int stalls = 0;
+	unsigned long deadline;
+	long timeout, wait;
+	bool done;
+	int err = 0;
 
 	scoped_guard(spinlock_irq, &substream->lock) {
 		runtime = substream->runtime;
@@ -258,19 +274,52 @@ int snd_rawmidi_drain_output(struct snd_rawmidi_substream *substream)
 			return -EINVAL;
 		snd_rawmidi_buffer_ref(runtime);
 		runtime->drain = 1;
+		prev_avail = runtime->avail;
+	}
+
+	/*
+	 * Wait for the device to consume the buffer.  Rather than always
+	 * sleeping for the whole timeout, sample the free space and stop
+	 * early once it has not increased for a second.  A substream that
+	 * is still making progress is given as long as it needs, within the
+	 * same overall limit as before.
+	 */
+	deadline = jiffies + SNDRV_RAWMIDI_DRAIN_TIMEOUT;
+	for (;;) {
+		/* signed difference, so this is safe across a jiffies wrap */
+		wait = (long)(deadline - jiffies);
+		if (wait <= 0) {
+			timeout = 0;
+			break;
+		}
+		wait = min_t(long, SNDRV_RAWMIDI_DRAIN_POLL, wait);
+		timeout = wait_event_interruptible_timeout(runtime->sleep,
+							   output_drained(runtime), wait);
+		scoped_guard(spinlock_irq, &substream->lock) {
+			avail = runtime->avail;
+			done = output_drained(runtime);
+		}
+		if (done || signal_pending(current))
+			break;
+		if (avail == prev_avail) {
+			if (++stalls >= SNDRV_RAWMIDI_DRAIN_STALLS) {
+				timeout = 0;
+				break;
+			}
+		} else {
+			stalls = 0;
+			prev_avail = avail;
+		}
 	}
 
-	timeout = wait_event_interruptible_timeout(runtime->sleep,
-				(runtime->avail >= runtime->buffer_size),
-				10*HZ);
-
 	scoped_guard(spinlock_irq, &substream->lock) {
 		if (signal_pending(current))
 			err = -ERESTARTSYS;
 		if (runtime->avail < runtime->buffer_size && !timeout) {
-			rmidi_warn(substream->rmidi,
-				   "rawmidi drain error (avail = %li, buffer_size = %li)\n",
-				   (long)runtime->avail, (long)runtime->buffer_size);
+			dev_warn_ratelimited(substream->rmidi->dev,
+					     "rawmidi drain error (avail = %li, buffer_size = %li)\n",
+					     (long)runtime->avail,
+					     (long)runtime->buffer_size);
 			err = -EIO;
 		}
 		runtime->drain = 0;
-- 
2.55.0