[RFC] ALSA: usb-audio: missing PM guards in device mixer paths

[Will Porter <mrwillporter@gmail.com>]

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