mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [BUG] ALSA: ump: NULL deref of legacy_rmidi after parse sets parsed
@ 2026-09-02  2:07 Qingyu Zhang
  2026-09-02  6:29 ` Takashi Iwai
  0 siblings, 1 reply; 4+ messages in thread
From: Qingyu Zhang @ 2026-09-02  2:07 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai; +Cc: linux-sound, linux-kernel

Hello,

ump_legacy_set_rawmidi_name() snprintf()s into ump->legacy_rmidi->name
when ump->parsed is true, but parsed is set at the end of
snd_ump_parse_endpoint() *before* snd_ump_attach_legacy_rawmidi().
A UMP packet in that window NULL-derefs.

Type: null-pointer dereference

* Summary

snd_ump_parse_endpoint() always does:

       error:
               ump->parsed = true;
               ...

ump_handle_ep_name_msg():

       if (ret && ump->parsed) {
               ump_set_rawmidi_name(ump);
               ump_legacy_set_rawmidi_name(ump);  /* rmidi may be NULL */
       }

ump_legacy_set_rawmidi_name():

       rmidi = ump->legacy_rmidi;
       snprintf(rmidi->name, ...);   /* no NULL check */

This runs from snd_ump_receive() on the USB input URB complete path
(in interrupt).

* Affected

37e0e14128e0. Needs CONFIG_SND_UMP, CONFIG_SND_UMP_LEGACY_RAWMIDI,
CONFIG_SND_USB_AUDIO, a MIDI 2.0 gadget or device. KASAN.

The natural window is parse-done vs attach. The QEMU PoC widens it
with a kprobe on snd_ump_receive (poc/widen_ump.c) plus dummy_hcd
configfs midi2, because the un-widened window is short.

* Reproduction

       # dummy_hcd + configfs usb_gadget midi2.usb0 (see poc/run.sh)
       # with widen_ump.ko: force parsed=1, legacy_rmidi=NULL on receive

KASAN: null-ptr-deref in snprintf from ump_legacy_set_rawmidi_name
<- ump_handle_ep_name_msg <- snd_ump_receive <- input_urb_complete.
Then "Fatal exception in interrupt".

* Expected

legacy_rmidi helpers no-op until attach has stored the pointer.

* Actual

IRQ-context NULL deref.

Please consider the suggested patch

Thanks.

Suggested patch:
```
diff --git a/sound/core/ump.c b/sound/core/ump.c
index d183c8a000bd..3d1a2ed3b476 100644
--- a/sound/core/ump.c
+++ b/sound/core/ump.c
@@ -1335,6 +1335,8 @@ static void update_legacy_names(struct
snd_ump_endpoint *ump)
{
    struct snd_rawmidi *rmidi = ump->legacy_rmidi;

+    if (!rmidi)
+        return;
    update_legacy_substreams(ump, rmidi, SNDRV_RAWMIDI_STREAM_INPUT);
    update_legacy_substreams(ump, rmidi, SNDRV_RAWMIDI_STREAM_OUTPUT);
}
@@ -1343,6 +1345,8 @@ static void ump_legacy_set_rawmidi_name(struct
snd_ump_endpoint *ump)
{
    struct snd_rawmidi *rmidi = ump->legacy_rmidi;

+    if (!rmidi)
+        return;
    snprintf(rmidi->name, sizeof(rmidi->name), "%.68s (MIDI 1.0)",
         ump->core.name);
}
```

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [BUG] ALSA: ump: NULL deref of legacy_rmidi after parse sets parsed
  2026-09-02  2:07 [BUG] ALSA: ump: NULL deref of legacy_rmidi after parse sets parsed Qingyu Zhang
@ 2026-09-02  6:29 ` Takashi Iwai
  2026-09-02  6:58   ` Qingyu Zhang
  0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2026-09-02  6:29 UTC (permalink / raw)
  To: Qingyu Zhang; +Cc: Jaroslav Kysela, Takashi Iwai, linux-sound, linux-kernel

On Wed, 02 Sep 2026 04:07:30 +0200,
Qingyu Zhang wrote:
> 
> Hello,
> 
> ump_legacy_set_rawmidi_name() snprintf()s into ump->legacy_rmidi->name
> when ump->parsed is true, but parsed is set at the end of
> snd_ump_parse_endpoint() *before* snd_ump_attach_legacy_rawmidi().
> A UMP packet in that window NULL-derefs.
> 
> Type: null-pointer dereference
> 
> * Summary
> 
> snd_ump_parse_endpoint() always does:
> 
>        error:
>                ump->parsed = true;
>                ...
> 
> ump_handle_ep_name_msg():
> 
>        if (ret && ump->parsed) {
>                ump_set_rawmidi_name(ump);
>                ump_legacy_set_rawmidi_name(ump);  /* rmidi may be NULL */
>        }
> 
> ump_legacy_set_rawmidi_name():
> 
>        rmidi = ump->legacy_rmidi;
>        snprintf(rmidi->name, ...);   /* no NULL check */
> 
> This runs from snd_ump_receive() on the USB input URB complete path
> (in interrupt).
> 
> * Affected
> 
> 37e0e14128e0. Needs CONFIG_SND_UMP, CONFIG_SND_UMP_LEGACY_RAWMIDI,
> CONFIG_SND_USB_AUDIO, a MIDI 2.0 gadget or device. KASAN.
> 
> The natural window is parse-done vs attach. The QEMU PoC widens it
> with a kprobe on snd_ump_receive (poc/widen_ump.c) plus dummy_hcd
> configfs midi2, because the un-widened window is short.
> 
> * Reproduction
> 
>        # dummy_hcd + configfs usb_gadget midi2.usb0 (see poc/run.sh)
>        # with widen_ump.ko: force parsed=1, legacy_rmidi=NULL on receive
> 
> KASAN: null-ptr-deref in snprintf from ump_legacy_set_rawmidi_name
> <- ump_handle_ep_name_msg <- snd_ump_receive <- input_urb_complete.
> Then "Fatal exception in interrupt".
> 
> * Expected
> 
> legacy_rmidi helpers no-op until attach has stored the pointer.
> 
> * Actual
> 
> IRQ-context NULL deref.
> 
> Please consider the suggested patch
> 
> Thanks.
> 
> Suggested patch:
> ```
> diff --git a/sound/core/ump.c b/sound/core/ump.c
> index d183c8a000bd..3d1a2ed3b476 100644
> --- a/sound/core/ump.c
> +++ b/sound/core/ump.c
> @@ -1335,6 +1335,8 @@ static void update_legacy_names(struct
> snd_ump_endpoint *ump)
> {
>     struct snd_rawmidi *rmidi = ump->legacy_rmidi;
> 
> +    if (!rmidi)
> +        return;
>     update_legacy_substreams(ump, rmidi, SNDRV_RAWMIDI_STREAM_INPUT);
>     update_legacy_substreams(ump, rmidi, SNDRV_RAWMIDI_STREAM_OUTPUT);
> }
> @@ -1343,6 +1345,8 @@ static void ump_legacy_set_rawmidi_name(struct
> snd_ump_endpoint *ump)
> {
>     struct snd_rawmidi *rmidi = ump->legacy_rmidi;
> 
> +    if (!rmidi)
> +        return;
>     snprintf(rmidi->name, sizeof(rmidi->name), "%.68s (MIDI 1.0)",
>          ump->core.name);
> }
> ```

Thanks for the report.  The suggested code change looks good.
Could you submit a patch in the proper format for upstreaming
(especially with your Signed-off-by tag)?


thanks,

Takashi

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [BUG] ALSA: ump: NULL deref of legacy_rmidi after parse sets parsed
  2026-09-02  6:29 ` Takashi Iwai
@ 2026-09-02  6:58   ` Qingyu Zhang
  2026-09-02  7:11     ` Takashi Iwai
  0 siblings, 1 reply; 4+ messages in thread
From: Qingyu Zhang @ 2026-09-02  6:58 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Jaroslav Kysela, linux-sound, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3085 bytes --]

Dear Takashi,

Thank you for your email. The formatted patch with the Signed-off-by
tag is attached; please check.

Yours,
Ian

>
> On Wed, 02 Sep 2026 04:07:30 +0200,
> Qingyu Zhang wrote:
> >
> > Hello,
> >
> > ump_legacy_set_rawmidi_name() snprintf()s into ump->legacy_rmidi->name
> > when ump->parsed is true, but parsed is set at the end of
> > snd_ump_parse_endpoint() *before* snd_ump_attach_legacy_rawmidi().
> > A UMP packet in that window NULL-derefs.
> >
> > Type: null-pointer dereference
> >
> > * Summary
> >
> > snd_ump_parse_endpoint() always does:
> >
> >        error:
> >                ump->parsed = true;
> >                ...
> >
> > ump_handle_ep_name_msg():
> >
> >        if (ret && ump->parsed) {
> >                ump_set_rawmidi_name(ump);
> >                ump_legacy_set_rawmidi_name(ump);  /* rmidi may be NULL */
> >        }
> >
> > ump_legacy_set_rawmidi_name():
> >
> >        rmidi = ump->legacy_rmidi;
> >        snprintf(rmidi->name, ...);   /* no NULL check */
> >
> > This runs from snd_ump_receive() on the USB input URB complete path
> > (in interrupt).
> >
> > * Affected
> >
> > 37e0e14128e0. Needs CONFIG_SND_UMP, CONFIG_SND_UMP_LEGACY_RAWMIDI,
> > CONFIG_SND_USB_AUDIO, a MIDI 2.0 gadget or device. KASAN.
> >
> > The natural window is parse-done vs attach. The QEMU PoC widens it
> > with a kprobe on snd_ump_receive (poc/widen_ump.c) plus dummy_hcd
> > configfs midi2, because the un-widened window is short.
> >
> > * Reproduction
> >
> >        # dummy_hcd + configfs usb_gadget midi2.usb0 (see poc/run.sh)
> >        # with widen_ump.ko: force parsed=1, legacy_rmidi=NULL on receive
> >
> > KASAN: null-ptr-deref in snprintf from ump_legacy_set_rawmidi_name
> > <- ump_handle_ep_name_msg <- snd_ump_receive <- input_urb_complete.
> > Then "Fatal exception in interrupt".
> >
> > * Expected
> >
> > legacy_rmidi helpers no-op until attach has stored the pointer.
> >
> > * Actual
> >
> > IRQ-context NULL deref.
> >
> > Please consider the suggested patch
> >
> > Thanks.
> >
> > Suggested patch:
> > ```
> > diff --git a/sound/core/ump.c b/sound/core/ump.c
> > index d183c8a000bd..3d1a2ed3b476 100644
> > --- a/sound/core/ump.c
> > +++ b/sound/core/ump.c
> > @@ -1335,6 +1335,8 @@ static void update_legacy_names(struct
> > snd_ump_endpoint *ump)
> > {
> >     struct snd_rawmidi *rmidi = ump->legacy_rmidi;
> >
> > +    if (!rmidi)
> > +        return;
> >     update_legacy_substreams(ump, rmidi, SNDRV_RAWMIDI_STREAM_INPUT);
> >     update_legacy_substreams(ump, rmidi, SNDRV_RAWMIDI_STREAM_OUTPUT);
> > }
> > @@ -1343,6 +1345,8 @@ static void ump_legacy_set_rawmidi_name(struct
> > snd_ump_endpoint *ump)
> > {
> >     struct snd_rawmidi *rmidi = ump->legacy_rmidi;
> >
> > +    if (!rmidi)
> > +        return;
> >     snprintf(rmidi->name, sizeof(rmidi->name), "%.68s (MIDI 1.0)",
> >          ump->core.name);
> > }
> > ```
>
> Thanks for the report.  The suggested code change looks good.
> Could you submit a patch in the proper format for upstreaming
> (especially with your Signed-off-by tag)?
>
>
> thanks,
>
> Takashi

[-- Attachment #2: 0001-ALSA-ump-do-not-touch-legacy_rmidi-before-it-exists.patch --]
[-- Type: application/x-patch, Size: 1521 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [BUG] ALSA: ump: NULL deref of legacy_rmidi after parse sets parsed
  2026-09-02  6:58   ` Qingyu Zhang
@ 2026-09-02  7:11     ` Takashi Iwai
  0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2026-09-02  7:11 UTC (permalink / raw)
  To: Qingyu Zhang; +Cc: Takashi Iwai, Jaroslav Kysela, linux-sound, linux-kernel

On Wed, 02 Sep 2026 08:58:34 +0200,
Qingyu Zhang wrote:
> 
> Dear Takashi,
> 
> Thank you for your email. The formatted patch with the Signed-off-by
> tag is attached; please check.

The Signed-off-by tag must be with a real name (or a known identity).
Please resubmit with the corrected tag.  And, at best, not as an
attachment to this thread, but a new thread via git-send-email or
such.


thanks,

Takashi

> 
> Yours,
> Ian
> 
> >
> > On Wed, 02 Sep 2026 04:07:30 +0200,
> > Qingyu Zhang wrote:
> > >
> > > Hello,
> > >
> > > ump_legacy_set_rawmidi_name() snprintf()s into ump->legacy_rmidi->name
> > > when ump->parsed is true, but parsed is set at the end of
> > > snd_ump_parse_endpoint() *before* snd_ump_attach_legacy_rawmidi().
> > > A UMP packet in that window NULL-derefs.
> > >
> > > Type: null-pointer dereference
> > >
> > > * Summary
> > >
> > > snd_ump_parse_endpoint() always does:
> > >
> > >        error:
> > >                ump->parsed = true;
> > >                ...
> > >
> > > ump_handle_ep_name_msg():
> > >
> > >        if (ret && ump->parsed) {
> > >                ump_set_rawmidi_name(ump);
> > >                ump_legacy_set_rawmidi_name(ump);  /* rmidi may be NULL */
> > >        }
> > >
> > > ump_legacy_set_rawmidi_name():
> > >
> > >        rmidi = ump->legacy_rmidi;
> > >        snprintf(rmidi->name, ...);   /* no NULL check */
> > >
> > > This runs from snd_ump_receive() on the USB input URB complete path
> > > (in interrupt).
> > >
> > > * Affected
> > >
> > > 37e0e14128e0. Needs CONFIG_SND_UMP, CONFIG_SND_UMP_LEGACY_RAWMIDI,
> > > CONFIG_SND_USB_AUDIO, a MIDI 2.0 gadget or device. KASAN.
> > >
> > > The natural window is parse-done vs attach. The QEMU PoC widens it
> > > with a kprobe on snd_ump_receive (poc/widen_ump.c) plus dummy_hcd
> > > configfs midi2, because the un-widened window is short.
> > >
> > > * Reproduction
> > >
> > >        # dummy_hcd + configfs usb_gadget midi2.usb0 (see poc/run.sh)
> > >        # with widen_ump.ko: force parsed=1, legacy_rmidi=NULL on receive
> > >
> > > KASAN: null-ptr-deref in snprintf from ump_legacy_set_rawmidi_name
> > > <- ump_handle_ep_name_msg <- snd_ump_receive <- input_urb_complete.
> > > Then "Fatal exception in interrupt".
> > >
> > > * Expected
> > >
> > > legacy_rmidi helpers no-op until attach has stored the pointer.
> > >
> > > * Actual
> > >
> > > IRQ-context NULL deref.
> > >
> > > Please consider the suggested patch
> > >
> > > Thanks.
> > >
> > > Suggested patch:
> > > ```
> > > diff --git a/sound/core/ump.c b/sound/core/ump.c
> > > index d183c8a000bd..3d1a2ed3b476 100644
> > > --- a/sound/core/ump.c
> > > +++ b/sound/core/ump.c
> > > @@ -1335,6 +1335,8 @@ static void update_legacy_names(struct
> > > snd_ump_endpoint *ump)
> > > {
> > >     struct snd_rawmidi *rmidi = ump->legacy_rmidi;
> > >
> > > +    if (!rmidi)
> > > +        return;
> > >     update_legacy_substreams(ump, rmidi, SNDRV_RAWMIDI_STREAM_INPUT);
> > >     update_legacy_substreams(ump, rmidi, SNDRV_RAWMIDI_STREAM_OUTPUT);
> > > }
> > > @@ -1343,6 +1345,8 @@ static void ump_legacy_set_rawmidi_name(struct
> > > snd_ump_endpoint *ump)
> > > {
> > >     struct snd_rawmidi *rmidi = ump->legacy_rmidi;
> > >
> > > +    if (!rmidi)
> > > +        return;
> > >     snprintf(rmidi->name, sizeof(rmidi->name), "%.68s (MIDI 1.0)",
> > >          ump->core.name);
> > > }
> > > ```
> >
> > Thanks for the report.  The suggested code change looks good.
> > Could you submit a patch in the proper format for upstreaming
> > (especially with your Signed-off-by tag)?
> >
> >
> > thanks,
> >
> > Takashi
> From: Ian <usupergate@gmail.com>
> Date: Mon, 31 Aug 2026 17:00:00 +0800
> Subject: [PATCH] ALSA: ump: do not touch legacy_rmidi before it exists
> 
> snd_ump_parse_endpoint() sets ump->parsed on every exit, including
> error, before the caller attaches the legacy rawmidi device.
> ump_handle_ep_name_msg() then treats parsed as "legacy_rmidi is live"
> and calls ump_legacy_set_rawmidi_name(), which snprintf()s into
> ump->legacy_rmidi->name. If a UMP packet arrives in that window
> (IRQ path from snd_ump_receive), legacy_rmidi is still NULL
> (KASAN null-ptr-deref in snprintf).
> 
> Guard the legacy helpers. parsed only means endpoint info was
> parsed, not that legacy_rmidi exists.
> 
> Fixes: 37e0e14128e0 ("ALSA: ump: Support UMP Endpoint and Function Block parsing")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ian <usupergate@gmail.com>
> ---
> diff --git a/sound/core/ump.c b/sound/core/ump.c
> index d183c8a000bd..3d1a2ed3b476 100644
> --- a/sound/core/ump.c
> +++ b/sound/core/ump.c
> @@ -1335,6 +1335,8 @@ static void update_legacy_names(struct snd_ump_endpoint *ump)
>  {
>  	struct snd_rawmidi *rmidi = ump->legacy_rmidi;
>  
> +	if (!rmidi)
> +		return;
>  	update_legacy_substreams(ump, rmidi, SNDRV_RAWMIDI_STREAM_INPUT);
>  	update_legacy_substreams(ump, rmidi, SNDRV_RAWMIDI_STREAM_OUTPUT);
>  }
> @@ -1343,6 +1345,8 @@ static void ump_legacy_set_rawmidi_name(struct snd_ump_endpoint *ump)
>  {
>  	struct snd_rawmidi *rmidi = ump->legacy_rmidi;
>  
> +	if (!rmidi)
> +		return;
>  	snprintf(rmidi->name, sizeof(rmidi->name), "%.68s (MIDI 1.0)",
>  		 ump->core.name);
>  }

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-02  7:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-02  2:07 [BUG] ALSA: ump: NULL deref of legacy_rmidi after parse sets parsed Qingyu Zhang
2026-09-02  6:29 ` Takashi Iwai
2026-09-02  6:58   ` Qingyu Zhang
2026-09-02  7:11     ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®