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@syzkaller.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@suse.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@syzkaller.appspotmail.com Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/00000000000011555605dceaff03@google.com Signed-off-by: Takashi Iwai tiwai@suse.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);