[1/2] ASoC: dapm: Fix UAF for snd_soc_pcm_runtime object

Commit Message

Sameer Pujar Aug. 23, 2024, 2:43 p.m. UTC

From: robelin <[e-mail]>

When using kernel with the following extra config,

  - CONFIG_KASAN=y
  - CONFIG_KASAN_GENERIC=y
  - CONFIG_KASAN_INLINE=y
  - CONFIG_KASAN_VMALLOC=y
  - CONFIG_FRAME_WARN=4096

kernel detects that snd_pcm_suspend_all() access a freed
'snd_soc_pcm_runtime' object when the system is suspended, which
leads to a use-after-free bug:

[   52.047746] BUG: KASAN: use-after-free in snd_pcm_suspend_all+0x1a8/0x270
[   52.047765] Read of size 1 at addr ffff0000b9434d50 by task systemd-sleep/2330
[   52.047771]
[   52.047776] CPU: 7 PID: 2330 Comm: systemd-sleep Tainted: G           O      5.15.148-tegra #9
[   52.047781] Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer Kit/Jetson, BIOS 36.4.0-gcid-36705213 07/01/2024
[   52.047785] Call trace:
[   52.047787]  dump_backtrace+0x0/0x3c0
[   52.047794]  show_stack+0x34/0x50
[   52.047797]  dump_stack_lvl+0x68/0x8c
[   52.047802]  print_address_description.constprop.0+0x74/0x2c0
[   52.047809]  kasan_report+0x210/0x230
[   52.047815]  __asan_report_load1_noabort+0x3c/0x50
[   52.047820]  snd_pcm_suspend_all+0x1a8/0x270
[   52.047824]  snd_soc_suspend+0x19c/0x4e0
[   52.047831]  dpm_prepare+0x1c0/0xb90
[   52.047838]  dpm_suspend_start+0x30/0xe0
[   52.047842]  suspend_devices_and_enter+0x220/0xfd0
[   52.047849]  pm_suspend+0x464/0x560
[   52.047853]  state_store+0xac/0x180
[   52.047856]  kobj_attr_store+0x3c/0x90
[   52.047861]  sysfs_kf_write+0x10c/0x180
[   52.047868]  kernfs_fop_write_iter+0x280/0x3e0
[   52.047873]  new_sync_write+0x2cc/0x4c0
[   52.047877]  vfs_write+0x514/0x760
[   52.047880]  ksys_write+0x100/0x1f0
[   52.047883]  __arm64_sys_write+0x78/0xc0
[   52.047887]  invoke_syscall+0x7c/0x280
[   52.047892]  el0_svc_common.constprop.0+0xcc/0x270
[   52.047897]  do_el0_svc+0xb0/0x100
[   52.047901]  el0_svc+0x3c/0x90
[   52.047905]  el0t_64_sync_handler+0xac/0x130
[   52.047908]  el0t_64_sync+0x1a4/0x1a8

The snd_pcm_sync_stop() has a NULL check on 'substream->runtime' before
making any access. So we need to always set 'substream->runtime' to NULL
everytime we kfree() it.

Fixes: a72706ed8208 ("ASoC: codec2codec: remove ephemeral variables")
Signed-off-by: robelin <[e-mail]>
Signed-off-by: Sameer Pujar <[e-mail]>
---
 sound/soc/soc-dapm.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Mark Brown Aug. 23, 2024, 10:03 p.m. UTC | #1

On Fri, Aug 23, 2024 at 02:43:41PM +0000, Sameer Pujar wrote:
> From: robelin <[e-mail]>
> 
> When using kernel with the following extra config,
> 
>   - CONFIG_KASAN=y
>   - CONFIG_KASAN_GENERIC=y
>   - CONFIG_KASAN_INLINE=y
>   - CONFIG_KASAN_VMALLOC=y
>   - CONFIG_FRAME_WARN=4096
> 
> kernel detects that snd_pcm_suspend_all() access a freed
> 'snd_soc_pcm_runtime' object when the system is suspended, which
> leads to a use-after-free bug:
> 
> [   52.047746] BUG: KASAN: use-after-free in snd_pcm_suspend_all+0x1a8/0x270
> [   52.047765] Read of size 1 at addr ffff0000b9434d50 by task systemd-sleep/2330
> [   52.047771]
> [   52.047776] CPU: 7 PID: 2330 Comm: systemd-sleep Tainted: G           O      5.15.148-tegra #9
> [   52.047781] Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer Kit/Jetson, BIOS 36.4.0-gcid-36705213 07/01/2024
> [   52.047785] Call trace:
> [   52.047787]  dump_backtrace+0x0/0x3c0
> [   52.047794]  show_stack+0x34/0x50


Please think hard before including complete backtraces in upstream
reports, they are very large and contain almost no useful information
relative to their size so often obscure the relevant content in your
message. If part of the backtrace is usefully illustrative (it often is
for search engines if nothing else) then it's usually better to pull out
the relevant sections.

Patch

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index d7d6dbb9d9ea..e5900cb65cf6 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -4056,6 +4056,7 @@  static int snd_soc_dai_link_event(struct snd_soc_dapm_widget *w,
 
 	case SND_SOC_DAPM_POST_PMD:
 		kfree(substream->runtime);
+		substream->runtime = NULL;
 		break;
 
 	default: