From 293385739d687be1b6f791bef03956a14214656d Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Fri, 24 Sep 2021 14:24:16 -0500 Subject: ALSA: hda: hdac_ext_stream: fix potential locking issues commit 868ddfcef31ff93ea8961b2e81ea7fe12f6f144b upstream. The code for hdac_ext_stream seems inherited from hdac_stream, and similar locking issues are present: the use of the bus->reg_lock spinlock is inconsistent, with only writes to specific fields being protected. Apply similar fix as in hdac_stream by protecting all accesses to 'link_locked' and 'decoupled' fields, with a new helper snd_hdac_ext_stream_decouple_locked() added to simplify code changes. Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20210924192417.169243-4-pierre-louis.bossart@linux.intel.com Signed-off-by: Takashi Iwai Signed-off-by: Greg Kroah-Hartman --- include/sound/hdaudio_ext.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include/sound') diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index ef88b20c7b0a..23dc8deac344 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -88,6 +88,8 @@ struct hdac_ext_stream *snd_hdac_ext_stream_assign(struct hdac_bus *bus, struct snd_pcm_substream *substream, int type); void snd_hdac_ext_stream_release(struct hdac_ext_stream *azx_dev, int type); +void snd_hdac_ext_stream_decouple_locked(struct hdac_bus *bus, + struct hdac_ext_stream *azx_dev, bool decouple); void snd_hdac_ext_stream_decouple(struct hdac_bus *bus, struct hdac_ext_stream *azx_dev, bool decouple); void snd_hdac_ext_stop_streams(struct hdac_bus *bus); -- cgit v1.2.3 From fbeb492694ce0441053de57699e1e2b7bc148a69 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Fri, 6 May 2022 12:10:09 +0300 Subject: ALSA: pcm: Fix races among concurrent hw_params and hw_free calls commit 92ee3c60ec9fe64404dc035e7c41277d74aa26cb upstream. Currently we have neither proper check nor protection against the concurrent calls of PCM hw_params and hw_free ioctls, which may result in a UAF. Since the existing PCM stream lock can't be used for protecting the whole ioctl operations, we need a new mutex to protect those racy calls. This patch introduced a new mutex, runtime->buffer_mutex, and applies it to both hw_params and hw_free ioctl code paths. Along with it, the both functions are slightly modified (the mmap_count check is moved into the state-check block) for code simplicity. Reported-by: Hu Jiahui Cc: Reviewed-by: Jaroslav Kysela Link: https://lore.kernel.org/r/20220322170720.3529-2-tiwai@suse.de Signed-off-by: Takashi Iwai [OP: backport to 5.4: adjusted context] Signed-off-by: Ovidiu Panait Signed-off-by: Greg Kroah-Hartman --- include/sound/pcm.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include/sound') diff --git a/include/sound/pcm.h b/include/sound/pcm.h index bbe6eb1ff5d2..24273d0f770b 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -395,6 +395,7 @@ struct snd_pcm_runtime { wait_queue_head_t sleep; /* poll sleep */ wait_queue_head_t tsleep; /* transfer sleep */ struct fasync_struct *fasync; + struct mutex buffer_mutex; /* protect for buffer changes */ /* -- private section -- */ void *private_data; -- cgit v1.2.3 From 9661bf674d6a82b76e4ae424438a8ce1e3ed855d Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Fri, 6 May 2022 12:10:13 +0300 Subject: 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 b248371628aa). 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: syzbot+6e5c88838328e99c7e1c@syzkaller.appspotmail.com Fixes: dca947d4d26d ("ALSA: pcm: Fix races among concurrent read/write and buffer changes") Cc: Link: https://lore.kernel.org/r/000000000000381a0d05db622a81@google.com Link: https://lore.kernel.org/r/20220330120903.4738-1-tiwai@suse.de Signed-off-by: Takashi Iwai [OP: backport to 5.4: adjusted context] Signed-off-by: Ovidiu Panait Signed-off-by: Greg Kroah-Hartman --- include/sound/pcm.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include/sound') diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 24273d0f770b..f0045f842a60 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -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; -- cgit v1.2.3