[PATCH 5.4 0/5] ALSA: pcm: backports for CVE-2022-1048

[PATCH 5.4 0/5] ALSA: pcm: backports for CVE-2022-1048

[Ovidiu Panait]

Contextual adjustments were made to apply to 5.4 stable tree.

Testing
-------
Running the PoC from [1] on 5.4.191 kernel produces the following oops:

qemu-system-x86_64 -nographic -serial mon:stdio -serial null -enable-kvm \
-net user,hostname=qemu0,hostfwd=tcp::36074-:22 -net nic \
-drive file=rootfs.ext4,format=raw -cpu host -m 4096 -kernel bzImage \
-append "console=ttyS0,115200 root=/dev/sda rw ip=dhcp " -soundhw ac97 -smp 2
root@intel-x86-64:~# ./poc
...
[   95.839647] BUG: Bad page state in process poc  pfn:bb860
[   95.841277] page:ffffea0002ee1800 refcount:-1 mapcount:0 mapping:0000000000000000 index:0x0
[   95.843521] flags: 0x100000000000000()
[   95.844539] raw: 0100000000000000 dead000000000100 dead000000000122 0000000000000000
[   95.846306] raw: 0000000000000000 0000000000000000 ffffffffffffffff 0000000000000000
[   95.847164] page dumped because: nonzero _refcount
[   95.847705] Modules linked in:
[   95.848063] CPU: 0 PID: 357 Comm: poc Tainted: G        W         5.4.191 #6
[   95.848839] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
[   95.849847] Call Trace:
[   95.850145]  dump_stack+0x76/0x9c
[   95.850549]  bad_page.cold+0xff/0x124
[   95.850980]  ? si_mem_available+0x2f0/0x2f0
[   95.851464]  ? _raw_spin_trylock_bh+0x120/0x120
[   95.851988]  ? __module_text_address+0xe/0x140
[   95.852494]  get_page_from_freelist+0x16f9/0x35b0
[   95.853034]  ? __isolate_free_page+0x460/0x460
[   95.853543]  ? save_stack+0x4c/0x80
[   95.853938]  ? save_stack+0x1b/0x80
[   95.854343]  ? __kasan_kmalloc.constprop.0+0xc2/0xd0
[   95.854897]  ? snd_pcm_lib_malloc_pages+0x2b8/0x680
[   95.855433]  ? snd_intel8x0_hw_params+0x106/0x550
[   95.855964]  ? snd_pcm_hw_params+0x2b5/0x1290
[   95.856438]  ? snd_pcm_common_ioctl+0x332/0x1a20
[   95.856954]  __alloc_pages_nodemask+0x274/0x610
[   95.857460]  ? __alloc_pages_slowpath+0x1ff0/0x1ff0
[   95.857992]  ? snd_pcm_hw_refine+0x8de/0xdd0
[   95.858467]  ? kfree+0x8c/0x230
[   95.858823]  __dma_direct_alloc_pages+0x18d/0x390
[   95.859339]  dma_direct_alloc_pages+0x1b/0x170
[   95.859827]  snd_dma_alloc_pages+0x1ae/0x380
[   95.860294]  snd_pcm_lib_malloc_pages+0x371/0x680
[   95.860812]  snd_intel8x0_hw_params+0x106/0x550
[   95.861311]  snd_pcm_hw_params+0x2b5/0x1290
[   95.861780]  ? _copy_from_user+0x70/0xa0
[   95.862214]  snd_pcm_common_ioctl+0x332/0x1a20
[   95.862699]  ? up_read+0x10/0x90
[   95.863070]  ? n_tty_write+0x7ba/0xf70
[   95.863484]  ? snd_pcm_status_user+0x120/0x120
[   95.863974]  ? _raw_spin_lock_irqsave+0x7b/0xd0
[   95.864473]  ? _raw_spin_trylock_bh+0x120/0x120
[   95.864975]  snd_pcm_ioctl+0x62/0xa0
[   95.865382]  do_vfs_ioctl+0x9af/0xf30
[   95.865790]  ? selinux_file_ioctl+0x3ca/0x530
[   95.866271]  ? ioctl_preallocate+0x1a0/0x1a0
[   95.866739]  ? selinux_capable+0x20/0x20
[   95.867172]  ? __fget_light+0xab/0x4c0
[   95.867588]  ? syscall_trace_enter+0x50e/0xb40
[   95.868074]  ? iterate_fd+0x180/0x180
[   95.868478]  ksys_ioctl+0x59/0x90
[   95.868853]  __x64_sys_ioctl+0x6a/0xb0
[   95.869278]  do_syscall_64+0x89/0x2e0
[   95.869681]  ? prepare_exit_to_usermode+0xec/0x190
[   95.870213]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   95.870764] RIP: 0033:0x7f6f375c8717
[   95.871157] Code: 00 00 90 48 8b 05 69 57 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 8
[   95.873187] RSP: 002b:00007ffdbdb71b48 EFLAGS: 00000206 ORIG_RAX: 0000000000000010
[   95.874009] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f6f375c8717
[   95.874780] RDX: 0000564d6f23c2a0 RSI: 00000000c2604111 RDI: 0000000000000003
[   95.875555] RBP: 00007ffdbdb71c20 R08: 0000000000000000 R09: 0000000000000010
[   95.876322] R10: 00007ffdbdb71a27 R11: 0000000000000206 R12: 0000564d6f15e120
[   95.877093] R13: 00007ffdbdb71d00 R14: 0000000000000000 R15: 0000000000000000
[   95.877864] Disabling lock debugging due to kernel taint
[   95.881630] ==================================================================
[   95.883522] BUG: KASAN: double-free or invalid-free in snd_pcm_lib_free_pages+0xe1/0x230
[   95.885570] 
[   95.885976] CPU: 1 PID: 371 Comm: poc Tainted: G    B   W         5.4.191 #6
[   95.887787] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
[   95.890095] Call Trace:
[   95.890505]  dump_stack+0x76/0x9c
[   95.890859]  print_address_description.constprop.0+0x16/0x200
[   95.891454]  ? snd_pcm_lib_free_pages+0xe1/0x230
[   95.891940]  kasan_report_invalid_free+0x61/0xa0
[   95.892429]  ? snd_pcm_lib_free_pages+0xe1/0x230
[   95.892921]  __kasan_slab_free+0x15e/0x170
[   95.893350]  ? snd_pcm_lib_free_pages+0xe1/0x230
[   95.893843]  kfree+0x8c/0x230
[   95.894163]  snd_pcm_lib_free_pages+0xe1/0x230
[   95.894633]  snd_pcm_common_ioctl+0x599/0x1a20
[   95.895089]  ? snd_pcm_status_user+0x120/0x120
[   95.895543]  snd_pcm_ioctl+0x62/0xa0
[   95.895912]  do_vfs_ioctl+0x9af/0xf30
[   95.896292]  ? selinux_file_ioctl+0x3ca/0x530
[   95.896752]  ? ioctl_preallocate+0x1a0/0x1a0
[   95.897184]  ? selinux_capable+0x20/0x20
[   95.897589]  ? __fget_light+0x2ab/0x4c0
[   95.898002]  ? iterate_fd+0x180/0x180
[   95.898385]  ksys_ioctl+0x59/0x90
[   95.898739]  __x64_sys_ioctl+0x6a/0xb0
[   95.899139]  do_syscall_64+0x89/0x2e0
[   95.899521]  ? syscall_return_slowpath+0x17a/0x1e0
[   95.900013]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   95.900532] RIP: 0033:0x7f6f375c8717
[   95.900905] Code: 00 00 90 48 8b 05 69 57 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 8
[   95.902809] RSP: 002b:00007f6f30b72ee8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[   95.903572] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f6f375c8717
[   95.904294] RDX: 0000000000000000 RSI: 0000000000004112 RDI: 0000000000000003
[   95.905009] RBP: 00007f6f30b72f00 R08: 00007f6f30b73700 R09: 00007f6f30b73700
[   95.905723] R10: 00007f6f30b739d0 R11: 0000000000000246 R12: 00007ffdbdb71ace
[   95.906442] R13: 00007ffdbdb71acf R14: 00007f6f30b72fc0 R15: 00007f6f30b73700


The testcase runs successfully after applying this patchset.

[1] https://www.openwall.com/lists/oss-security/2022/03/28/4


Takashi Iwai (5):
  ALSA: pcm: Fix races among concurrent hw_params and hw_free calls
  ALSA: pcm: Fix races among concurrent read/write and buffer changes
  ALSA: pcm: Fix races among concurrent prepare and hw_params/hw_free
    calls
  ALSA: pcm: Fix races among concurrent prealloc proc writes
  ALSA: pcm: Fix potential AB/BA lock with buffer_mutex and mmap_lock

 include/sound/pcm.h     |   2 +
 sound/core/pcm.c        |   3 ++
 sound/core/pcm_lib.c    |   5 ++
 sound/core/pcm_memory.c |  11 ++--
 sound/core/pcm_native.c | 110 ++++++++++++++++++++++++++++------------
 5 files changed, 95 insertions(+), 36 deletions(-)

-- 
2.36.0

[PATCH 5.4 1/5] ALSA: pcm: Fix races among concurrent hw_params and hw_free calls

[Ovidiu Panait]

From: Takashi Iwai <tiwai@suse.de>

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 <kirin.say@gmail.com>
Cc: <stable@vger.kernel.org>
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Link: https://lore.kernel.org/r/20220322170720.3529-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
[OP: backport to 5.4: adjusted context]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
 include/sound/pcm.h     |  1 +
 sound/core/pcm.c        |  2 ++
 sound/core/pcm_native.c | 55 +++++++++++++++++++++++++++--------------
 3 files changed, 39 insertions(+), 19 deletions(-)

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;
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index f8ce961c28d6..c9335d1d0e44 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -969,6 +969,7 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
 	init_waitqueue_head(&runtime->tsleep);
 
 	runtime->status->state = SNDRV_PCM_STATE_OPEN;
+	mutex_init(&runtime->buffer_mutex);
 
 	substream->runtime = runtime;
 	substream->private_data = pcm->private_data;
@@ -1000,6 +1001,7 @@ void snd_pcm_detach_substream(struct snd_pcm_substream *substream)
 	substream->runtime = NULL;
 	if (substream->timer)
 		spin_unlock_irq(&substream->timer->lock);
+	mutex_destroy(&runtime->buffer_mutex);
 	kfree(runtime);
 	put_pid(substream->pid);
 	substream->pid = NULL;
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index dbe9a65cc1d4..b15ef9df114a 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -630,33 +630,40 @@ static int snd_pcm_hw_params_choose(struct snd_pcm_substream *pcm,
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_SND_PCM_OSS)
+#define is_oss_stream(substream)	((substream)->oss.oss)
+#else
+#define is_oss_stream(substream)	false
+#endif
+
 static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_pcm_hw_params *params)
 {
 	struct snd_pcm_runtime *runtime;
-	int err, usecs;
+	int err = 0, usecs;
 	unsigned int bits;
 	snd_pcm_uframes_t frames;
 
 	if (PCM_RUNTIME_CHECK(substream))
 		return -ENXIO;
 	runtime = substream->runtime;
+	mutex_lock(&runtime->buffer_mutex);
 	snd_pcm_stream_lock_irq(substream);
 	switch (runtime->status->state) {
 	case SNDRV_PCM_STATE_OPEN:
 	case SNDRV_PCM_STATE_SETUP:
 	case SNDRV_PCM_STATE_PREPARED:
+		if (!is_oss_stream(substream) &&
+		    atomic_read(&substream->mmap_count))
+			err = -EBADFD;
 		break;
 	default:
-		snd_pcm_stream_unlock_irq(substream);
-		return -EBADFD;
+		err = -EBADFD;
+		break;
 	}
 	snd_pcm_stream_unlock_irq(substream);
-#if IS_ENABLED(CONFIG_SND_PCM_OSS)
-	if (!substream->oss.oss)
-#endif
-		if (atomic_read(&substream->mmap_count))
-			return -EBADFD;
+	if (err)
+		goto unlock;
 
 	params->rmask = ~0U;
 	err = snd_pcm_hw_refine(substream, params);
@@ -733,14 +740,19 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 	if ((usecs = period_to_usecs(runtime)) >= 0)
 		pm_qos_add_request(&substream->latency_pm_qos_req,
 				   PM_QOS_CPU_DMA_LATENCY, usecs);
-	return 0;
+	err = 0;
  _error:
-	/* hardware might be unusable from this time,
-	   so we force application to retry to set
-	   the correct hardware parameter settings */
-	snd_pcm_set_state(substream, SNDRV_PCM_STATE_OPEN);
-	if (substream->ops->hw_free != NULL)
-		substream->ops->hw_free(substream);
+	if (err) {
+		/* hardware might be unusable from this time,
+		 * so we force application to retry to set
+		 * the correct hardware parameter settings
+		 */
+		snd_pcm_set_state(substream, SNDRV_PCM_STATE_OPEN);
+		if (substream->ops->hw_free != NULL)
+			substream->ops->hw_free(substream);
+	}
+ unlock:
+	mutex_unlock(&runtime->buffer_mutex);
 	return err;
 }
 
@@ -773,22 +785,27 @@ static int snd_pcm_hw_free(struct snd_pcm_substream *substream)
 	if (PCM_RUNTIME_CHECK(substream))
 		return -ENXIO;
 	runtime = substream->runtime;
+	mutex_lock(&runtime->buffer_mutex);
 	snd_pcm_stream_lock_irq(substream);
 	switch (runtime->status->state) {
 	case SNDRV_PCM_STATE_SETUP:
 	case SNDRV_PCM_STATE_PREPARED:
+		if (atomic_read(&substream->mmap_count))
+			result = -EBADFD;
 		break;
 	default:
-		snd_pcm_stream_unlock_irq(substream);
-		return -EBADFD;
+		result = -EBADFD;
+		break;
 	}
 	snd_pcm_stream_unlock_irq(substream);
-	if (atomic_read(&substream->mmap_count))
-		return -EBADFD;
+	if (result)
+		goto unlock;
 	if (substream->ops->hw_free)
 		result = substream->ops->hw_free(substream);
 	snd_pcm_set_state(substream, SNDRV_PCM_STATE_OPEN);
 	pm_qos_remove_request(&substream->latency_pm_qos_req);
+ unlock:
+	mutex_unlock(&runtime->buffer_mutex);
 	return result;
 }
 
-- 
2.36.0

[PATCH 5.4 2/5] ALSA: pcm: Fix races among concurrent read/write and buffer changes

[Ovidiu Panait]

From: Takashi Iwai <tiwai@suse.de>

commit dca947d4d26dbf925a64a6cfb2ddbc035e831a3d upstream.

In the current PCM design, the read/write syscalls (as well as the
equivalent ioctls) are allowed before the PCM stream is running, that
is, at PCM PREPARED state.  Meanwhile, we also allow to re-issue
hw_params and hw_free ioctl calls at the PREPARED state that may
change or free the buffers, too.  The problem is that there is no
protection against those mix-ups.

This patch applies the previously introduced runtime->buffer_mutex to
the read/write operations so that the concurrent hw_params or hw_free
call can no longer interfere during the operation.  The mutex is
unlocked before scheduling, so we don't take it too long.

Cc: <stable@vger.kernel.org>
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Link: https://lore.kernel.org/r/20220322170720.3529-3-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
 sound/core/pcm_lib.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index fd300c3addde..fdb141e426ac 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -1861,9 +1861,11 @@ static int wait_for_avail(struct snd_pcm_substream *substream,
 		if (avail >= runtime->twake)
 			break;
 		snd_pcm_stream_unlock_irq(substream);
+		mutex_unlock(&runtime->buffer_mutex);
 
 		tout = schedule_timeout(wait_time);
 
+		mutex_lock(&runtime->buffer_mutex);
 		snd_pcm_stream_lock_irq(substream);
 		set_current_state(TASK_INTERRUPTIBLE);
 		switch (runtime->status->state) {
@@ -2157,6 +2159,7 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
 
 	nonblock = !!(substream->f_flags & O_NONBLOCK);
 
+	mutex_lock(&runtime->buffer_mutex);
 	snd_pcm_stream_lock_irq(substream);
 	err = pcm_accessible_state(runtime);
 	if (err < 0)
@@ -2244,6 +2247,7 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
 	if (xfer > 0 && err >= 0)
 		snd_pcm_update_state(substream, runtime);
 	snd_pcm_stream_unlock_irq(substream);
+	mutex_unlock(&runtime->buffer_mutex);
 	return xfer > 0 ? (snd_pcm_sframes_t)xfer : err;
 }
 EXPORT_SYMBOL(__snd_pcm_lib_xfer);
-- 
2.36.0

[PATCH 5.4 3/5] ALSA: pcm: Fix races among concurrent prepare and hw_params/hw_free calls

[Ovidiu Panait]

From: Takashi Iwai <tiwai@suse.de>

commit 3c3201f8c7bb77eb53b08a3ca8d9a4ddc500b4c0 upstream.

Like the previous fixes to hw_params and hw_free ioctl races, we need
to paper over the concurrent prepare ioctl calls against hw_params and
hw_free, too.

This patch implements the locking with the existing
runtime->buffer_mutex for prepare ioctls.  Unlike the previous case
for snd_pcm_hw_hw_params() and snd_pcm_hw_free(), snd_pcm_prepare() is
performed to the linked streams, hence the lock can't be applied
simply on the top.  For tracking the lock in each linked substream, we
modify snd_pcm_action_group() slightly and apply the buffer_mutex for
the case stream_lock=false (formerly there was no lock applied)
there.

Cc: <stable@vger.kernel.org>
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Link: https://lore.kernel.org/r/20220322170720.3529-4-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
[OP: backport to 5.4: adjusted context]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
 sound/core/pcm_native.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index b15ef9df114a..4f53e6103fd5 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1042,15 +1042,17 @@ struct action_ops {
  */
 static int snd_pcm_action_group(const struct action_ops *ops,
 				struct snd_pcm_substream *substream,
-				int state, int do_lock)
+				int state, int stream_lock)
 {
 	struct snd_pcm_substream *s = NULL;
 	struct snd_pcm_substream *s1;
 	int res = 0, depth = 1;
 
 	snd_pcm_group_for_each_entry(s, substream) {
-		if (do_lock && s != substream) {
-			if (s->pcm->nonatomic)
+		if (s != substream) {
+			if (!stream_lock)
+				mutex_lock_nested(&s->runtime->buffer_mutex, depth);
+			else if (s->pcm->nonatomic)
 				mutex_lock_nested(&s->self_group.mutex, depth);
 			else
 				spin_lock_nested(&s->self_group.lock, depth);
@@ -1078,18 +1080,18 @@ static int snd_pcm_action_group(const struct action_ops *ops,
 		ops->post_action(s, state);
 	}
  _unlock:
-	if (do_lock) {
-		/* unlock streams */
-		snd_pcm_group_for_each_entry(s1, substream) {
-			if (s1 != substream) {
-				if (s1->pcm->nonatomic)
-					mutex_unlock(&s1->self_group.mutex);
-				else
-					spin_unlock(&s1->self_group.lock);
-			}
-			if (s1 == s)	/* end */
-				break;
+	/* unlock streams */
+	snd_pcm_group_for_each_entry(s1, substream) {
+		if (s1 != substream) {
+			if (!stream_lock)
+				mutex_unlock(&s1->runtime->buffer_mutex);
+			else if (s1->pcm->nonatomic)
+				mutex_unlock(&s1->self_group.mutex);
+			else
+				spin_unlock(&s1->self_group.lock);
 		}
+		if (s1 == s)	/* end */
+			break;
 	}
 	return res;
 }
@@ -1219,10 +1221,12 @@ static int snd_pcm_action_nonatomic(const struct action_ops *ops,
 
 	/* Guarantee the group members won't change during non-atomic action */
 	down_read(&snd_pcm_link_rwsem);
+	mutex_lock(&substream->runtime->buffer_mutex);
 	if (snd_pcm_stream_linked(substream))
 		res = snd_pcm_action_group(ops, substream, state, 0);
 	else
 		res = snd_pcm_action_single(ops, substream, state);
+	mutex_unlock(&substream->runtime->buffer_mutex);
 	up_read(&snd_pcm_link_rwsem);
 	return res;
 }
-- 
2.36.0

[PATCH 5.4 4/5] ALSA: pcm: Fix races among concurrent prealloc proc writes

[Ovidiu Panait]

From: Takashi Iwai <tiwai@suse.de>

commit 69534c48ba8ce552ce383b3dfdb271ffe51820c3 upstream.

We have no protection against concurrent PCM buffer preallocation
changes via proc files, and it may potentially lead to UAF or some
weird problem.  This patch applies the PCM open_mutex to the proc
write operation for avoiding the racy proc writes and the PCM stream
open (and further operations).

Cc: <stable@vger.kernel.org>
Reviewed-by: Jaroslav Kysela <perex@perex.cz>
Link: https://lore.kernel.org/r/20220322170720.3529-5-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
[OP: backport to 5.4: adjusted context]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
 sound/core/pcm_memory.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c
index 7600dcdf5fd4..9aea1d6fb054 100644
--- a/sound/core/pcm_memory.c
+++ b/sound/core/pcm_memory.c
@@ -133,19 +133,20 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry,
 	size_t size;
 	struct snd_dma_buffer new_dmab;
 
+	mutex_lock(&substream->pcm->open_mutex);
 	if (substream->runtime) {
 		buffer->error = -EBUSY;
-		return;
+		goto unlock;
 	}
 	if (!snd_info_get_line(buffer, line, sizeof(line))) {
 		snd_info_get_str(str, line, sizeof(str));
 		size = simple_strtoul(str, NULL, 10) * 1024;
 		if ((size != 0 && size < 8192) || size > substream->dma_max) {
 			buffer->error = -EINVAL;
-			return;
+			goto unlock;
 		}
 		if (substream->dma_buffer.bytes == size)
-			return;
+			goto unlock;
 		memset(&new_dmab, 0, sizeof(new_dmab));
 		new_dmab.dev = substream->dma_buffer.dev;
 		if (size > 0) {
@@ -153,7 +154,7 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry,
 						substream->dma_buffer.dev.dev,
 						size, &new_dmab) < 0) {
 				buffer->error = -ENOMEM;
-				return;
+				goto unlock;
 			}
 			substream->buffer_bytes_max = size;
 		} else {
@@ -165,6 +166,8 @@ static void snd_pcm_lib_preallocate_proc_write(struct snd_info_entry *entry,
 	} else {
 		buffer->error = -EINVAL;
 	}
+ unlock:
+	mutex_unlock(&substream->pcm->open_mutex);
 }
 
 static inline void preallocate_info_init(struct snd_pcm_substream *substream)
-- 
2.36.0

[PATCH 5.4 5/5] ALSA: pcm: Fix potential AB/BA lock with buffer_mutex and mmap_lock

[Ovidiu Panait]

From: Takashi Iwai <tiwai@suse.de>

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: <stable@vger.kernel.org>
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 <tiwai@suse.de>
[OP: backport to 5.4: adjusted context]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
 include/sound/pcm.h     |  1 +
 sound/core/pcm.c        |  1 +
 sound/core/pcm_lib.c    |  9 +++++----
 sound/core/pcm_native.c | 39 ++++++++++++++++++++++++++++++++-------
 4 files changed, 39 insertions(+), 11 deletions(-)

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;
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index c9335d1d0e44..3561cdceaadc 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -970,6 +970,7 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
 
 	runtime->status->state = SNDRV_PCM_STATE_OPEN;
 	mutex_init(&runtime->buffer_mutex);
+	atomic_set(&runtime->buffer_accessing, 0);
 
 	substream->runtime = runtime;
 	substream->private_data = pcm->private_data;
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index fdb141e426ac..1bce55533519 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -1861,11 +1861,9 @@ static int wait_for_avail(struct snd_pcm_substream *substream,
 		if (avail >= runtime->twake)
 			break;
 		snd_pcm_stream_unlock_irq(substream);
-		mutex_unlock(&runtime->buffer_mutex);
 
 		tout = schedule_timeout(wait_time);
 
-		mutex_lock(&runtime->buffer_mutex);
 		snd_pcm_stream_lock_irq(substream);
 		set_current_state(TASK_INTERRUPTIBLE);
 		switch (runtime->status->state) {
@@ -2159,7 +2157,6 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
 
 	nonblock = !!(substream->f_flags & O_NONBLOCK);
 
-	mutex_lock(&runtime->buffer_mutex);
 	snd_pcm_stream_lock_irq(substream);
 	err = pcm_accessible_state(runtime);
 	if (err < 0)
@@ -2214,10 +2211,15 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
 			err = -EINVAL;
 			goto _end_unlock;
 		}
+		if (!atomic_inc_unless_negative(&runtime->buffer_accessing)) {
+			err = -EBUSY;
+			goto _end_unlock;
+		}
 		snd_pcm_stream_unlock_irq(substream);
 		err = writer(substream, appl_ofs, data, offset, frames,
 			     transfer);
 		snd_pcm_stream_lock_irq(substream);
+		atomic_dec(&runtime->buffer_accessing);
 		if (err < 0)
 			goto _end_unlock;
 		err = pcm_accessible_state(runtime);
@@ -2247,7 +2249,6 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream,
 	if (xfer > 0 && err >= 0)
 		snd_pcm_update_state(substream, runtime);
 	snd_pcm_stream_unlock_irq(substream);
-	mutex_unlock(&runtime->buffer_mutex);
 	return xfer > 0 ? (snd_pcm_sframes_t)xfer : err;
 }
 EXPORT_SYMBOL(__snd_pcm_lib_xfer);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 4f53e6103fd5..57a4991fa0f3 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -630,6 +630,24 @@ static int snd_pcm_hw_params_choose(struct snd_pcm_substream *pcm,
 	return 0;
 }
 
+/* acquire buffer_mutex; if it's in r/w operation, return -EBUSY, otherwise
+ * block the further r/w operations
+ */
+static int snd_pcm_buffer_access_lock(struct snd_pcm_runtime *runtime)
+{
+	if (!atomic_dec_unless_positive(&runtime->buffer_accessing))
+		return -EBUSY;
+	mutex_lock(&runtime->buffer_mutex);
+	return 0; /* keep buffer_mutex, unlocked by below */
+}
+
+/* release buffer_mutex and clear r/w access flag */
+static void snd_pcm_buffer_access_unlock(struct snd_pcm_runtime *runtime)
+{
+	mutex_unlock(&runtime->buffer_mutex);
+	atomic_inc(&runtime->buffer_accessing);
+}
+
 #if IS_ENABLED(CONFIG_SND_PCM_OSS)
 #define is_oss_stream(substream)	((substream)->oss.oss)
 #else
@@ -640,14 +658,16 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_pcm_hw_params *params)
 {
 	struct snd_pcm_runtime *runtime;
-	int err = 0, usecs;
+	int err, usecs;
 	unsigned int bits;
 	snd_pcm_uframes_t frames;
 
 	if (PCM_RUNTIME_CHECK(substream))
 		return -ENXIO;
 	runtime = substream->runtime;
-	mutex_lock(&runtime->buffer_mutex);
+	err = snd_pcm_buffer_access_lock(runtime);
+	if (err < 0)
+		return err;
 	snd_pcm_stream_lock_irq(substream);
 	switch (runtime->status->state) {
 	case SNDRV_PCM_STATE_OPEN:
@@ -752,7 +772,7 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 			substream->ops->hw_free(substream);
 	}
  unlock:
-	mutex_unlock(&runtime->buffer_mutex);
+	snd_pcm_buffer_access_unlock(runtime);
 	return err;
 }
 
@@ -785,7 +805,9 @@ static int snd_pcm_hw_free(struct snd_pcm_substream *substream)
 	if (PCM_RUNTIME_CHECK(substream))
 		return -ENXIO;
 	runtime = substream->runtime;
-	mutex_lock(&runtime->buffer_mutex);
+	result = snd_pcm_buffer_access_lock(runtime);
+	if (result < 0)
+		return result;
 	snd_pcm_stream_lock_irq(substream);
 	switch (runtime->status->state) {
 	case SNDRV_PCM_STATE_SETUP:
@@ -805,7 +827,7 @@ static int snd_pcm_hw_free(struct snd_pcm_substream *substream)
 	snd_pcm_set_state(substream, SNDRV_PCM_STATE_OPEN);
 	pm_qos_remove_request(&substream->latency_pm_qos_req);
  unlock:
-	mutex_unlock(&runtime->buffer_mutex);
+	snd_pcm_buffer_access_unlock(runtime);
 	return result;
 }
 
@@ -1221,12 +1243,15 @@ static int snd_pcm_action_nonatomic(const struct action_ops *ops,
 
 	/* Guarantee the group members won't change during non-atomic action */
 	down_read(&snd_pcm_link_rwsem);
-	mutex_lock(&substream->runtime->buffer_mutex);
+	res = snd_pcm_buffer_access_lock(substream->runtime);
+	if (res < 0)
+		goto unlock;
 	if (snd_pcm_stream_linked(substream))
 		res = snd_pcm_action_group(ops, substream, state, 0);
 	else
 		res = snd_pcm_action_single(ops, substream, state);
-	mutex_unlock(&substream->runtime->buffer_mutex);
+	snd_pcm_buffer_access_unlock(substream->runtime);
+ unlock:
 	up_read(&snd_pcm_link_rwsem);
 	return res;
 }
-- 
2.36.0

Re: [PATCH 5.4 0/5] ALSA: pcm: backports for CVE-2022-1048

[Greg KH]

On Fri, May 06, 2022 at 12:10:08PM +0300, Ovidiu Panait wrote:
> Contextual adjustments were made to apply to 5.4 stable tree.
> 
> Testing
> -------
> Running the PoC from [1] on 5.4.191 kernel produces the following oops:
> 
> qemu-system-x86_64 -nographic -serial mon:stdio -serial null -enable-kvm \
> -net user,hostname=qemu0,hostfwd=tcp::36074-:22 -net nic \
> -drive file=rootfs.ext4,format=raw -cpu host -m 4096 -kernel bzImage \
> -append "console=ttyS0,115200 root=/dev/sda rw ip=dhcp " -soundhw ac97 -smp 2
> root@intel-x86-64:~# ./poc
> ...
> [   95.839647] BUG: Bad page state in process poc  pfn:bb860
> [   95.841277] page:ffffea0002ee1800 refcount:-1 mapcount:0 mapping:0000000000000000 index:0x0
> [   95.843521] flags: 0x100000000000000()
> [   95.844539] raw: 0100000000000000 dead000000000100 dead000000000122 0000000000000000
> [   95.846306] raw: 0000000000000000 0000000000000000 ffffffffffffffff 0000000000000000
> [   95.847164] page dumped because: nonzero _refcount
> [   95.847705] Modules linked in:
> [   95.848063] CPU: 0 PID: 357 Comm: poc Tainted: G        W         5.4.191 #6
> [   95.848839] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> [   95.849847] Call Trace:
> [   95.850145]  dump_stack+0x76/0x9c
> [   95.850549]  bad_page.cold+0xff/0x124
> [   95.850980]  ? si_mem_available+0x2f0/0x2f0
> [   95.851464]  ? _raw_spin_trylock_bh+0x120/0x120
> [   95.851988]  ? __module_text_address+0xe/0x140
> [   95.852494]  get_page_from_freelist+0x16f9/0x35b0
> [   95.853034]  ? __isolate_free_page+0x460/0x460
> [   95.853543]  ? save_stack+0x4c/0x80
> [   95.853938]  ? save_stack+0x1b/0x80
> [   95.854343]  ? __kasan_kmalloc.constprop.0+0xc2/0xd0
> [   95.854897]  ? snd_pcm_lib_malloc_pages+0x2b8/0x680
> [   95.855433]  ? snd_intel8x0_hw_params+0x106/0x550
> [   95.855964]  ? snd_pcm_hw_params+0x2b5/0x1290
> [   95.856438]  ? snd_pcm_common_ioctl+0x332/0x1a20
> [   95.856954]  __alloc_pages_nodemask+0x274/0x610
> [   95.857460]  ? __alloc_pages_slowpath+0x1ff0/0x1ff0
> [   95.857992]  ? snd_pcm_hw_refine+0x8de/0xdd0
> [   95.858467]  ? kfree+0x8c/0x230
> [   95.858823]  __dma_direct_alloc_pages+0x18d/0x390
> [   95.859339]  dma_direct_alloc_pages+0x1b/0x170
> [   95.859827]  snd_dma_alloc_pages+0x1ae/0x380
> [   95.860294]  snd_pcm_lib_malloc_pages+0x371/0x680
> [   95.860812]  snd_intel8x0_hw_params+0x106/0x550
> [   95.861311]  snd_pcm_hw_params+0x2b5/0x1290
> [   95.861780]  ? _copy_from_user+0x70/0xa0
> [   95.862214]  snd_pcm_common_ioctl+0x332/0x1a20
> [   95.862699]  ? up_read+0x10/0x90
> [   95.863070]  ? n_tty_write+0x7ba/0xf70
> [   95.863484]  ? snd_pcm_status_user+0x120/0x120
> [   95.863974]  ? _raw_spin_lock_irqsave+0x7b/0xd0
> [   95.864473]  ? _raw_spin_trylock_bh+0x120/0x120
> [   95.864975]  snd_pcm_ioctl+0x62/0xa0
> [   95.865382]  do_vfs_ioctl+0x9af/0xf30
> [   95.865790]  ? selinux_file_ioctl+0x3ca/0x530
> [   95.866271]  ? ioctl_preallocate+0x1a0/0x1a0
> [   95.866739]  ? selinux_capable+0x20/0x20
> [   95.867172]  ? __fget_light+0xab/0x4c0
> [   95.867588]  ? syscall_trace_enter+0x50e/0xb40
> [   95.868074]  ? iterate_fd+0x180/0x180
> [   95.868478]  ksys_ioctl+0x59/0x90
> [   95.868853]  __x64_sys_ioctl+0x6a/0xb0
> [   95.869278]  do_syscall_64+0x89/0x2e0
> [   95.869681]  ? prepare_exit_to_usermode+0xec/0x190
> [   95.870213]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [   95.870764] RIP: 0033:0x7f6f375c8717
> [   95.871157] Code: 00 00 90 48 8b 05 69 57 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 8
> [   95.873187] RSP: 002b:00007ffdbdb71b48 EFLAGS: 00000206 ORIG_RAX: 0000000000000010
> [   95.874009] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f6f375c8717
> [   95.874780] RDX: 0000564d6f23c2a0 RSI: 00000000c2604111 RDI: 0000000000000003
> [   95.875555] RBP: 00007ffdbdb71c20 R08: 0000000000000000 R09: 0000000000000010
> [   95.876322] R10: 00007ffdbdb71a27 R11: 0000000000000206 R12: 0000564d6f15e120
> [   95.877093] R13: 00007ffdbdb71d00 R14: 0000000000000000 R15: 0000000000000000
> [   95.877864] Disabling lock debugging due to kernel taint
> [   95.881630] ==================================================================
> [   95.883522] BUG: KASAN: double-free or invalid-free in snd_pcm_lib_free_pages+0xe1/0x230
> [   95.885570] 
> [   95.885976] CPU: 1 PID: 371 Comm: poc Tainted: G    B   W         5.4.191 #6
> [   95.887787] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> [   95.890095] Call Trace:
> [   95.890505]  dump_stack+0x76/0x9c
> [   95.890859]  print_address_description.constprop.0+0x16/0x200
> [   95.891454]  ? snd_pcm_lib_free_pages+0xe1/0x230
> [   95.891940]  kasan_report_invalid_free+0x61/0xa0
> [   95.892429]  ? snd_pcm_lib_free_pages+0xe1/0x230
> [   95.892921]  __kasan_slab_free+0x15e/0x170
> [   95.893350]  ? snd_pcm_lib_free_pages+0xe1/0x230
> [   95.893843]  kfree+0x8c/0x230
> [   95.894163]  snd_pcm_lib_free_pages+0xe1/0x230
> [   95.894633]  snd_pcm_common_ioctl+0x599/0x1a20
> [   95.895089]  ? snd_pcm_status_user+0x120/0x120
> [   95.895543]  snd_pcm_ioctl+0x62/0xa0
> [   95.895912]  do_vfs_ioctl+0x9af/0xf30
> [   95.896292]  ? selinux_file_ioctl+0x3ca/0x530
> [   95.896752]  ? ioctl_preallocate+0x1a0/0x1a0
> [   95.897184]  ? selinux_capable+0x20/0x20
> [   95.897589]  ? __fget_light+0x2ab/0x4c0
> [   95.898002]  ? iterate_fd+0x180/0x180
> [   95.898385]  ksys_ioctl+0x59/0x90
> [   95.898739]  __x64_sys_ioctl+0x6a/0xb0
> [   95.899139]  do_syscall_64+0x89/0x2e0
> [   95.899521]  ? syscall_return_slowpath+0x17a/0x1e0
> [   95.900013]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [   95.900532] RIP: 0033:0x7f6f375c8717
> [   95.900905] Code: 00 00 90 48 8b 05 69 57 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 8
> [   95.902809] RSP: 002b:00007f6f30b72ee8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [   95.903572] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f6f375c8717
> [   95.904294] RDX: 0000000000000000 RSI: 0000000000004112 RDI: 0000000000000003
> [   95.905009] RBP: 00007f6f30b72f00 R08: 00007f6f30b73700 R09: 00007f6f30b73700
> [   95.905723] R10: 00007f6f30b739d0 R11: 0000000000000246 R12: 00007ffdbdb71ace
> [   95.906442] R13: 00007ffdbdb71acf R14: 00007f6f30b72fc0 R15: 00007f6f30b73700
> 
> 
> The testcase runs successfully after applying this patchset.
> 
> [1] https://www.openwall.com/lists/oss-security/2022/03/28/4
> 
> 
> Takashi Iwai (5):
>   ALSA: pcm: Fix races among concurrent hw_params and hw_free calls
>   ALSA: pcm: Fix races among concurrent read/write and buffer changes
>   ALSA: pcm: Fix races among concurrent prepare and hw_params/hw_free
>     calls
>   ALSA: pcm: Fix races among concurrent prealloc proc writes
>   ALSA: pcm: Fix potential AB/BA lock with buffer_mutex and mmap_lock
> 
>  include/sound/pcm.h     |   2 +
>  sound/core/pcm.c        |   3 ++
>  sound/core/pcm_lib.c    |   5 ++
>  sound/core/pcm_memory.c |  11 ++--
>  sound/core/pcm_native.c | 110 ++++++++++++++++++++++++++++------------
>  5 files changed, 95 insertions(+), 36 deletions(-)
> 
> -- 
> 2.36.0
> 

All now queued up, thanks.

greg k-h