ALSA: core: Fix potential data race at fasync handling
[ Upstream commit 8146cd333d235ed32d48bb803fdf743472d7c783 ] In snd_fasync_work_fn(), which is the offload work for traversing and processing the pending fasync list, the call of kill_fasync() is done outside the snd_fasync_lock for avoiding deadlocks. The problem is that its the references of fasync->on, fasync->signal and fasync->poll are done there also outside the lock. Since these may be modified by snd_kill_fasync() call concurrently from other process, inconsistent values might be passed to kill_fasync(). Although there shouldn't be critical UAF, it's still better to be addressed. This patch moves the kill_fasync() argument evaluations inside the snd_fasync_lock for avoiding the data races above. The handling in fasync->on flag is optimized in the loop to skip directly. Also, for more clarity, snd_fasync_free() takes the lock and unlink the pending entry more directly instead of clearing fasync->on flag. Reported-by: Jake Lamberson <lamberson.jake@gmail.com> Fixes: ef34a0ae7a26 ("ALSA: core: Add async signal helpers") Cc: <stable@vger.kernel.org> Link: https://patch.msgid.link/20260420061721.3253644-1-tiwai@suse.de Signed-off-by: Takashi Iwai <tiwai@suse.de> [ replaced scoped_guard(spinlock_irq, &snd_fasync_lock) with explicit spin_lock_irq()/spin_unlock_irq() ] Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Ulrich Hecht <uli@kernel.org>
This commit is contained in:
1 parent
c4f5a473c9
commit
4be0306847
1 file changed
+11
-3
+11
-3
@@ -186,14 +186,18 @@ static LIST_HEAD(snd_fasync_list);
|
||||
static void snd_fasync_work_fn(struct work_struct *work)
|
||||
{
|
||||
struct snd_fasync *fasync;
|
||||
int signal, poll;
|
||||
|
||||
spin_lock_irq(&snd_fasync_lock);
|
||||
while (!list_empty(&snd_fasync_list)) {
|
||||
fasync = list_first_entry(&snd_fasync_list, struct snd_fasync, list);
|
||||
list_del_init(&fasync->list);
|
||||
if (!fasync->on)
|
||||
continue;
|
||||
signal = fasync->signal;
|
||||
poll = fasync->poll;
|
||||
spin_unlock_irq(&snd_fasync_lock);
|
||||
if (fasync->on)
|
||||
kill_fasync(&fasync->fasync, fasync->signal, fasync->poll);
|
||||
kill_fasync(&fasync->fasync, signal, poll);
|
||||
spin_lock_irq(&snd_fasync_lock);
|
||||
}
|
||||
spin_unlock_irq(&snd_fasync_lock);
|
||||
@@ -249,7 +253,11 @@ void snd_fasync_free(struct snd_fasync *fasync)
|
||||
{
|
||||
if (!fasync)
|
||||
return;
|
||||
fasync->on = 0;
|
||||
|
||||
spin_lock_irq(&snd_fasync_lock);
|
||||
list_del_init(&fasync->list);
|
||||
spin_unlock_irq(&snd_fasync_lock);
|
||||
|
||||
flush_work(&snd_fasync_work);
|
||||
kfree(fasync);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user