Re: [RFC PATCH v6 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] Race condition between devres unwinding on a failing secondary interface and snd_usb_mixer_disconnect leads to Use-After-Free of `struct topping_master`.
--

commit 5c451ddb148b4a001ba73165f1af998978db8641
Author: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>

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

This adds a component master for the Topping M62's vendor controls to connect
the USB audio interface with the HID interface. The lifecycle context is
stored in devres on the audio control interface.

> diff --git a/sound/usb/mixer_topping.c b/sound/usb/mixer_topping.c
> new file mode 100644
> --- /dev/null
> +++ b/sound/usb/mixer_topping.c
[ ... ]
> +static void topping_master_teardown(void *data)
> +{
> +	struct topping_master *tm = data;
> +
> +	component_master_del(tm->dev, &topping_master_ops);
> +
> +	if (tm->mixer) {
> +		tm->mixer->private_data = NULL;
> +		tm->mixer->private_free = NULL;

[Severity: High]
Is there a missing lock here when clearing the mixer's private fields?

If usb_audio_probe() fails on a secondary interface, devres is synchronously
unwound and topping_master_teardown() runs while the orphaned mixer is still
in chip->mixer_list. If an unbind happens concurrently on the first
interface, snd_usb_mixer_disconnect() can race with this teardown.

Since snd_usb_mixer_disconnect() reads mixer->private_free without holding
register_mutex, modifying these pointers without synchronization introduces a
Time-of-Check to Time-of-Use (TOCTOU) race.

> +	}
> +}
> +
> +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.  The action clears
> +	 * mixer->private_data on its way through, so nothing is left
> +	 * pointing at the storage dropped below.
> +	 */
> +	devm_release_action(tm->dev, topping_master_teardown, tm);

[Severity: High]
Could this dereference tm->dev after tm has already been freed?

Due to the race condition described above, snd_usb_mixer_disconnect() might
read mixer->private_free just before it is cleared by
topping_master_teardown().

If the teardown finishes and devres frees the struct topping_master memory,
topping_private_free() will then be executed with a freed tm pointer,
leading to a Use-After-Free when accessing tm->dev.

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