[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <s5hk0bl1m6c.wl-tiwai@suse.de>
Date: Tue, 19 Apr 2022 11:01:47 +0200
From: Takashi Iwai <tiwai@...e.de>
To: syzbot <syzbot+70e777a39907d6d5fd0a@...kaller.appspotmail.com>
Cc: alsa-devel@...a-project.org, coding@...ic.se, colin.king@...el.com,
linux-kernel@...r.kernel.org, perex@...ex.cz,
syzkaller-bugs@...glegroups.com, tiwai@...e.com
Subject: Re: [syzbot] KASAN: use-after-free Read in __snd_rawmidi_transmit_peek
On Mon, 18 Apr 2022 12:12:23 +0200,
syzbot wrote:
>
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: a19944809fe9 Merge tag 'hardening-v5.18-rc3' of git://git...
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=16a40ae0f00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=eb177500e563582f
> dashboard link: https://syzkaller.appspot.com/bug?extid=70e777a39907d6d5fd0a
> compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1590dfa8f00000
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+70e777a39907d6d5fd0a@...kaller.appspotmail.com
>
> ==================================================================
> BUG: KASAN: use-after-free in __snd_rawmidi_transmit_peek+0x261/0x360 sound/core/rawmidi.c:1286
Looks like a leftover work. The fix patch is below.
Takashi
-- 8< --
From: Takashi Iwai <tiwai@...e.de>
Subject: [PATCH] ALSA: usb-audio: Fix potential use-after-free at releasing
MIDI streams
As recently spotted by syzbot, the USB MIDI code may leave a work that
handles the pending output queue even after a stream gets released
(either internally via sequencer or via device files), which may lead
to use-after-free. We have already a proper drop of pending URBs but
this is executed only at the device disconnection.
This patch moves the URB clean-up code into both the code path that is
called at releasing, namely, snd_usb_midi_in_endpoint_delete() and
snd_usb_midi_out_endpoint_clear() functions, so that the pending URBs
and work are cleared properly before releasing the resources.
Reported-by: syzbot+70e777a39907d6d5fd0a@...kaller.appspotmail.com
Cc: <stable@...r.kernel.org>
Link: https://lore.kernel.org/r/00000000000011555605dceaff03@google.com
Signed-off-by: Takashi Iwai <tiwai@...e.de>
---
sound/usb/midi.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/sound/usb/midi.c b/sound/usb/midi.c
index 2c01649c70f6..8e5281f7f1fc 100644
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -1247,6 +1247,10 @@ static void snd_usbmidi_in_endpoint_delete(struct snd_usb_midi_in_endpoint *ep)
{
unsigned int i;
+ for (i = 0; i < INPUT_URBS; ++i)
+ if (ep->urbs[i])
+ usb_kill_urb(ep->urbs[i]);
+
for (i = 0; i < INPUT_URBS; ++i)
if (ep->urbs[i])
free_urb_and_buffer(ep->umidi, ep->urbs[i],
@@ -1327,6 +1331,18 @@ static void snd_usbmidi_out_endpoint_clear(struct snd_usb_midi_out_endpoint *ep)
{
unsigned int i;
+ cancel_work_sync(&ep->work);
+ for (i = 0; i < OUTPUT_URBS; ++i)
+ if (ep->urbs[i].urb)
+ usb_kill_urb(ep->urbs[i].urb);
+ if (ep->umidi->usb_protocol_ops->finish_out_endpoint)
+ ep->umidi->usb_protocol_ops->finish_out_endpoint(ep);
+ ep->active_urbs = 0;
+ if (ep->drain_urbs) {
+ ep->drain_urbs = 0;
+ wake_up(&ep->drain_wait);
+ }
+
for (i = 0; i < OUTPUT_URBS; ++i)
if (ep->urbs[i].urb) {
free_urb_and_buffer(ep->umidi, ep->urbs[i].urb,
@@ -1469,7 +1485,7 @@ static void snd_usbmidi_free(struct snd_usb_midi *umidi)
void snd_usbmidi_disconnect(struct list_head *p)
{
struct snd_usb_midi *umidi;
- unsigned int i, j;
+ unsigned int i;
umidi = list_entry(p, struct snd_usb_midi, list);
/*
@@ -1487,22 +1503,6 @@ void snd_usbmidi_disconnect(struct list_head *p)
for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i) {
struct snd_usb_midi_endpoint *ep = &umidi->endpoints[i];
- if (ep->out)
- cancel_work_sync(&ep->out->work);
- if (ep->out) {
- for (j = 0; j < OUTPUT_URBS; ++j)
- usb_kill_urb(ep->out->urbs[j].urb);
- if (umidi->usb_protocol_ops->finish_out_endpoint)
- umidi->usb_protocol_ops->finish_out_endpoint(ep->out);
- ep->out->active_urbs = 0;
- if (ep->out->drain_urbs) {
- ep->out->drain_urbs = 0;
- wake_up(&ep->out->drain_wait);
- }
- }
- if (ep->in)
- for (j = 0; j < INPUT_URBS; ++j)
- usb_kill_urb(ep->in->urbs[j]);
/* free endpoints here; later call can result in Oops */
if (ep->out)
snd_usbmidi_out_endpoint_clear(ep->out);
--
2.31.1
Powered by blists - more mailing lists