On 5/7/2020 11:56 AM, Takashi Iwai wrote:
On Thu, 07 May 2020 10:23:02 +0200, Greg Kroah-Hartman wrote:
On Thu, May 07, 2020 at 04:04:25PM +0800, butt3rflyh4ck wrote:
I report a bug (in linux-5.7-rc1) found by syzkaller.
kernel config: https://github.com/butterflyhack/syzkaller-fuzz/blob/master/v5.7.0-rc1.confi... reproducer: https://github.com/butterflyhack/syzkaller-fuzz/blob/master/repro.cprog
I test the reproducer in linux-5.7-rc4 and crash too.
Great, care to create a fix for this and send it to the proper maintainers? That's the best way to get it fixed, otherwise it just goes in the file with the rest of the syzbot reports we are burried under.
Don't worry, I already prepared a fix patch below :)
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: rawmidi: Fix racy buffer resize under concurrent accesses
The rawmidi core allows user to resize the runtime buffer via ioctl, and this may lead to UAF when performed during concurrent reads or writes.
This patch fixes the race by introducing a reference counter for the runtime buffer access and returns -EBUSY error when the resize is performed concurrently.
Reported-by: butt3rflyh4ck butterflyhuangxx@gmail.com Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/CAFcO6XMWpUVK_yzzCpp8_XP7+=oUpQvuBeCbMffEDkpe8jWrf... Signed-off-by: Takashi Iwai tiwai@suse.de
include/sound/rawmidi.h | 1 + sound/core/rawmidi.c | 29 ++++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/include/sound/rawmidi.h b/include/sound/rawmidi.h index a36b7227a15a..334842daa904 100644 --- a/include/sound/rawmidi.h
(...)
@@ -1021,6 +1039,7 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream, unsigned long appl_ptr;
spin_lock_irqsave(&runtime->lock, flags);
- snd_rawmidi_buffer_ref(runtime); while (count > 0 && runtime->avail) { count1 = runtime->buffer_size - runtime->appl_ptr; if (count1 > count)
@@ -1040,13 +1059,17 @@ static long snd_rawmidi_kernel_read1(struct snd_rawmidi_substream *substream, spin_unlock_irqrestore(&runtime->lock, flags);
First unlock
if (copy_to_user(userbuf + result, runtime->buffer + appl_ptr, count1)) {
return result > 0 ? result : -EFAULT;
if (!result)result = -EFAULT;goto out;
goto -> Second unlock
} spin_lock_irqsave(&runtime->lock, flags); } result += count1; count -= count1;}
- out:
- snd_rawmidi_buffer_unref(runtime); spin_unlock_irqrestore(&runtime->lock, flags);
Second unlock
return result; }
So if I follow this correctly, you call spin_unlock_irqrestore twice in case of error?