linux-kernel - Re: Bug: slab-out-of-bounds in snd_seq_oss_synth

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ed1qydgi.wl-tiwai@suse.de>
Date: Sun, 29 Dec 2024 11:45:49 +0100
From: Takashi Iwai <tiwai@...e.de>
To: Kun Hu <huk23@...udan.edu.cn>
Cc: perex@...ex.cz,
	tiwai@...e.com,
	linux-sound@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	"jjtan24@...udan.edu.cn" <jjtan24@...udan.edu.cn>
Subject: Re: Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

On Wed, 25 Dec 2024 06:37:47 +0100,
Kun Hu wrote:
> 
> Hello,
> 
> > BUG: KASAN: slab-out-of-bounds in snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
> 
> We further analyzed the issue at line 516 in ./sound/core/seq/oss/seq_oss_synth.c. 
> The slab-out-of-bounds crash occurs in line 509, when sysex->len = 128. Specifically, the write operation to dest[0] accesses memory beyond the bounds of sysex->buf (128 byte).
> To resolve this issue, we suggest adding 6 lines of code to validate the legality of the address write to sysex->buf before entering the loop:
> 
> if (sysex->len >= MAX_SYSEX_BUFLEN) { 
>    sysex->len = 0; 
>    sysex->skip = 1; 
>    return -EINVAL;  /* Exit early if sysex->len is out of bounds */ 
> }

The only place incrementing sysex->len is that loop and it has already
the bounce check which resets sysex->len to 0.  That is, the likely
reason of the buffer overflow is rather the racy calls of this
function.

If so, a potential fix would be rather to protect the concurrency,
e.g. a simplest form is something like below.
Could you check whether it addresses your problem?


thanks,

Takashi

-- 8< --
--- a/sound/core/seq/oss/seq_oss_synth.c
+++ b/sound/core/seq/oss/seq_oss_synth.c
@@ -66,6 +66,7 @@ static struct seq_oss_synth midi_synth_dev = {
 };
 
 static DEFINE_SPINLOCK(register_lock);
+static DEFINE_MUTEX(sysex_mutex);
 
 /*
  * prototypes
@@ -497,6 +498,7 @@ snd_seq_oss_synth_sysex(struct seq_oss_devinfo *dp, int dev, unsigned char *buf,
 	if (!info)
 		return -ENXIO;
 
+	guard(mutex)(&sysex_mutex);
 	sysex = info->sysex;
 	if (sysex == NULL) {
 		sysex = kzalloc(sizeof(*sysex), GFP_KERNEL);