Re: [RFC PATCH v3 2/2] ALSA: usb-audio: bind the Topping M62's vendor controls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Use-after-free of `tm` devres allocation in `topping_private_free()` when probe fails on a secondary interface.
--

commit 609b467189291cd5e9a1099df6471716a2475cca
Author: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>

ALSA: usb-audio: bind the Topping M62's vendor controls

This commit adds a component master for the Topping M62 vendor controls
to bind the ALSA sound card to the corresponding HID driver. It uses the
component framework to manage the lifetime between the ALSA card and the
HID vendor controls, allocating its context using devres on the audio
control interface.

> diff --git a/sound/usb/mixer_topping.c b/sound/usb/mixer_topping.c
> new file mode 100644
> index 0000000000000..d97d895a4493d
> --- /dev/null
> +++ b/sound/usb/mixer_topping.c
[ ... ]
> +static void topping_private_free(struct usb_mixer_interface *mixer)
> +{
> +	struct topping_master *tm = mixer->private_data;
> +
> +	if (!tm)
> +		return;
> +
> +	/*
> +	 * Reached from snd_usb_mixer_disconnect(), on an unplug and on an
> +	 * unbind of the audio interface alike.  component_master_del() runs
> +	 * topping_master_unbind() on the way, so the HID side has taken its
> +	 * kcontrols off this card before the card is taken apart.
> +	 */
> +	component_master_del(tm->dev, &topping_master_ops);

[Severity: High]
Can this result in a use-after-free of the `tm` structure if probe fails on a
secondary interface?

If a USB device matching the Topping M62 quirk has multiple interfaces, and
the primary interface probes successfully but the secondary interface's probe
fails (e.g., due to ALSA card slots being exhausted or memory allocation
failure), the USB core will clean up the secondary interface's devres, freeing
`tm`.

However, looking at `usb_audio_probe()` in `sound/usb/card.c`:

    if (!chip->num_interfaces)
        snd_card_free(chip->card);

For a secondary interface (`chip->num_interfaces > 0`), the ALSA card and
`chip->mixer_list` are intentionally not freed when probe fails. The mixer
appended during the failed probe remains in the list.

Later, when the primary interface unbinds, `snd_usb_mixer_disconnect()` is
called and invokes this `topping_private_free()` function, which will access
the already freed `tm->dev`.

> +	devres_destroy(tm->dev, topping_master_release, NULL, NULL);
> +	mixer->private_data = NULL;
> +}
> +
> +int snd_topping_init(struct usb_mixer_interface *mixer)
> +{
> +	struct snd_usb_audio *chip = mixer->chip;
> +	struct component_match *match = NULL;
> +	struct usb_interface *intf;
> +	struct topping_master *tm;
> +	struct device *dev;
> +	int err;
[ ... ]
> +	tm = devres_alloc(topping_master_release, sizeof(*tm), GFP_KERNEL);
> +	if (!tm)
> +		return -ENOMEM;
> +	tm->dev = dev;
> +	tm->card = chip->card;
> +	devres_add(dev, tm);
> +
> +	mixer->private_data = tm;
> +	mixer->private_free = topping_private_free;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904144300.529289-1-mikhail.v.gavrilov@gmail.com?part=2