* [RFC] ALSA: usb-audio: missing PM guards in device mixer paths
@ 2026-08-24 23:03 Will Porter
2026-08-25 8:17 ` Takashi Iwai
2026-08-27 23:21 ` [PATCH 0/6] ALSA: usb-audio: guard device-specific control transfers Will Porter
0 siblings, 2 replies; 9+ messages in thread
From: Will Porter @ 2026-08-24 23:03 UTC (permalink / raw)
To: Takashi Iwai
Cc: Jaroslav Kysela, Geoffrey D. Bennett, Asahi Lina, Chris J Arges,
Nick Kossifidis, Detlef Urban, Roy Vegard Ovesen, fenugrec,
Frederic Popp, Cássio Gabriel, Shipei Qu, linux-sound,
linux-kernel
Hi,
I found user-triggered USB transfers without the snd_usb_lock guard in six
snd-usb-audio mixer files. The affected paths include ALSA controls in all six
files. mixer_scarlett2.c and fcp.c also have unguarded hwdep operations.
I would appreciate guidance on three questions:
* Should the control callbacks take snd_usb_lock and preserve their current
fresh-read behavior, even when polling prevents runtime suspend?
* How much of each synchronous hwdep operation should the shutdown and PM
reference cover, especially around userspace copies and protocol waits?
* Should Scarlett2 flash erase hold a runtime-PM reference until completion,
or should runtime suspend return -EBUSY while erase is active?
I checked sound.git for-next at e72d5659a260. Representative control paths
that reach the device are:
mixer_s1810c.c snd_s1810c_switch_get
-> snd_s1810c_get_switch_state
-> snd_sc1810c_get_status_field -> snd_usb_ctl_msg
mixer_us16x08.c snd_us16x08_meter_get
-> snd_us16x08_send_urb/snd_us16x08_recv_urb
-> snd_usb_ctl_msg
mixer_scarlett.c scarlett_ctl_meter_get -> snd_usb_ctl_msg
mixer_scarlett2.c scarlett2_meter_ctl_get
-> scarlett2_usb_get_meter_levels -> scarlett2_usb
-> scarlett2_usb_tx -> snd_usb_ctl_msg
mixer_quirks.c snd_rme_digiface_rate_get
-> snd_rme_digiface_get_status_val
-> snd_rme_digiface_read_status -> snd_usb_ctl_msg
fcp.c fcp_meter_ctl_get -> fcp_usb -> fcp_usb_tx
-> snd_usb_ctl_msg
I scoped this audit to user-triggered control and hwdep paths in the
snd-usb-audio mixer files. Other sound/usb subdrivers are outside this RFC.
The RME Digiface get and put callbacks in mixer_quirks.c share this omission.
Most other direct control transfers in that file already take the guard.
The Scarlett2 hwdep read and write callbacks call scarlett2_usb(). Several
ioctls also reach that transport. These include reboot, erase, erase-progress,
and segment selection when an erase is pending. scarlett2_hwdep_open() can
also poll erase progress. The FCP FCP_IOCTL_INIT and FCP_IOCTL_CMD operations
transfer data through snd_usb_ctl_msg(). The other FCP hwdep operations do not
transfer USB data.
The Studio 1810c, Scarlett2, and FCP drivers use private mutexes to serialize
their protocols. mixer_us16x08.c uses chip->mutex. Those locks do not acquire
a runtime-PM reference. The Scarlett Gen 1 meter and RME Digiface helpers do
not use a protocol mutex around their single control requests.
The common mixer paths do take the PM guard. get_ctl_value_v1() and the v2
path in mixer.c use CLASS(snd_usb_lock, pm)(chip) before snd_usb_ctl_msg().
Many mixer_quirks.c callbacks do the same. The Forte callbacks in
mixer_scarlett.c use the older explicit snd_usb_lock_shutdown() form, but the
Scarlett Gen 1 meter callback does not.
Failure mechanism
-----------------
When the USB device enters runtime suspend, usb_suspend_both() clears
udev->can_submit. An unguarded callback then reaches snd_usb_ctl_msg() without
calling snd_usb_autoresume(). The submission reaches usb_hcd_link_urb_to_ep(),
which returns -EHOSTUNREACH for !urb->dev->can_submit.
The visible result depends on the driver:
* Studio 1810c control reads return the transfer error. Its write callback
can instead report no change after a failed transfer.
* The Scarlett Gen 1 meter returns the transfer error.
* The RME Digiface callbacks return the transfer error and log it. Its
VOLATILE status controls can repeat that log on each poll.
* The Scarlett2 and FCP transports log the transfer error and convert it to
-EINVAL. Their VOLATILE meter controls can repeat that log on each poll.
* The TASCAM meter callback ignores both send and receive results. It parses
a zero-filled receive buffer, advances its four-step polling sequence, and
returns the persistent meter store as a successful read. Because the zero
buffer has no valid packet signature, this usually returns stale values.
usb_audio_driver supports autosuspend. I did not find the affected USB IDs in
the entries that set QUIRK_FLAG_DISABLE_AUTOSUSPEND. Practical reachability
still depends on the complete USB device and its userspace. A sibling
interface or a polling process can keep the device active.
Hardware reproduction of the mechanism
---------------------------------------
I do not own hardware for these in-tree paths. I reproduced the mechanism
with an out-of-tree Audient iD14 MkI mixer extension that had the same missing
guard. I read an unguarded device-specific control and a guarded mixer.c
control in one runtime-suspended window:
unguarded Audient control -> "No route to host", no value,
device remained suspended
guarded mixer.c control -> value returned, device became active
After I added CLASS(snd_usb_lock, pm)(chip) to the Audient callback, that
callback returned a value and resumed the device. This test demonstrates the
PM mechanism. It does not test an affected in-tree device.
The iD14 did not reach runtime suspend in its normal desktop configuration.
Its HID interface exposes the monitor knob as a mouse, and the desktop keeps
that input node open. usbhid then holds a runtime-PM reference for the USB
device. PipeWire also polls the control device. For the test, I stopped the
userspace audio stack, unbound the HID interface, and set power/control to
auto. Whether an equivalent masking condition exists on the affected devices
is unknown.
Prior design history
--------------------
Commit 47ab15459382 ("ALSA: usb-audio: Avoid nested autoresume calls")
introduced snd_usb_lock_shutdown(). Its stated purposes include autoresume and
delaying disconnect cleanup until concurrent operations finish. The current
guard is the automatic-cleanup form of that helper.
During review of the original FCP driver, Takashi asked whether its hwdep and
control paths needed suspend, resume, and disconnect handling:
https://lore.kernel.org/linux-sound/87jzbidfs5.wl-tiwai@suse.de/
The v2 response reported system-suspend and disconnect tests while polling
meters 20 times per second:
https://lore.kernel.org/linux-sound/cover.1735495639.git.g@b4.vu/
Later revisions added private_suspend cleanup for the notification URB. I did
not find discussion of runtime-autosuspend references for the synchronous
control transfers, or shutdown accounting for the callbacks that issue them.
Guard scope
-----------
The likely control-path fix is:
CLASS(snd_usb_lock, pm)(chip);
if (pm.err < 0)
return -EIO;
The PM guard should precede the protocol mutex. This order matches the common
mixer paths and avoids a lock-order inversion if autoresume invokes a mixer
resume hook. Each driver still needs a lock-order audit. Callbacks that only
read a software shadow do not need the guard.
Existing code provides a policy precedent for meter and status reads. The
VOLATILE RME class-compliant status controls were added by commit d39f1d68fe1d
("ALSA: usb-audio: Add custom mixer status quirks for RME CC devices"). Those
controls take the guard around fresh device reads. Following that precedent,
the affected callbacks should preserve their current fresh-data behavior and
wake a suspended device. A polling mixer GUI can therefore keep the device
active. Cached behavior would require a separate cache and update design.
Omitting the guard does not provide valid cached behavior.
The hwdep paths need shutdown and PM accounting from the first use of
disconnect-sensitive driver state through the complete device transaction.
Input data may be copied before taking the guard only if that work does not
dereference private mixer data. A locally buffered response may be copied out
after releasing it. Otherwise, the reference must cover the userspace copy.
It must also remain held across protocol sleeps and command-response waits.
Scarlett2 flash erase is asynchronous across file operations. USB PM guidance
for asynchronous output holds a PM reference from submission until the output
queue drains:
https://docs.kernel.org/driver-api/usb/power-management.html
By analogy, runtime PM should remain referenced from erase submission through
completion. Alternatively, the runtime-suspend path should return -EBUSY while
an erase is active. A scoped guard in one ioctl does not cover that interval.
A runtime-PM reference does not prevent system suspend, which remains a
separate policy question. I have not reduced the hwdep changes to patches
because this operation boundary needs review first.
The missing usage_count reference also means usb_audio_disconnect() does not
wait for these transfers through snd_refcount_sync(). Other ALSA lifetime
rules may protect some control or hwdep paths. I have not completed that
lifetime audit, so I am not making a memory-safety claim here.
If this scope and guard placement look correct, I can prepare patches for the
six files. I can compile-test them, but I cannot test them on the affected
hardware. Tests from the device owners would be valuable.
Assisted-by: Claude:claude-opus-5
Assisted-by: Antigravity:gemini-3.1-pro-high
Assisted-by: Codex:gpt-5.6-sol
Thanks,
Will Porter
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [RFC] ALSA: usb-audio: missing PM guards in device mixer paths 2026-08-24 23:03 [RFC] ALSA: usb-audio: missing PM guards in device mixer paths Will Porter @ 2026-08-25 8:17 ` Takashi Iwai 2026-08-27 23:21 ` [PATCH 0/6] ALSA: usb-audio: guard device-specific control transfers Will Porter 1 sibling, 0 replies; 9+ messages in thread From: Takashi Iwai @ 2026-08-25 8:17 UTC (permalink / raw) To: Will Porter Cc: Takashi Iwai, Jaroslav Kysela, Geoffrey D. Bennett, Asahi Lina, Chris J Arges, Nick Kossifidis, Detlef Urban, Roy Vegard Ovesen, fenugrec, Frederic Popp, Cássio Gabriel, Shipei Qu, linux-sound, linux-kernel On Tue, 25 Aug 2026 01:03:02 +0200, Will Porter wrote: > > Hi, > > I found user-triggered USB transfers without the snd_usb_lock guard in six > snd-usb-audio mixer files. The affected paths include ALSA controls in all six > files. mixer_scarlett2.c and fcp.c also have unguarded hwdep operations. > > I would appreciate guidance on three questions: > > * Should the control callbacks take snd_usb_lock and preserve their current > fresh-read behavior, even when polling prevents runtime suspend? > * How much of each synchronous hwdep operation should the shutdown and PM > reference cover, especially around userspace copies and protocol waits? > * Should Scarlett2 flash erase hold a runtime-PM reference until completion, > or should runtime suspend return -EBUSY while erase is active? > > I checked sound.git for-next at e72d5659a260. Representative control paths > that reach the device are: > > mixer_s1810c.c snd_s1810c_switch_get > -> snd_s1810c_get_switch_state > -> snd_sc1810c_get_status_field -> snd_usb_ctl_msg > mixer_us16x08.c snd_us16x08_meter_get > -> snd_us16x08_send_urb/snd_us16x08_recv_urb > -> snd_usb_ctl_msg > mixer_scarlett.c scarlett_ctl_meter_get -> snd_usb_ctl_msg > mixer_scarlett2.c scarlett2_meter_ctl_get > -> scarlett2_usb_get_meter_levels -> scarlett2_usb > -> scarlett2_usb_tx -> snd_usb_ctl_msg > mixer_quirks.c snd_rme_digiface_rate_get > -> snd_rme_digiface_get_status_val > -> snd_rme_digiface_read_status -> snd_usb_ctl_msg > fcp.c fcp_meter_ctl_get -> fcp_usb -> fcp_usb_tx > -> snd_usb_ctl_msg > > I scoped this audit to user-triggered control and hwdep paths in the > snd-usb-audio mixer files. Other sound/usb subdrivers are outside this RFC. > > The RME Digiface get and put callbacks in mixer_quirks.c share this omission. > Most other direct control transfers in that file already take the guard. > > The Scarlett2 hwdep read and write callbacks call scarlett2_usb(). Several > ioctls also reach that transport. These include reboot, erase, erase-progress, > and segment selection when an erase is pending. scarlett2_hwdep_open() can > also poll erase progress. The FCP FCP_IOCTL_INIT and FCP_IOCTL_CMD operations > transfer data through snd_usb_ctl_msg(). The other FCP hwdep operations do not > transfer USB data. > > The Studio 1810c, Scarlett2, and FCP drivers use private mutexes to serialize > their protocols. mixer_us16x08.c uses chip->mutex. Those locks do not acquire > a runtime-PM reference. The Scarlett Gen 1 meter and RME Digiface helpers do > not use a protocol mutex around their single control requests. > > The common mixer paths do take the PM guard. get_ctl_value_v1() and the v2 > path in mixer.c use CLASS(snd_usb_lock, pm)(chip) before snd_usb_ctl_msg(). > Many mixer_quirks.c callbacks do the same. The Forte callbacks in > mixer_scarlett.c use the older explicit snd_usb_lock_shutdown() form, but the > Scarlett Gen 1 meter callback does not. > > Failure mechanism > ----------------- > > When the USB device enters runtime suspend, usb_suspend_both() clears > udev->can_submit. An unguarded callback then reaches snd_usb_ctl_msg() without > calling snd_usb_autoresume(). The submission reaches usb_hcd_link_urb_to_ep(), > which returns -EHOSTUNREACH for !urb->dev->can_submit. > > The visible result depends on the driver: > > * Studio 1810c control reads return the transfer error. Its write callback > can instead report no change after a failed transfer. > * The Scarlett Gen 1 meter returns the transfer error. > * The RME Digiface callbacks return the transfer error and log it. Its > VOLATILE status controls can repeat that log on each poll. > * The Scarlett2 and FCP transports log the transfer error and convert it to > -EINVAL. Their VOLATILE meter controls can repeat that log on each poll. > * The TASCAM meter callback ignores both send and receive results. It parses > a zero-filled receive buffer, advances its four-step polling sequence, and > returns the persistent meter store as a successful read. Because the zero > buffer has no valid packet signature, this usually returns stale values. > > usb_audio_driver supports autosuspend. I did not find the affected USB IDs in > the entries that set QUIRK_FLAG_DISABLE_AUTOSUSPEND. Practical reachability > still depends on the complete USB device and its userspace. A sibling > interface or a polling process can keep the device active. > > Hardware reproduction of the mechanism > --------------------------------------- > > I do not own hardware for these in-tree paths. I reproduced the mechanism > with an out-of-tree Audient iD14 MkI mixer extension that had the same missing > guard. I read an unguarded device-specific control and a guarded mixer.c > control in one runtime-suspended window: > > unguarded Audient control -> "No route to host", no value, > device remained suspended > guarded mixer.c control -> value returned, device became active > > After I added CLASS(snd_usb_lock, pm)(chip) to the Audient callback, that > callback returned a value and resumed the device. This test demonstrates the > PM mechanism. It does not test an affected in-tree device. > > The iD14 did not reach runtime suspend in its normal desktop configuration. > Its HID interface exposes the monitor knob as a mouse, and the desktop keeps > that input node open. usbhid then holds a runtime-PM reference for the USB > device. PipeWire also polls the control device. For the test, I stopped the > userspace audio stack, unbound the HID interface, and set power/control to > auto. Whether an equivalent masking condition exists on the affected devices > is unknown. > > Prior design history > -------------------- > > Commit 47ab15459382 ("ALSA: usb-audio: Avoid nested autoresume calls") > introduced snd_usb_lock_shutdown(). Its stated purposes include autoresume and > delaying disconnect cleanup until concurrent operations finish. The current > guard is the automatic-cleanup form of that helper. > > During review of the original FCP driver, Takashi asked whether its hwdep and > control paths needed suspend, resume, and disconnect handling: > > https://lore.kernel.org/linux-sound/87jzbidfs5.wl-tiwai@suse.de/ > > The v2 response reported system-suspend and disconnect tests while polling > meters 20 times per second: > > https://lore.kernel.org/linux-sound/cover.1735495639.git.g@b4.vu/ > > Later revisions added private_suspend cleanup for the notification URB. I did > not find discussion of runtime-autosuspend references for the synchronous > control transfers, or shutdown accounting for the callbacks that issue them. > > Guard scope > ----------- > > The likely control-path fix is: > > CLASS(snd_usb_lock, pm)(chip); > if (pm.err < 0) > return -EIO; > > The PM guard should precede the protocol mutex. This order matches the common > mixer paths and avoids a lock-order inversion if autoresume invokes a mixer > resume hook. Each driver still needs a lock-order audit. Callbacks that only > read a software shadow do not need the guard. > > Existing code provides a policy precedent for meter and status reads. The > VOLATILE RME class-compliant status controls were added by commit d39f1d68fe1d > ("ALSA: usb-audio: Add custom mixer status quirks for RME CC devices"). Those > controls take the guard around fresh device reads. Following that precedent, > the affected callbacks should preserve their current fresh-data behavior and > wake a suspended device. A polling mixer GUI can therefore keep the device > active. Cached behavior would require a separate cache and update design. > Omitting the guard does not provide valid cached behavior. > > The hwdep paths need shutdown and PM accounting from the first use of > disconnect-sensitive driver state through the complete device transaction. > Input data may be copied before taking the guard only if that work does not > dereference private mixer data. A locally buffered response may be copied out > after releasing it. Otherwise, the reference must cover the userspace copy. > It must also remain held across protocol sleeps and command-response waits. > > Scarlett2 flash erase is asynchronous across file operations. USB PM guidance > for asynchronous output holds a PM reference from submission until the output > queue drains: > > https://docs.kernel.org/driver-api/usb/power-management.html > > By analogy, runtime PM should remain referenced from erase submission through > completion. Alternatively, the runtime-suspend path should return -EBUSY while > an erase is active. A scoped guard in one ioctl does not cover that interval. > A runtime-PM reference does not prevent system suspend, which remains a > separate policy question. I have not reduced the hwdep changes to patches > because this operation boundary needs review first. > > The missing usage_count reference also means usb_audio_disconnect() does not > wait for these transfers through snd_refcount_sync(). Other ALSA lifetime > rules may protect some control or hwdep paths. I have not completed that > lifetime audit, so I am not making a memory-safety claim here. > > If this scope and guard placement look correct, I can prepare patches for the > six files. I can compile-test them, but I cannot test them on the affected > hardware. Tests from the device owners would be valuable. > > Assisted-by: Claude:claude-opus-5 > Assisted-by: Antigravity:gemini-3.1-pro-high > Assisted-by: Codex:gpt-5.6-sol > > Thanks, > Will Porter In general, the code accessing the hardware via usb_ctl_msg() should be covered by snd_usb_lock. In the case of scarlett2, protecting scarlett2_usb() alone would cover most cases, I guess. thanks, Takashi ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/6] ALSA: usb-audio: guard device-specific control transfers 2026-08-24 23:03 [RFC] ALSA: usb-audio: missing PM guards in device mixer paths Will Porter 2026-08-25 8:17 ` Takashi Iwai @ 2026-08-27 23:21 ` Will Porter 2026-08-27 23:21 ` [PATCH 1/6] ALSA: usb-audio: Add PM guard to Studio 1810c controls Will Porter ` (5 more replies) 1 sibling, 6 replies; 9+ messages in thread From: Will Porter @ 2026-08-27 23:21 UTC (permalink / raw) To: Takashi Iwai Cc: Jaroslav Kysela, Geoffrey D. Bennett, Asahi Lina, Chris J Arges, Nick Kossifidis, Detlef Urban, Roy Vegard Ovesen, fenugrec, Frederic Popp, Cássio Gabriel, Shipei Qu, linux-sound, linux-kernel, Will Porter Takashi confirmed that hardware access through usb_ctl_msg() should hold snd_usb_lock, and suggested guarding scarlett2_usb() as the central point for Scarlett2. This series adds that protection to the six device-specific paths identified by the RFC audit. Each guard covers a complete synchronous USB transaction. The guards are outside private data and USB mutexes when a resume callback could affect lock ordering. FCP holds one reference across its complete initialization sequence so runtime suspend cannot intervene between step zero, notification-URB setup, and the two initialization commands. Scarlett2 uses a dedicated unguarded helper only for the suspend-time config save. That call already runs inside the USB suspend callback. The guarded path covers normal controls and hwdep operations. The series does not claim to hold a PM reference across Scarlett2's asynchronous flash-erase interval. The affected devices were unavailable for runtime testing. I reproduced the failure mechanism with an Audient iD14 MkI mixer path before the RFC. I then built the complete sound/usb/ directory with the configuration from the running x86-64 distribution kernel (7.2.0-ogc4.1.fc44.x86_64): make olddefconfig make -j4 W=1 sound/usb/ All six changed objects compiled without warnings, and the build linked sound/usb/snd-usb-audio.o. The series also passes scripts/checkpatch.pl --strict with no errors, warnings, or checks. Link: https://lore.kernel.org/r/20260824230302.27965-1-mrwillporter@gmail.com Will Porter (6): ALSA: usb-audio: Add PM guard to Studio 1810c controls ALSA: usb-audio: Add PM guards to US-16x08 transfers ALSA: usb-audio: Add PM guard to Scarlett meter reads ALSA: usb-audio: Guard Scarlett2 protocol transfers ALSA: usb-audio: Add PM guards to RME Digiface controls ALSA: usb-audio: Guard FCP protocol transfers sound/usb/fcp.c | 8 ++++++++ sound/usb/mixer_quirks.c | 8 ++++++++ sound/usb/mixer_s1810c.c | 8 ++++++++ sound/usb/mixer_scarlett.c | 4 ++++ sound/usb/mixer_scarlett2.c | 36 ++++++++++++++++++++++++++++++------ sound/usb/mixer_us16x08.c | 7 +++++++ 6 files changed, 65 insertions(+), 6 deletions(-) base-commit: 58c1c30074a8d1179e17a0188cd695b2f416bf75 -- 2.47.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/6] ALSA: usb-audio: Add PM guard to Studio 1810c controls 2026-08-27 23:21 ` [PATCH 0/6] ALSA: usb-audio: guard device-specific control transfers Will Porter @ 2026-08-27 23:21 ` Will Porter 2026-08-27 23:21 ` [PATCH 2/6] ALSA: usb-audio: Add PM guards to US-16x08 transfers Will Porter ` (4 subsequent siblings) 5 siblings, 0 replies; 9+ messages in thread From: Will Porter @ 2026-08-27 23:21 UTC (permalink / raw) To: Takashi Iwai Cc: Jaroslav Kysela, Geoffrey D. Bennett, Asahi Lina, Chris J Arges, Nick Kossifidis, Detlef Urban, Roy Vegard Ovesen, fenugrec, Frederic Popp, Cássio Gabriel, Shipei Qu, linux-sound, linux-kernel, Will Porter The Studio 1810c control callbacks issue vendor transfers without preventing runtime suspend or disconnect. A transfer attempted after runtime suspend can fail because the USB device cannot accept submissions. Take snd_usb_lock before the driver data and USB mutexes in both callbacks. This resumes the device before either the state read or control write. It also prevents disconnect cleanup from racing the complete operation. Assisted-by: Codex:gpt-5.6-sol Signed-off-by: Will Porter <mrwillporter@gmail.com> --- sound/usb/mixer_s1810c.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sound/usb/mixer_s1810c.c b/sound/usb/mixer_s1810c.c index 2e5a8d37ec57..bdb5e3aaff3b 100644 --- a/sound/usb/mixer_s1810c.c +++ b/sound/usb/mixer_s1810c.c @@ -474,6 +474,10 @@ snd_s1810c_switch_get(struct snd_kcontrol *kctl, u32 state = 0; int ret; + CLASS(snd_usb_lock, pm)(mixer->chip); + if (pm.err < 0) + return -EIO; + guard(mutex)(&private->data_mutex); ret = snd_s1810c_get_switch_state(mixer, kctl, &state); if (ret < 0) @@ -504,6 +508,10 @@ snd_s1810c_switch_set(struct snd_kcontrol *kctl, u32 newval = 0; int ret = 0; + CLASS(snd_usb_lock, pm)(mixer->chip); + if (pm.err < 0) + return -EIO; + guard(mutex)(&private->data_mutex); ret = snd_s1810c_get_switch_state(mixer, kctl, &curval); if (ret < 0) -- 2.47.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/6] ALSA: usb-audio: Add PM guards to US-16x08 transfers 2026-08-27 23:21 ` [PATCH 0/6] ALSA: usb-audio: guard device-specific control transfers Will Porter 2026-08-27 23:21 ` [PATCH 1/6] ALSA: usb-audio: Add PM guard to Studio 1810c controls Will Porter @ 2026-08-27 23:21 ` Will Porter 2026-08-27 23:21 ` [PATCH 3/6] ALSA: usb-audio: Add PM guard to Scarlett meter reads Will Porter ` (3 subsequent siblings) 5 siblings, 0 replies; 9+ messages in thread From: Will Porter @ 2026-08-27 23:21 UTC (permalink / raw) To: Takashi Iwai Cc: Jaroslav Kysela, Geoffrey D. Bennett, Asahi Lina, Chris J Arges, Nick Kossifidis, Detlef Urban, Roy Vegard Ovesen, fenugrec, Frederic Popp, Cássio Gabriel, Shipei Qu, linux-sound, linux-kernel, Will Porter The TASCAM control helpers submit vendor requests without preventing runtime suspend or disconnect. This affects mixer writes and the volatile meter path. Protect both send and receive helpers with snd_usb_lock. Acquire the PM guard before chip->mutex in the receive path so autoresume cannot invert the mutex order. Assisted-by: Codex:gpt-5.6-sol Signed-off-by: Will Porter <mrwillporter@gmail.com> --- sound/usb/mixer_us16x08.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sound/usb/mixer_us16x08.c b/sound/usb/mixer_us16x08.c index ebff185cbd2c..14fb1ad764a7 100644 --- a/sound/usb/mixer_us16x08.c +++ b/sound/usb/mixer_us16x08.c @@ -151,6 +151,9 @@ static const char *const route_names[] = { static int snd_us16x08_recv_urb(struct snd_usb_audio *chip, unsigned char *buf, int size) { + CLASS(snd_usb_lock, pm)(chip); + if (pm.err < 0) + return -EIO; guard(mutex)(&chip->mutex); snd_usb_ctl_msg(chip->dev, @@ -165,6 +168,10 @@ static int snd_us16x08_recv_urb(struct snd_usb_audio *chip, */ static int snd_us16x08_send_urb(struct snd_usb_audio *chip, char *buf, int size) { + CLASS(snd_usb_lock, pm)(chip); + if (pm.err < 0) + return -EIO; + return snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0), SND_US16X08_URB_REQUEST, SND_US16X08_URB_REQUESTTYPE, 0, 0, buf, size); -- 2.47.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/6] ALSA: usb-audio: Add PM guard to Scarlett meter reads 2026-08-27 23:21 ` [PATCH 0/6] ALSA: usb-audio: guard device-specific control transfers Will Porter 2026-08-27 23:21 ` [PATCH 1/6] ALSA: usb-audio: Add PM guard to Studio 1810c controls Will Porter 2026-08-27 23:21 ` [PATCH 2/6] ALSA: usb-audio: Add PM guards to US-16x08 transfers Will Porter @ 2026-08-27 23:21 ` Will Porter 2026-08-27 23:21 ` [PATCH 4/6] ALSA: usb-audio: Guard Scarlett2 protocol transfers Will Porter ` (2 subsequent siblings) 5 siblings, 0 replies; 9+ messages in thread From: Will Porter @ 2026-08-27 23:21 UTC (permalink / raw) To: Takashi Iwai Cc: Jaroslav Kysela, Geoffrey D. Bennett, Asahi Lina, Chris J Arges, Nick Kossifidis, Detlef Urban, Roy Vegard Ovesen, fenugrec, Frederic Popp, Cássio Gabriel, Shipei Qu, linux-sound, linux-kernel, Will Porter The Scarlett Gen 1 meter callback reads the device without preventing runtime suspend or disconnect. The transfer can fail when userspace polls the volatile control after the device suspends. Hold snd_usb_lock across the meter request. This matches the guarded Forte and common mixer control paths in this file. Assisted-by: Codex:gpt-5.6-sol Signed-off-by: Will Porter <mrwillporter@gmail.com> --- sound/usb/mixer_scarlett.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sound/usb/mixer_scarlett.c b/sound/usb/mixer_scarlett.c index 673eb8d8724d..369968565c19 100644 --- a/sound/usb/mixer_scarlett.c +++ b/sound/usb/mixer_scarlett.c @@ -707,6 +707,10 @@ static int scarlett_ctl_meter_get(struct snd_kcontrol *kctl, int idx = snd_usb_ctrl_intf(elem->head.mixer->hostif) | (elem->head.id << 8); int err; + CLASS(snd_usb_lock, pm)(chip); + if (pm.err < 0) + return -EIO; + err = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), UAC2_CS_MEM, -- 2.47.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/6] ALSA: usb-audio: Guard Scarlett2 protocol transfers 2026-08-27 23:21 ` [PATCH 0/6] ALSA: usb-audio: guard device-specific control transfers Will Porter ` (2 preceding siblings ...) 2026-08-27 23:21 ` [PATCH 3/6] ALSA: usb-audio: Add PM guard to Scarlett meter reads Will Porter @ 2026-08-27 23:21 ` Will Porter 2026-08-27 23:21 ` [PATCH 5/6] ALSA: usb-audio: Add PM guards to RME Digiface controls Will Porter 2026-08-27 23:21 ` [PATCH 6/6] ALSA: usb-audio: Guard FCP protocol transfers Will Porter 5 siblings, 0 replies; 9+ messages in thread From: Will Porter @ 2026-08-27 23:21 UTC (permalink / raw) To: Takashi Iwai Cc: Jaroslav Kysela, Geoffrey D. Bennett, Asahi Lina, Chris J Arges, Nick Kossifidis, Detlef Urban, Roy Vegard Ovesen, fenugrec, Frederic Popp, Cássio Gabriel, Shipei Qu, linux-sound, linux-kernel, Will Porter Scarlett2 controls and hwdep operations reach the proprietary USB transport without preventing runtime suspend or disconnect. Protect the central request-and-response helper. One reference then covers the command, acknowledgment wait, and response. The runtime and system resume hook submits only the notification URB. It takes no protocol mutex. Thus, the transport guard does not invert the existing data_mutex or usb_mutex order. Keep the suspend-time config save on the unguarded helper because it runs inside the USB suspend callback. This change protects each USB transaction. It does not hold a runtime-PM reference across the asynchronous flash-erase interval. Assisted-by: Codex:gpt-5.6-sol Signed-off-by: Will Porter <mrwillporter@gmail.com> --- sound/usb/mixer_scarlett2.c | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/sound/usb/mixer_scarlett2.c b/sound/usb/mixer_scarlett2.c index 502854cc9f9f..ed5fe746d438 100644 --- a/sound/usb/mixer_scarlett2.c +++ b/sound/usb/mixer_scarlett2.c @@ -2603,9 +2603,9 @@ static int scarlett2_usb_rx(struct usb_device *dev, int interface, } /* Send a proprietary format request to the Scarlett interface */ -static int scarlett2_usb( - struct usb_mixer_interface *mixer, u32 cmd, - void *req_data, u16 req_size, void *resp_data, u16 resp_size) +static int scarlett2_usb_nopm(struct usb_mixer_interface *mixer, u32 cmd, + void *req_data, u16 req_size, + void *resp_data, u16 resp_size) { struct scarlett2_data *private = mixer->private_data; struct usb_device *dev = mixer->chip->dev; @@ -2713,6 +2713,18 @@ static int scarlett2_usb( return err; } +static int scarlett2_usb(struct usb_mixer_interface *mixer, u32 cmd, + void *req_data, u16 req_size, + void *resp_data, u16 resp_size) +{ + CLASS(snd_usb_lock, pm)(mixer->chip); + if (pm.err < 0) + return -EIO; + + return scarlett2_usb_nopm(mixer, cmd, req_data, req_size, + resp_data, resp_size); +} + /* Send a USB message to get data; result placed in *buf */ static int scarlett2_usb_get( struct usb_mixer_interface *mixer, @@ -3020,9 +3032,21 @@ static int scarlett2_usb_set_config_buf( /* Send SCARLETT2_USB_DATA_CMD SCARLETT2_USB_CONFIG_SAVE */ static void scarlett2_config_save(struct usb_mixer_interface *mixer) { - int err; + __le32 req = cpu_to_le32(SCARLETT2_USB_CONFIG_SAVE); + int err = scarlett2_usb(mixer, SCARLETT2_USB_DATA_CMD, + &req, sizeof(req), NULL, 0); + + if (err < 0) + usb_audio_err(mixer->chip, "config save failed: %d\n", err); +} + +/* The USB suspend callback must not acquire another PM reference. */ +static void scarlett2_config_save_nopm(struct usb_mixer_interface *mixer) +{ + __le32 req = cpu_to_le32(SCARLETT2_USB_CONFIG_SAVE); + int err = scarlett2_usb_nopm(mixer, SCARLETT2_USB_DATA_CMD, + &req, sizeof(req), NULL, 0); - err = scarlett2_usb_activate_config(mixer, SCARLETT2_USB_CONFIG_SAVE); if (err < 0) usb_audio_err(mixer->chip, "config save failed: %d\n", err); } @@ -8639,7 +8663,7 @@ static void scarlett2_private_suspend(struct usb_mixer_interface *mixer) struct scarlett2_data *private = mixer->private_data; if (cancel_delayed_work_sync(&private->work)) - scarlett2_config_save(private->mixer); + scarlett2_config_save_nopm(private->mixer); scarlett2_cleanup_urb(mixer); } -- 2.47.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 5/6] ALSA: usb-audio: Add PM guards to RME Digiface controls 2026-08-27 23:21 ` [PATCH 0/6] ALSA: usb-audio: guard device-specific control transfers Will Porter ` (3 preceding siblings ...) 2026-08-27 23:21 ` [PATCH 4/6] ALSA: usb-audio: Guard Scarlett2 protocol transfers Will Porter @ 2026-08-27 23:21 ` Will Porter 2026-08-27 23:21 ` [PATCH 6/6] ALSA: usb-audio: Guard FCP protocol transfers Will Porter 5 siblings, 0 replies; 9+ messages in thread From: Will Porter @ 2026-08-27 23:21 UTC (permalink / raw) To: Takashi Iwai Cc: Jaroslav Kysela, Geoffrey D. Bennett, Asahi Lina, Chris J Arges, Nick Kossifidis, Detlef Urban, Roy Vegard Ovesen, fenugrec, Frederic Popp, Cássio Gabriel, Shipei Qu, linux-sound, linux-kernel, Will Porter The RME Digiface status and register helpers issue vendor requests without preventing runtime suspend or disconnect. The volatile status controls can repeatedly reach these unguarded paths while userspace polls them. Protect both helpers with snd_usb_lock. All Digiface get and put callbacks then resume the device and hold the disconnect reference across their transfer. Assisted-by: Codex:gpt-5.6-sol Signed-off-by: Will Porter <mrwillporter@gmail.com> --- sound/usb/mixer_quirks.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c index a1f5592cc5d5..fc622eb95dc5 100644 --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -3480,6 +3480,10 @@ static int snd_rme_digiface_write_reg(struct snd_kcontrol *kcontrol, int item, u struct usb_device *dev = chip->dev; int err; + CLASS(snd_usb_lock, pm)(chip); + if (pm.err < 0) + return -EIO; + err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), item, USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE, @@ -3499,6 +3503,10 @@ static int snd_rme_digiface_read_status(struct snd_kcontrol *kcontrol, u32 statu __le32 buf[4] = {}; int err; + CLASS(snd_usb_lock, pm)(chip); + if (pm.err < 0) + return -EIO; + err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), RME_DIGIFACE_READ_STATUS, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, -- 2.47.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 6/6] ALSA: usb-audio: Guard FCP protocol transfers 2026-08-27 23:21 ` [PATCH 0/6] ALSA: usb-audio: guard device-specific control transfers Will Porter ` (4 preceding siblings ...) 2026-08-27 23:21 ` [PATCH 5/6] ALSA: usb-audio: Add PM guards to RME Digiface controls Will Porter @ 2026-08-27 23:21 ` Will Porter 5 siblings, 0 replies; 9+ messages in thread From: Will Porter @ 2026-08-27 23:21 UTC (permalink / raw) To: Takashi Iwai Cc: Jaroslav Kysela, Geoffrey D. Bennett, Asahi Lina, Chris J Arges, Nick Kossifidis, Detlef Urban, Roy Vegard Ovesen, fenugrec, Frederic Popp, Cássio Gabriel, Shipei Qu, linux-sound, linux-kernel, Will Porter FCP meter and hwdep operations issue control transfers without preventing runtime suspend or disconnect. Protect the central request-and-response transport. One reference then covers the command, acknowledgment wait, and response. The initial step-zero request bypasses that transport. Hold an outer reference across the complete initialization sequence so the device stays active through step zero, notification-URB setup, and both initialization commands. The central transport keeps its guard for calls outside initialization; the existing active counter balances the nested calls. FCP has no private resume callback. Its suspend callback only removes the notification URB. Taking the initialization and transport guards under the existing protocol mutex causes no resume-side lock inversion. Assisted-by: Codex:gpt-5.6-sol Signed-off-by: Will Porter <mrwillporter@gmail.com> --- sound/usb/fcp.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sound/usb/fcp.c b/sound/usb/fcp.c index 5fc2131b4561..68bb7eabf107 100644 --- a/sound/usb/fcp.c +++ b/sound/usb/fcp.c @@ -191,6 +191,10 @@ static int fcp_usb(struct usb_mixer_interface *mixer, u32 opcode, const int max_retries = 5; int err; + CLASS(snd_usb_lock, pm)(mixer->chip); + if (pm.err < 0) + return -EIO; + if (!private->urb) return -ENODEV; @@ -1026,6 +1030,10 @@ static int fcp_init(struct usb_mixer_interface *mixer, struct usb_device *dev = mixer->chip->dev; int err; + CLASS(snd_usb_lock, pm)(mixer->chip); + if (pm.err < 0) + return -EIO; + err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0), FCP_USB_REQ_STEP0, USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN, -- 2.47.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-08-27 23:22 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-08-24 23:03 [RFC] ALSA: usb-audio: missing PM guards in device mixer paths Will Porter 2026-08-25 8:17 ` Takashi Iwai 2026-08-27 23:21 ` [PATCH 0/6] ALSA: usb-audio: guard device-specific control transfers Will Porter 2026-08-27 23:21 ` [PATCH 1/6] ALSA: usb-audio: Add PM guard to Studio 1810c controls Will Porter 2026-08-27 23:21 ` [PATCH 2/6] ALSA: usb-audio: Add PM guards to US-16x08 transfers Will Porter 2026-08-27 23:21 ` [PATCH 3/6] ALSA: usb-audio: Add PM guard to Scarlett meter reads Will Porter 2026-08-27 23:21 ` [PATCH 4/6] ALSA: usb-audio: Guard Scarlett2 protocol transfers Will Porter 2026-08-27 23:21 ` [PATCH 5/6] ALSA: usb-audio: Add PM guards to RME Digiface controls Will Porter 2026-08-27 23:21 ` [PATCH 6/6] ALSA: usb-audio: Guard FCP protocol transfers Will Porter
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®