On Tue, 23 Sep 2025 04:40:13 +0200, Alan Stern wrote: > > On Tue, Sep 23, 2025 at 04:47:20AM +0530, Brahmajit Das wrote: > > Syzkaller reported a general protection fault in snd_usbmidi_do_output(), > > caused by dereferencing a NULL URB pointer when accessing > > ep->urbs[urb_index].urb. > > > > This can happen in rare race conditions where the URB was not initialized > > or was already freed (e.g. during disconnect or after errors), and the > > output timer or other path tries to reuse it. > > > > Fix this by checking if the URB is NULL before accessing it, and skipping > > the current slot if it is. > > > > Reported-by: syzbot+f02665daa2abeef4a947@xxxxxxxxxxxxxxxxxxxxxxxxx > > Link: https://syzkaller.appspot.com/bug?extid=f02665daa2abeef4a947 > > > > Signed-off-by: Brahmajit Das <listout@xxxxxxxxxxx> > > --- > > sound/usb/midi.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/sound/usb/midi.c b/sound/usb/midi.c > > index acb3bf92857c..7919a39decb4 100644 > > --- a/sound/usb/midi.c > > +++ b/sound/usb/midi.c > > @@ -307,6 +307,10 @@ static void snd_usbmidi_do_output(struct snd_usb_midi_out_endpoint *ep) > > for (;;) { > > if (!(ep->active_urbs & (1 << urb_index))) { > > urb = ep->urbs[urb_index].urb; > > + if (!urb) { > > + // Skip this urb > > + goto next_urb; > > + } > > What prevents the URB from being freed right here? If this happens, > the code below would access memory that was deallocated. > > To prevent races, you have to use some sort of lock or other > synchronization mechanism. A simple test won't work. Yes. The timer instance itself should have been killed before releasing the resources. I guess the patch below could cover it, but since I'm traveling, I can't check it for now. Will check later once after I'm back. thanks, Takashi --- a/sound/usb/midi.c +++ b/sound/usb/midi.c @@ -1511,6 +1511,7 @@ static void snd_usbmidi_free(struct snd_usb_midi *umidi) { int i; + timer_shutdown_sync(&umidi->error_timer); for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i) { struct snd_usb_midi_endpoint *ep = &umidi->endpoints[i]; if (ep->out) @@ -1519,7 +1520,6 @@ static void snd_usbmidi_free(struct snd_usb_midi *umidi) snd_usbmidi_in_endpoint_delete(ep->in); } mutex_destroy(&umidi->mutex); - timer_shutdown_sync(&umidi->error_timer); kfree(umidi); }
