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@xxxxxxxxxxxxxxxxxxxxxxxxx > > ================================================================== > 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@xxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxx Cc: <stable@xxxxxxxxxxxxxxx> Link: https://lore.kernel.org/r/00000000000011555605dceaff03@xxxxxxxxxx Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> --- 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
