Re: [PATCH] ALSA: FCP: do not copy out an uninitialised init response

[Takashi Iwai]

On Wed, 05 Aug 2026 03:38:04 +0200,
Baul Lee wrote:
> 
> fcp_ioctl_init() allocates its response buffer with kmalloc() and copies
> the whole buffer back to userspace:
> 
> 	buf_size = init.step0_resp_size + init.step2_resp_size;
> 
> 	void *resp __free(kfree) =
> 		kmalloc(buf_size, GFP_KERNEL);
> 	...
> 	if (copy_to_user(arg->resp, resp, buf_size))
> 		return -EFAULT;
> 
> Nothing clears the buffer, and the only writer of its leading
> step0_resp_size bytes is the step-0 control transfer:
> 
> 	err = snd_usb_ctl_msg(dev, usb_rcvctrlpipe(dev, 0),
> 		FCP_USB_REQ_STEP0,
> 		USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN,
> 		0, private->bInterfaceNumber,
> 		step0_resp, private->step0_resp_size);
> 	if (err < 0)
> 		return err;
> 
> usb_fill_control_urb() does not set URB_SHORT_NOT_OK, so a short or
> zero-length data stage completes with status 0 and snd_usb_ctl_msg()
> returns a small actual_length.  The only check is err < 0, so a short
> transfer is accepted as success.
> 
> snd_usb_ctl_msg() copies the full size back unconditionally:
> 
> 	buf = kmemdup(data, size, GFP_KERNEL);
> 	...
> 	memcpy(data, buf, size);
> 
> Bytes the device never wrote are therefore restored into resp unchanged
> and copied to userspace.  step0_resp_size and step2_resp_size are each
> validated only to 1..255, so the caller also picks the slab cache, from
> kmalloc-8 up to kmalloc-512.
> 
> On 7.2.0-rc5 (arm64), device answering step 0 with a zero-length data
> stage, s0 = s2 = 255:
> 
>   # init_on_alloc off, no spray
>   step0 window [0,255): nonzero=94/255
>   000: 00 80 60 06 00 00 ff ff 18 00 00 00 57 01 ea 01
>   010: 08 78 22 13 00 00 ff ff a8 c4 5f 80 00 80 ff ff
> 
>   # same kernel, kmalloc-512 pre-seeded with an 8-byte tag
>   step0 window [0,255): nonzero=219/255  tagbytes=232
> 
>   # identical run, init_on_alloc=1
>   step0 window [0,255): nonzero=0/255  tagbytes=0
> 
>   # all three runs
>   step2 window [255,510): device words matched=62/62
> 
> a8 c4 5f 80 00 80 ff ff is the little-endian kernel text address
> ffff8000805fc4a8.  The step-2 window is unaffected, so the disclosure is
> exactly the step-0 region.
> 
> Zero the buffer, and require the step-0 transfer to deliver the full
> step0_resp_size bytes so a short data stage is reported as an error.
> 
> Discovered by XBOW, triaged by Baul Lee <baul.lee@xxxxxxxx>
> 
> Fixes: 46757a3e7d50 ("ALSA: FCP: Add Focusrite Control Protocol driver")
> Reported-by: Federico Kirschbaum <federico.kirschbaum@xxxxxxxx>
> Reported-by: Baul Lee <baul.lee@xxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Baul Lee <baul.lee@xxxxxxxx>

Applied to for-next branch now.  Thanks.


Takashi