Re: [PATCH v7 2/2] ALSA: usb-audio: bind the Topping M62's vendor controls
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-30 21:50:23
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] sound/usb: Use-After-Free in topping_private_free() via TOCTOU race -- commit a868f2097e7f0e27683675ee55b1cecdef99afcf Author: Mikhail Gavrilov [off-list ref] Subject: ALSA: usb-audio: bind the Topping M62's vendor controls This patch introduces a component master for the Topping M62's vendor controls, binding them to the hid driver that speaks the vendor protocol. It utilizes the component framework and devres to manage the lifetime of the controls when either the audio or hid interfaces bind and unbind.
quoted hunk ↗ jump to hunk
diff --git a/sound/usb/mixer_topping.c b/sound/usb/mixer_topping.c new file mode 100644 index 0000000000000..5557bc41f05f0 --- /dev/null +++ b/sound/usb/mixer_topping.c
[ ... ]
quoted hunk ↗ jump to hunk
+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: Critical]
Can this clearing of mixer->private_data happen concurrently with
snd_usb_mixer_disconnect() without synchronization?
If a secondary interface fails to probe after creating a mixer,
usb_audio_probe() returns an error but leaves the mixer on the card's
mixer_list because chip->num_interfaces > 0.
The driver core then unwinds devres for the secondary interface without
holding ALSA's register_mutex.
Concurrently, if the device is unplugged, snd_usb_mixer_disconnect()
iterates the mixers and invokes:
if (mixer->private_free)
mixer->private_free(mixer);
Does this lockless teardown open a time-of-check to time-of-use race window
before the tm context is freed by devres?
quoted hunk ↗ jump to hunk
+ +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: Critical] Could this dereference of tm->dev result in a use-after-free regression? If snd_usb_mixer_disconnect() reads mixer->private_data into tm, but is then preempted by the concurrent devres unwind, tm could be freed before devm_release_action() is called. For example, could this sequence occur: cpu2 (snd_usb_mixer_disconnect) topping_private_free() tm = mixer->private_data; cpu1 (devres unwind) topping_master_teardown() tm->mixer->private_data = NULL; devres frees tm cpu2 (snd_usb_mixer_disconnect) devm_release_action(tm->dev, ...) Is there any synchronization missing here to protect against this? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260930213322.32454-1-mikhail.v.gavrilov@gmail.com?part=2