| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 04 Sep 2022 12:27:43 +0200
From: Takashi Iwai <tiwai@...e.de>
To: butt3rflyh4ck <butterflyhuangxx@...il.com>
Cc: perex@...ex.cz, tiwai@...e.com, cuibixuan@...ux.alibaba.com,
alsa-devel@...a-project.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: A new null-ptr-deref Write bug in snd_pcm_format_set_silence
On Sun, 04 Sep 2022 11:48:37 +0200,
butt3rflyh4ck wrote:
>
> Hi, there is a new null-ptr-deref Write bug in
> snd_pcm_format_set_slience in sound/core/pcm_misc.c in the latest
> upstream kernel and can reproduce it.
> We call SNDCTL_DSP_SYNC and SNDCTL_DSP_SPEED in multiple threads to
> trigger the vulnerability.
>
> See the Call Trace:
> ==================================================================
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
> kasan_report+0xb1/0x1e0 mm/kasan/report.c:495
> check_region_inline mm/kasan/generic.c:183 [inline]
> kasan_check_range+0x13d/0x180 mm/kasan/generic.c:189
> memset+0x20/0x40 mm/kasan/shadow.c:44
> snd_pcm_format_set_silence sound/core/pcm_misc.c:441 [inline]
> snd_pcm_format_set_silence+0x215/0x350 sound/core/pcm_misc.c:424
> snd_pcm_oss_sync+0x60e/0x800 sound/core/oss/pcm_oss.c:1690
> snd_pcm_oss_ioctl+0x2087/0x3420 sound/core/oss/pcm_oss.c:2634
> vfs_ioctl fs/ioctl.c:51 [inline]
> __do_sys_ioctl fs/ioctl.c:870 [inline]
> __se_sys_ioctl fs/ioctl.c:856 [inline]
> __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:856
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> ==================================================================
> We can see the function snd_pcm_format_set_silence code below:
> ```
> int snd_pcm_format_set_silence(snd_pcm_format_t format, void *data,
> unsigned int samples)
> {
> int width;
> unsigned char *dst;
> const unsigned char *pat;
>
> if (!valid_format(format))
> return -EINVAL;
> if (samples == 0)
> return 0;
> width = pcm_formats[(INT)format].phys; /* physical width */
> pat = pcm_formats[(INT)format].silence;
> if (!width || !pat)
> return -EINVAL;
> /* signed or 1 byte data */
> if (pcm_formats[(INT)format].signd == 1 || width <= 8) {
> unsigned int bytes = samples * width / 8;
> memset(data, *pat, bytes); ///// [1] ---------> data is NULL
> return 0;
> }
> ......
> }
> ```
> [1], the data pointer is NULL, we can know snd_pcm_format_set_silence
> called in line 1690 in sound/core/oss/pcm_oss.c from call stack trace.
> let we see code below:
> ```
> static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file)
> {
> int err = 0;
> unsigned int saved_f_flags;
> struct snd_pcm_substream *substream;
> struct snd_pcm_runtime *runtime;
> snd_pcm_format_t format;
> unsigned long width;
> size_t size;
>
> substream = pcm_oss_file->streams[SNDRV_PCM_STREAM_PLAYBACK];
> if (substream != NULL) {
> runtime = substream->runtime;
> if (atomic_read(&substream->mmap_count))
> goto __direct;
> err = snd_pcm_oss_make_ready(substream);
> if (err < 0)
> return err;
> atomic_inc(&runtime->oss.rw_ref);
> if (mutex_lock_interruptible(&runtime->oss.params_lock)) {
> atomic_dec(&runtime->oss.rw_ref);
> return -ERESTARTSYS;
> }
> format = snd_pcm_oss_format_from(runtime->oss.format);
> width = snd_pcm_format_physical_width(format);
> if (runtime->oss.buffer_used > 0) {
> #ifdef OSS_DEBUG
> pcm_dbg(substream->pcm, "sync: buffer_used\n");
> #endif
> size = (8 * (runtime->oss.period_bytes -
> runtime->oss.buffer_used) + 7) / width;
> snd_pcm_format_set_silence(format,
> runtime->oss.buffer
> + runtime->oss.buffer_used, ///// [2]
> size);
> err = snd_pcm_oss_sync1(substream,
> runtime->oss.period_bytes);
> if (err < 0)
> goto unlock;
> } else if (runtime->oss.period_ptr > 0) {
>
> ```
> [2] runtime->oss.buffer + runtime->oss.buffer_used is the data
> pointer, but runtime->oss.buffer is NULL here but it doesn't make
> sense.
> runtime->oss.buffter is allocated by kvzalloc, if runtime->oss_buffer
> is NULL, it would return an ENOMEM error.
> Maybe I think there is a race condition, the runtime->oss.buffer is
> freed and set to NULL but we can use runtime->oss.buffter via ioctl.
Yeah, likely it's in a small race window between two calls.
Could you try the patch below?
thanks,
Takashi
---
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -1672,14 +1672,14 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file)
runtime = substream->runtime;
if (atomic_read(&substream->mmap_count))
goto __direct;
- err = snd_pcm_oss_make_ready(substream);
- if (err < 0)
- return err;
atomic_inc(&runtime->oss.rw_ref);
if (mutex_lock_interruptible(&runtime->oss.params_lock)) {
atomic_dec(&runtime->oss.rw_ref);
return -ERESTARTSYS;
}
+ err = snd_pcm_oss_make_ready_locked(substream);
+ if (err < 0)
+ goto unlock;
format = snd_pcm_oss_format_from(runtime->oss.format);
width = snd_pcm_format_physical_width(format);
if (runtime->oss.buffer_used > 0) {
Powered by blists - more mailing lists