RE: Re: [PATCH] ALSA: usb-audio: Fix NULL pointer dereference in snd_usb_mixer_controls_badd

[Haein Lee]

Thank you for your quick response.

The problem is that `assoc` in snd_usb_create_streams() is a local variable. Even though it’s assigned in the NuForce UDH-100 case, its scope is limited to the function. As a result, when snd_usb_mixer_controls_badd() later assumes that assoc is valid, it actually isn’t.

I was able to reproduce this bug using syzkaller on my machine. I’ll attach the related materials — it’s basically triggered by sending a malicious USB device descriptor.

Personally, I’m not entirely satisfied with the patch, since it still leaves the case where usb_ifnum_to_if(dev, ctrlif)->intf_assoc is NULL even though the BADD profile is valid (as seen in the NuForce UDH-100 case). However, I believe this is the only remaining code of the issue, and adding a dedicated function just for this case would make the code unnecessarily complex.

Please correct me if I’m wrong. If we wrap up this discussion, I’ll send the patch in the proper format.

Thanks,
Haein Lee

-----Original Message-----
From:  "Takashi Iwai" <tiwai@xxxxxxx>
To:     "Haein Lee" <lhi0729@xxxxxxxxxxx>; 
Cc:      <perex@xxxxxxxx>;   <tiwai@xxxxxxxx>;   <linux-sound@xxxxxxxxxxxxxxx>;   <linux-kernel@xxxxxxxxxxxxxxx>; 
Sent:  2025-11-11 (화) 21:48:30 (UTC+09:00)
Subject: Re: [PATCH] ALSA: usb-audio: Fix NULL pointer dereference in snd_usb_mixer_controls_badd

On Tue, 11 Nov 2025 13:35:13 +0100,
Haein Lee wrote:
> 
> 
> In snd_usb_create_streams(), for UAC version 3 devices, the Interface
> Association Descriptor (IAD) is retrieved via usb_ifnum_to_if(). If this
> call fails, a fallback routine attempts to obtain the IAD from the next
> interface and sets a BADD profile. However, snd_usb_mixer_controls_badd()
> assumes the IAD retrieved from usb_ifnum_to_if() is always valid without
> performing a NULL check. This can lead to a NULL pointer dereference when
> usb_ifnum_to_if() fails to find the interface descriptor.
> 
> This patch adds a NULL pointer check after calling usb_ifnum_to_if() in
> snd_usb_mixer_controls_badd() to prevent the dereference.

Practically seen, this won't happen because the NULL intf_assoc is
already checked at snd_usb_create_streams() as a fatal error before
reaching this point.  But it would be still safer to have a sanity
check here, too.

In anyway, the patch seems to have a bad format that can't be
applied.  Tabs and spaces seem corrupted by your mailer.

Could you resubmit with a proper format (and add the information I
mentioned in the above)?


thanks,

Takashi

> ---
>  sound/usb/mixer.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index 6f00e0d52382..72b900505d2c 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -3086,6 +3086,8 @@ static int snd_usb_mixer_controls_badd(struct
> usb_mixer_interface *mixer,
>      int i;
>  
>      assoc = usb_ifnum_to_if(dev, ctrlif)->intf_assoc;
> +    if (!assoc)
> +        return -EINVAL;
>  
>      /* Detect BADD capture/playback channels from AS EP descriptors */
>      for (i = 0; i < assoc->bInterfaceCount; i++) {
> -- 
> 2.34.1
> 
> [mail-r]
>

Attachment: report0
Description: Binary data

Attachment: repro.prog
Description: Binary data

Attachment: repro.cprog
Description: Binary data

Attachment: .config
Description: XML document