On Tue, Mar 1, 2016 at 6:43 PM, Takashi Iwai <tiwai@xxxxxxx> wrote: > On Tue, 01 Mar 2016 16:45:22 +0100, > Dmitry Vyukov wrote: >> >> On Tue, Mar 1, 2016 at 4:31 PM, Takashi Iwai <tiwai@xxxxxxx> wrote: >> > On Tue, 01 Mar 2016 16:04:43 +0100, >> > Dmitry Vyukov wrote: >> >> >> >> On Tue, Mar 1, 2016 at 4:01 PM, Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote: >> >> > On Tue, Mar 1, 2016 at 3:44 PM, Takashi Iwai <tiwai@xxxxxxx> wrote: >> >> >> On Tue, 01 Mar 2016 13:33:27 +0100, >> >> >> Dmitry Vyukov wrote: >> >> >>> >> >> >>> Hello, >> >> >>> >> >> >>> The following program creates an unkillable process: >> >> >> ..... >> >> >>> The hang stack is: >> >> >>> >> >> >>> [<ffffffff85309f77>] snd_seq_oss_writeq_sync+0x327/0x790 >> >> >>> sound/core/seq/oss/seq_oss_writeq.c:121 >> >> >> >> >> >> This is >> >> >> wait_event_interruptible_timeout(q->sync_sleep, ! q->sync_event_put, HZ); >> >> >> >> >> >> and this should return zero >> >> >> if (signal_pending(current)) >> >> >> /* interrupted - return 0 to finish sync */ >> >> >> q->sync_event_put = 0; >> >> >> if (! q->sync_event_put || q->sync_time >= time) >> >> >> return 0; >> >> >> return 1; >> >> >> >> >> >>> [<ffffffff852fa353>] snd_seq_oss_drain_write+0x113/0x160 >> >> >> >> >> >> ... and this loop should break: >> >> >> while (snd_seq_oss_writeq_sync(dp->writeq)) >> >> >> ; >> >> >> >> >> >> So, I see no obvious error in the code, so far. >> >> >> >> >> >> I'm running your test program now with 8 parallel runs, but I couldn't >> >> >> reproduce it. Any other specifics? >> >> >> >> >> >> Hummm.. for me this process hangs every time, even if I just run it >> >> once from console. >> >> I've now retested in on clean commit >> >> fc77dbd34c5c99bce46d40a2491937c3bcbd10af (without KASAN/KCOV and with >> >> release gcc) and also got the same hang. >> >> I can only think of a different config. I've attached mine, please try with it. >> >> >> >> Are signals delivered if we are already in process of dying? >> >> When I kill -9 this process it does not react, so presumably wait is >> >> not unblocked... >> > >> > Good point. >> > >> > But above all, it doesn't make much sense to loop this sync call. >> > When wait_event*() returns, it should go out. So, the patch like >> > below should be good enough and fix your issue, too. >> >> >> >> The patch fixes the hang for me. There is a second delay at exit, but >> otherwise the process exits fine. >> >> Tested-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx> >> >> Thanks for quick fix! > > OK, I got the same result finally on my VM, too. I had to disable > INPUT_PCSPKR that conflicts with SND_PCSP. > > I reconsidered about this problem again, and concluded that the > currently implemented drain behavior is simply superfluous. So, the > fix is just to remove all these relevant calls. The patch is attached > below. Try this one instead of the previous one. Replaced the previous fix with this one. > -- 8< -- > From: Takashi Iwai <tiwai@xxxxxxx> > Subject: [PATCH] ALSA: seq: oss: Don't drain at closing a client > > The OSS sequencer client tries to drain the pending events at > releasing. Unfortunately, as spotted by syzkaller fuzzer, this may > lead to an unkillable process state when the event has been queued at > the far future. Since the process being released can't be signaled > any longer, it remains and waits for the echo-back event in that far > future. > > Back to history, the draining feature was implemented at the time we > misinterpreted POSIX definition for blocking file operation. > Actually, such a behavior is superfluous at release, and we should > just release the device as is instead of keeping it up forever. > > This patch just removes the draining call that may block the release > for too long time unexpectedly. > > BugLink: http://lkml.kernel.org/r/CACT4Y+Y4kD-aBGj37rf-xBw9bH3GMU6P+MYg4W1e-s-paVD2pg@xxxxxxxxxxxxxx > Reported-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> > --- > sound/core/seq/oss/seq_oss.c | 2 -- > sound/core/seq/oss/seq_oss_device.h | 1 - > sound/core/seq/oss/seq_oss_init.c | 16 ---------------- > 3 files changed, 19 deletions(-) > > diff --git a/sound/core/seq/oss/seq_oss.c b/sound/core/seq/oss/seq_oss.c > index 8db156b207f1..8cdf489df80e 100644 > --- a/sound/core/seq/oss/seq_oss.c > +++ b/sound/core/seq/oss/seq_oss.c > @@ -149,8 +149,6 @@ odev_release(struct inode *inode, struct file *file) > if ((dp = file->private_data) == NULL) > return 0; > > - snd_seq_oss_drain_write(dp); > - > mutex_lock(®ister_mutex); > snd_seq_oss_release(dp); > mutex_unlock(®ister_mutex); > diff --git a/sound/core/seq/oss/seq_oss_device.h b/sound/core/seq/oss/seq_oss_device.h > index b43924325249..d7b4d016b547 100644 > --- a/sound/core/seq/oss/seq_oss_device.h > +++ b/sound/core/seq/oss/seq_oss_device.h > @@ -127,7 +127,6 @@ int snd_seq_oss_write(struct seq_oss_devinfo *dp, const char __user *buf, int co > unsigned int snd_seq_oss_poll(struct seq_oss_devinfo *dp, struct file *file, poll_table * wait); > > void snd_seq_oss_reset(struct seq_oss_devinfo *dp); > -void snd_seq_oss_drain_write(struct seq_oss_devinfo *dp); > > /* */ > void snd_seq_oss_process_queue(struct seq_oss_devinfo *dp, abstime_t time); > diff --git a/sound/core/seq/oss/seq_oss_init.c b/sound/core/seq/oss/seq_oss_init.c > index 6779e82b46dd..92c96a95a903 100644 > --- a/sound/core/seq/oss/seq_oss_init.c > +++ b/sound/core/seq/oss/seq_oss_init.c > @@ -436,22 +436,6 @@ snd_seq_oss_release(struct seq_oss_devinfo *dp) > > > /* > - * Wait until the queue is empty (if we don't have nonblock) > - */ > -void > -snd_seq_oss_drain_write(struct seq_oss_devinfo *dp) > -{ > - if (! dp->timer->running) > - return; > - if (is_write_mode(dp->file_mode) && !is_nonblock_mode(dp->file_mode) && > - dp->writeq) { > - while (snd_seq_oss_writeq_sync(dp->writeq)) > - ; > - } > -} > - > - > -/* > * reset sequencer devices > */ > void > -- > 2.7.2 > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
