On Mon, 01 Aug 2022 10:05:59 +0200,
Jaroslav Kysela wrote:
>
> On 28. 07. 22 14:59, Takashi Iwai wrote:
> > Currently the call of kill_fasync() from an interrupt handler might
> > lead to potential spin deadlocks, as spotted by syzkaller.
> > Unfortunately, it's not so trivial to fix this lock chain as it's
> > involved with the tasklist_lock that is touched in allover places.
> >
> > As a temporary workaround, this patch provides the way to defer the
> > async signal notification in a work. The new helper functions,
> > snd_fasync_helper() and snd_kill_faync() are replacements for
> > fasync_helper() and kill_fasync(), respectively. In addition,
> > snd_fasync_free() needs to be called at the destructor of the relevant
> > file object.
> >
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
>
> ...
>
> > +void snd_kill_fasync(struct snd_fasync *fasync, int signal, int poll)
> > +{
> > + unsigned long flags;
> > +
> > + if (!fasync || !fasync->on)
> > + return;
> > + spin_lock_irqsave(&snd_fasync_lock, flags);
> > + fasync->signal = signal;
> > + fasync->poll = poll;
> > + list_move(&fasync->list, &snd_fasync_list);
> > + schedule_work(&snd_fasync_work);
> > + spin_unlock_irqrestore(&snd_fasync_lock, flags);
> > +}
>
> The schedule_work() may be called outside the spinlock - it calls
> queue_work_on() / __queue_work() which has already own protection for
> the concurrent execution.
It can be outside, too, but scheduling earlier reduces the possible
unnecessary scheduling. Suppose that a list is added while the work
is already running in another CPU. If we call schedule_work() outside
this lock, it might be already the time after the work has processed
the queued item, and hence it can be a superfluous scheduling call.
thanks,
Takashi
