On 01. 08. 22 12:13, Takashi Iwai wrote:
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.
It's really a negligible optimization. It would be better to not block other
CPUs here to allow insertion of the new event. Also the __queue_work() is a
bit complex code, so the call outside the spin lock may be better.
But either code is acceptable for me.
Jaroslav
--
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
