[PATCH 6.12.y 6.6.y 6.1.y 5.15.y 5.10.y 5.4.y] ALSA: usb-audio: Kill timer properly at removal

[PATCH 6.12.y 6.6.y 6.1.y 5.15.y 5.10.y 5.4.y] ALSA: usb-audio: Kill timer properly at removal

[Jeongjun Park]

From: Takashi Iwai <tiwai@suse.de>

[ Upstream commit 0718a78f6a9f04b88d0dc9616cc216b31c5f3cf1 ]

The USB-audio MIDI code initializes the timer, but in a rare case, the
driver might be freed without the disconnect call.  This leaves the
timer in an active state while the assigned object is released via
snd_usbmidi_free(), which ends up with a kernel warning when the debug
configuration is enabled, as spotted by fuzzer.

For avoiding the problem, put timer_shutdown_sync() at
snd_usbmidi_free(), so that the timer can be killed properly.
While we're at it, replace the existing timer_delete_sync() at the
disconnect callback with timer_shutdown_sync(), too.

Reported-by: syzbot+d8f72178ab6783a7daea@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/681c70d7.050a0220.a19a9.00c6.GAE@google.com
Cc: <stable@vger.kernel.org>
Link: https://patch.msgid.link/20250519212031.14436-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
[ del_timer vs timer_delete differences ]
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 sound/usb/midi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/usb/midi.c b/sound/usb/midi.c
index a792ada18863..c3de2b137435 100644
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -1530,6 +1530,7 @@ 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);
 }
 
@@ -1553,7 +1554,7 @@ void snd_usbmidi_disconnect(struct list_head *p)
 	spin_unlock_irq(&umidi->disc_lock);
 	up_write(&umidi->disc_rwsem);
 
-	del_timer_sync(&umidi->error_timer);
+	timer_shutdown_sync(&umidi->error_timer);
 
 	for (i = 0; i < MIDI_MAX_ENDPOINTS; ++i) {
 		struct snd_usb_midi_endpoint *ep = &umidi->endpoints[i];
--

Re: [PATCH 6.12.y 6.6.y 6.1.y 5.15.y 5.10.y 5.4.y] ALSA: usb-audio: Kill timer properly at removal

[Greg KH]

On Wed, Oct 08, 2025 at 12:58:08AM +0900, Jeongjun Park wrote:
> From: Takashi Iwai <tiwai@suse.de>
> 
> [ Upstream commit 0718a78f6a9f04b88d0dc9616cc216b31c5f3cf1 ]
> 
> The USB-audio MIDI code initializes the timer, but in a rare case, the
> driver might be freed without the disconnect call.  This leaves the
> timer in an active state while the assigned object is released via
> snd_usbmidi_free(), which ends up with a kernel warning when the debug
> configuration is enabled, as spotted by fuzzer.
> 
> For avoiding the problem, put timer_shutdown_sync() at
> snd_usbmidi_free(), so that the timer can be killed properly.
> While we're at it, replace the existing timer_delete_sync() at the
> disconnect callback with timer_shutdown_sync(), too.
> 
> Reported-by: syzbot+d8f72178ab6783a7daea@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/681c70d7.050a0220.a19a9.00c6.GAE@google.com
> Cc: <stable@vger.kernel.org>
> Link: https://patch.msgid.link/20250519212031.14436-1-tiwai@suse.de
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> [ del_timer vs timer_delete differences ]
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> ---
>  sound/usb/midi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/usb/midi.c b/sound/usb/midi.c
> index a792ada18863..c3de2b137435 100644
> --- a/sound/usb/midi.c
> +++ b/sound/usb/midi.c
> @@ -1530,6 +1530,7 @@ 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);

This function is not in older kernel versions, you did not test this
build :(

I've applied this to 6.6.y and newer, but for 6.1.y and older, please
use the proper function.

thanks,

greg k-h

Re: [PATCH 6.12.y 6.6.y 6.1.y 5.15.y 5.10.y 5.4.y] ALSA: usb-audio: Kill timer properly at removal

[Jeongjun Park]

Hi Greg,

Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Oct 08, 2025 at 12:58:08AM +0900, Jeongjun Park wrote:
> > From: Takashi Iwai <tiwai@suse.de>
> >
> > [ Upstream commit 0718a78f6a9f04b88d0dc9616cc216b31c5f3cf1 ]
> >
> > The USB-audio MIDI code initializes the timer, but in a rare case, the
> > driver might be freed without the disconnect call.  This leaves the
> > timer in an active state while the assigned object is released via
> > snd_usbmidi_free(), which ends up with a kernel warning when the debug
> > configuration is enabled, as spotted by fuzzer.
> >
> > For avoiding the problem, put timer_shutdown_sync() at
> > snd_usbmidi_free(), so that the timer can be killed properly.
> > While we're at it, replace the existing timer_delete_sync() at the
> > disconnect callback with timer_shutdown_sync(), too.
> >
> > Reported-by: syzbot+d8f72178ab6783a7daea@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/681c70d7.050a0220.a19a9.00c6.GAE@google.com
> > Cc: <stable@vger.kernel.org>
> > Link: https://patch.msgid.link/20250519212031.14436-1-tiwai@suse.de
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > [ del_timer vs timer_delete differences ]
> > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> > ---
> >  sound/usb/midi.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/usb/midi.c b/sound/usb/midi.c
> > index a792ada18863..c3de2b137435 100644
> > --- a/sound/usb/midi.c
> > +++ b/sound/usb/midi.c
> > @@ -1530,6 +1530,7 @@ 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);
>
> This function is not in older kernel versions, you did not test this
> build :(
>
> I've applied this to 6.6.y and newer, but for 6.1.y and older, please
> use the proper function.

Sorry, I didn't realize that the timer_shutdown_sync() implementation
commit wasn't backported to versions prior to 6.2.

But why wasn't this feature backported to previous versions? I understand
that most drivers write separate patches for pre-6.2 versions that don't
implement timer_shutdown_sync() to address this type of bug.

Is there some other, unavoidable reason I'm unaware of?

>
> thanks,
>
> greg k-h

Regards,
Jeongjun Park

Re: [PATCH 6.12.y 6.6.y 6.1.y 5.15.y 5.10.y 5.4.y] ALSA: usb-audio: Kill timer properly at removal

[Greg KH]

On Thu, Oct 09, 2025 at 08:23:42PM +0900, Jeongjun Park wrote:
> Hi Greg,
> 
> Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Oct 08, 2025 at 12:58:08AM +0900, Jeongjun Park wrote:
> > > From: Takashi Iwai <tiwai@suse.de>
> > >
> > > [ Upstream commit 0718a78f6a9f04b88d0dc9616cc216b31c5f3cf1 ]
> > >
> > > The USB-audio MIDI code initializes the timer, but in a rare case, the
> > > driver might be freed without the disconnect call.  This leaves the
> > > timer in an active state while the assigned object is released via
> > > snd_usbmidi_free(), which ends up with a kernel warning when the debug
> > > configuration is enabled, as spotted by fuzzer.
> > >
> > > For avoiding the problem, put timer_shutdown_sync() at
> > > snd_usbmidi_free(), so that the timer can be killed properly.
> > > While we're at it, replace the existing timer_delete_sync() at the
> > > disconnect callback with timer_shutdown_sync(), too.
> > >
> > > Reported-by: syzbot+d8f72178ab6783a7daea@syzkaller.appspotmail.com
> > > Closes: https://lore.kernel.org/681c70d7.050a0220.a19a9.00c6.GAE@google.com
> > > Cc: <stable@vger.kernel.org>
> > > Link: https://patch.msgid.link/20250519212031.14436-1-tiwai@suse.de
> > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > [ del_timer vs timer_delete differences ]
> > > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> > > ---
> > >  sound/usb/midi.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/sound/usb/midi.c b/sound/usb/midi.c
> > > index a792ada18863..c3de2b137435 100644
> > > --- a/sound/usb/midi.c
> > > +++ b/sound/usb/midi.c
> > > @@ -1530,6 +1530,7 @@ 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);
> >
> > This function is not in older kernel versions, you did not test this
> > build :(
> >
> > I've applied this to 6.6.y and newer, but for 6.1.y and older, please
> > use the proper function.
> 
> Sorry, I didn't realize that the timer_shutdown_sync() implementation
> commit wasn't backported to versions prior to 6.2.
> 
> But why wasn't this feature backported to previous versions? I understand
> that most drivers write separate patches for pre-6.2 versions that don't
> implement timer_shutdown_sync() to address this type of bug.

Features are not backported to older kernels.  If you want this fix in
an older kernel, then you need to either backport that feature, or fix
up the driver to use whatever was used before that function existed.

thanks,

greg k-h

Re: [PATCH 6.12.y 6.6.y 6.1.y 5.15.y 5.10.y 5.4.y] ALSA: usb-audio: Kill timer properly at removal

[Jeongjun Park]

Hi Greg,

Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Oct 09, 2025 at 08:23:42PM +0900, Jeongjun Park wrote:
> > Hi Greg,
> >
> > Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Wed, Oct 08, 2025 at 12:58:08AM +0900, Jeongjun Park wrote:
> > > > From: Takashi Iwai <tiwai@suse.de>
> > > >
> > > > [ Upstream commit 0718a78f6a9f04b88d0dc9616cc216b31c5f3cf1 ]
> > > >
> > > > The USB-audio MIDI code initializes the timer, but in a rare case, the
> > > > driver might be freed without the disconnect call.  This leaves the
> > > > timer in an active state while the assigned object is released via
> > > > snd_usbmidi_free(), which ends up with a kernel warning when the debug
> > > > configuration is enabled, as spotted by fuzzer.
> > > >
> > > > For avoiding the problem, put timer_shutdown_sync() at
> > > > snd_usbmidi_free(), so that the timer can be killed properly.
> > > > While we're at it, replace the existing timer_delete_sync() at the
> > > > disconnect callback with timer_shutdown_sync(), too.
> > > >
> > > > Reported-by: syzbot+d8f72178ab6783a7daea@syzkaller.appspotmail.com
> > > > Closes: https://lore.kernel.org/681c70d7.050a0220.a19a9.00c6.GAE@google.com
> > > > Cc: <stable@vger.kernel.org>
> > > > Link: https://patch.msgid.link/20250519212031.14436-1-tiwai@suse.de
> > > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > > [ del_timer vs timer_delete differences ]
> > > > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> > > > ---
> > > >  sound/usb/midi.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/sound/usb/midi.c b/sound/usb/midi.c
> > > > index a792ada18863..c3de2b137435 100644
> > > > --- a/sound/usb/midi.c
> > > > +++ b/sound/usb/midi.c
> > > > @@ -1530,6 +1530,7 @@ 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);
> > >
> > > This function is not in older kernel versions, you did not test this
> > > build :(
> > >
> > > I've applied this to 6.6.y and newer, but for 6.1.y and older, please
> > > use the proper function.
> >
> > Sorry, I didn't realize that the timer_shutdown_sync() implementation
> > commit wasn't backported to versions prior to 6.2.
> >
> > But why wasn't this feature backported to previous versions? I understand
> > that most drivers write separate patches for pre-6.2 versions that don't
> > implement timer_shutdown_sync() to address this type of bug.
>
> Features are not backported to older kernels.  If you want this fix in
> an older kernel, then you need to either backport that feature, or fix
> up the driver to use whatever was used before that function existed.

I could write a separate patch to fix this bug, but I'm not happy that
great features like timer_shutdown_sync() haven't been backported.

So, I'll try to backport the "timers: Provide timer_shutdown[_sync]()"
patch series sometime soon.

>
> thanks,
>
> greg k-h

Regards,
Jeongjun Park