ALSA: FCP: Use a private URB for the notification endpoint

fcp_init_notify() used mixer->urb, which snd_usb_mixer_status_create()
allocates for the optional UAC2 status interrupt endpoint and mixer.c
kills, resubmits and frees. On a device with that endpoint,
fcp_init_notify()'s "already set up" early return fires on the status
URB and returns success without doing anything. No FCP notification
URB is submitted, and cmd_done is left zeroed because it is
initialised past that early return and nowhere else. fcp_init() then
issues init1_opcode and wait_for_completion_timeout() would crash
adding to the zeroed wait.head. fcp_cleanup_urb() would also kill and
free mixer.c's status URB.

Use a separate URB in fcp_data, and initialise cmd_done in
fcp_init_private() where fcp_data is allocated. fcp_init_notify() is
reached again after suspend via fcp_reinit(), and the URB kill path in
fcp_notify() completes cmd_done, leaving a stale count that would
satisfy the next command's wait before the device ACKs. Use
reinit_completion() to clear it.

Fixes: 46757a3e7d ("ALSA: FCP: Add Focusrite Control Protocol driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Geoffrey D. Bennett <g@b4.vu>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/2cad281e6434024ca48a9ecc94fa19d6777e9be7.1786290885.git.g@b4.vu
This commit is contained in:
Geoffrey D. Bennett
2026-08-10 03:36:01 +09:30
committed by Takashi Iwai
parent 7097666b99
commit 918b8d231c

View File

@@ -82,6 +82,7 @@ struct fcp_data {
struct mutex mutex; /* serialise access to the device */
struct completion cmd_done; /* wait for command completion */
struct file *file; /* hwdep file */
struct urb *urb; /* FCP notification endpoint */
struct fcp_notify notify;
@@ -186,7 +187,7 @@ static int fcp_usb(struct usb_mixer_interface *mixer, u32 opcode,
const int max_retries = 5;
int err;
if (!mixer->urb)
if (!private->urb)
return -ENODEV;
struct fcp_usb_packet *req __free(kfree) = NULL;
@@ -301,7 +302,7 @@ static int fcp_reinit(struct usb_mixer_interface *mixer)
{
struct fcp_data *private = mixer->private_data;
if (mixer->urb)
if (private->urb)
return 0;
void *step0_resp __free(kfree) =
@@ -893,13 +894,15 @@ static int fcp_hwdep_init(struct usb_mixer_interface *mixer)
static void fcp_cleanup_urb(struct usb_mixer_interface *mixer)
{
if (!mixer->urb)
struct fcp_data *private = mixer->private_data;
if (!private->urb)
return;
usb_kill_urb(mixer->urb);
kfree(mixer->urb->transfer_buffer);
usb_free_urb(mixer->urb);
mixer->urb = NULL;
usb_kill_urb(private->urb);
kfree(private->urb->transfer_buffer);
usb_free_urb(private->urb);
private->urb = NULL;
}
static void fcp_private_free(struct usb_mixer_interface *mixer)
@@ -970,37 +973,37 @@ static int fcp_init_notify(struct usb_mixer_interface *mixer)
int err;
/* Already set up */
if (mixer->urb)
if (private->urb)
return 0;
if (usb_pipe_type_check(dev, pipe))
return -EINVAL;
mixer->urb = usb_alloc_urb(0, GFP_KERNEL);
if (!mixer->urb)
private->urb = usb_alloc_urb(0, GFP_KERNEL);
if (!private->urb)
return -ENOMEM;
transfer_buffer = kmalloc(private->wMaxPacketSize, GFP_KERNEL);
if (!transfer_buffer) {
usb_free_urb(mixer->urb);
mixer->urb = NULL;
usb_free_urb(private->urb);
private->urb = NULL;
return -ENOMEM;
}
usb_fill_int_urb(mixer->urb, dev, pipe,
usb_fill_int_urb(private->urb, dev, pipe,
transfer_buffer, private->wMaxPacketSize,
fcp_notify, mixer, private->bInterval);
init_completion(&private->cmd_done);
reinit_completion(&private->cmd_done);
err = usb_submit_urb(mixer->urb, GFP_KERNEL);
err = usb_submit_urb(private->urb, GFP_KERNEL);
if (err) {
usb_audio_err(mixer->chip,
"%s: usb_submit_urb failed: %d\n",
__func__, err);
kfree(transfer_buffer);
usb_free_urb(mixer->urb);
mixer->urb = NULL;
usb_free_urb(private->urb);
private->urb = NULL;
}
return err;
@@ -1053,6 +1056,7 @@ static int fcp_init_private(struct usb_mixer_interface *mixer)
return -ENOMEM;
mutex_init(&private->mutex);
init_completion(&private->cmd_done);
init_waitqueue_head(&private->notify.queue);
spin_lock_init(&private->notify.lock);