ALSA: pcm: Fix potential AB/BA lock with buffer_mutex and mmap_lock

commit bc55cfd5718c7c23e5524582e9fa70b4d10f2433 upstream.

syzbot caught a potential deadlock between the PCM
runtime->buffer_mutex and the mm->mmap_lock.  It was brought by the
recent fix to cover the racy read/write and other ioctls, and in that
commit, I overlooked a (hopefully only) corner case that may take the
revert lock, namely, the OSS mmap.  The OSS mmap operation
exceptionally allows to re-configure the parameters inside the OSS
mmap syscall, where mm->mmap_mutex is already held.  Meanwhile, the
copy_from/to_user calls at read/write operations also take the
mm->mmap_lock internally, hence it may lead to a AB/BA deadlock.

A similar problem was already seen in the past and we fixed it with a
refcount (in commit b248371628).  The former fix covered only the
call paths with OSS read/write and OSS ioctls, while we need to cover
the concurrent access via both ALSA and OSS APIs now.

This patch addresses the problem above by replacing the buffer_mutex
lock in the read/write operations with a refcount similar as we've
used for OSS.  The new field, runtime->buffer_accessing, keeps the
number of concurrent read/write operations.  Unlike the former
buffer_mutex protection, this protects only around the
copy_from/to_user() calls; the other codes are basically protected by
the PCM stream lock.  The refcount can be a negative, meaning blocked
by the ioctls.  If a negative value is seen, the read/write aborts
with -EBUSY.  In the ioctl side, OTOH, they check this refcount, too,
and set to a negative value for blocking unless it's already being
accessed.

Reported-by: [email protected]
Fixes: dca947d4d26d ("ALSA: pcm: Fix races among concurrent read/write and buffer changes")
Cc: <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Iwai <[email protected]>
[OP: backport to 5.4: adjusted context]
Signed-off-by: Ovidiu Panait <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
This commit is contained in:
Takashi Iwai 2022-05-06 12:10:13 +03:00 • committed by Greg Kroah-Hartman
commit 9661bf674d
4 changed files with 39 additions and 11 deletions

View file

@ -396,6 +396,7 @@ struct snd_pcm_runtime {
wait_queue_head_t tsleep; /* transfer sleep */
struct fasync_struct *fasync;
struct mutex buffer_mutex; /* protect for buffer changes */
atomic_t buffer_accessing; /* >0: in r/w operation, <0: blocked */
/* -- private section -- */
void *private_data;