[PATCH] ALSA: FCP: fix OOB write in fcp_meter_ctl_get()

[PATCH] ALSA: FCP: fix OOB write in fcp_meter_ctl_get()

[Baul Lee]

fcp_ioctl_set_meter_map() bounds the user-supplied Level Meter map size
by the driver's own limit of 255

	if (map.map_size < 1 || map.map_size > 255 ||
	    map.meter_slots < 1 || map.meter_slots > 255)
		return -EINVAL;

and passes it to fcp_add_new_ctl() as the control's channel count, where
it is stored as elem->channels.

Every control read writes into struct snd_ctl_elem_value, whose integer
array is declared long value[128], so the limit is 128, not 255.
fcp_meter_ctl_get() stores one 64-bit word per channel into that array
with no bound of its own:

	for (i = 0; i < elem->channels; i++) {
		int idx = private->meter_level_map[i];
		int value = idx < 0 ? 0 : le32_to_cpu(resp[idx]);

		ucontrol->value.integer.value[i] = value;
	}

snd_ctl_elem_read_user() serves that object from
memdup_user(_control, sizeof(*control)), 1224 bytes on LP64 out of
kmalloc-2048.  offsetof(struct snd_ctl_elem_value, value) is 72, so
element i is written at byte 72 + 8 * i and element 144 already lands
past the allocation.  At map_size 255 the last store ends at byte 2112,
888 bytes past the object and 64 bytes into the adjacent slab object.
The stored words come from the device and meter_level_map[] selects
which word lands in which slot, so extent and contents are both
controlled.

The core does not catch this.  snd_ctl_check_elem_info() is reached only
from __snd_ctl_elem_info(), which snd_ctl_elem_read() calls under
CONFIG_SND_CTL_DEBUG; without that option snd_ctl_skip_validation() is a
compile-time true.  __snd_ctl_add_replace() validates kcontrol->count and
never inspects elem->channels.

Installing an oversized map needs CAP_SYS_RAWIO, but the control outlives
the hwdep descriptor that created it, so the out-of-bounds stores are
issued by any process able to read controls on /dev/snd/controlC0.

KASAN on 7.2.0-rc5 (arm64), triggered by an unprivileged control read:

  BUG: KASAN: slab-out-of-bounds in fcp_meter_ctl_get
  Write of size 8 at addr ffff000017af04c8 by task fcp_trigger/185
   __asan_store8
   fcp_meter_ctl_get
   snd_ctl_elem_read
   snd_ctl_ioctl
  Allocated by task 185:
   memdup_user
   snd_ctl_ioctl
  The buggy address is located 0 bytes to the right of
   allocated 1224-byte region [ffff000017af0000, ffff000017af04c8)

Bound the map size by the ABI limit rather than by 255, and bound the
store loop at the sink so it cannot run past the value array whatever
elem->channels holds.

Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>

Fixes: 46757a3e7d50 ("ALSA: FCP: Add Focusrite Control Protocol driver")
Reported-by: Federico Kirschbaum <federico.kirschbaum@xbow.com>
Reported-by: Baul Lee <baul.lee@xbow.com>
Cc: stable@vger.kernel.org
Signed-off-by: Baul Lee <baul.lee@xbow.com>
---
diff --git a/sound/usb/fcp.c b/sound/usb/fcp.c
index 6f5dcd35e1d4..2bf572c6fdc4 100644
--- a/sound/usb/fcp.c
+++ b/sound/usb/fcp.c
@@ -129,6 +129,10 @@ struct fcp_data {
 
 #define FCP_SEGMENT_APP_GOLD 0
 
+#define FCP_MAX_METER_MAP_SIZE \
+	(sizeof_field(struct snd_ctl_elem_value, value.integer.value) / \
+	 sizeof(long))
+
 /* Forward declarations */
 static int fcp_init(struct usb_mixer_interface *mixer,
 		    void *step0_resp, void *step2_resp);
@@ -410,6 +414,9 @@ static int fcp_meter_ctl_get(struct snd_kcontrol *kctl,
 	if (err < 0)
 		return err;
 
+	if (WARN_ON_ONCE(elem->channels > FCP_MAX_METER_MAP_SIZE))
+		return -EINVAL;
+
 	/* copy & translate from resp[] using meter_level_map[] */
 	for (i = 0; i < elem->channels; i++) {
 		int idx = private->meter_level_map[i];
@@ -636,7 +643,8 @@ static int fcp_ioctl_set_meter_map(struct usb_mixer_interface *mixer,
 	}
 
 	/* Validate the map size */
-	if (map.map_size < 1 || map.map_size > 255 ||
+	if (map.map_size < 1 ||
+	    map.map_size > FCP_MAX_METER_MAP_SIZE ||
 	    map.meter_slots < 1 || map.meter_slots > 255)
 		return -EINVAL;
 
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] ALSA: FCP: fix OOB write in fcp_meter_ctl_get()

[Takashi Iwai]

On Tue, 04 Aug 2026 14:36:11 +0200,
Baul Lee wrote:
> 
> fcp_ioctl_set_meter_map() bounds the user-supplied Level Meter map size
> by the driver's own limit of 255
> 
> 	if (map.map_size < 1 || map.map_size > 255 ||
> 	    map.meter_slots < 1 || map.meter_slots > 255)
> 		return -EINVAL;
> 
> and passes it to fcp_add_new_ctl() as the control's channel count, where
> it is stored as elem->channels.
> 
> Every control read writes into struct snd_ctl_elem_value, whose integer
> array is declared long value[128], so the limit is 128, not 255.
> fcp_meter_ctl_get() stores one 64-bit word per channel into that array
> with no bound of its own:
> 
> 	for (i = 0; i < elem->channels; i++) {
> 		int idx = private->meter_level_map[i];
> 		int value = idx < 0 ? 0 : le32_to_cpu(resp[idx]);
> 
> 		ucontrol->value.integer.value[i] = value;
> 	}
> 
> snd_ctl_elem_read_user() serves that object from
> memdup_user(_control, sizeof(*control)), 1224 bytes on LP64 out of
> kmalloc-2048.  offsetof(struct snd_ctl_elem_value, value) is 72, so
> element i is written at byte 72 + 8 * i and element 144 already lands
> past the allocation.  At map_size 255 the last store ends at byte 2112,
> 888 bytes past the object and 64 bytes into the adjacent slab object.
> The stored words come from the device and meter_level_map[] selects
> which word lands in which slot, so extent and contents are both
> controlled.
> 
> The core does not catch this.  snd_ctl_check_elem_info() is reached only
> from __snd_ctl_elem_info(), which snd_ctl_elem_read() calls under
> CONFIG_SND_CTL_DEBUG; without that option snd_ctl_skip_validation() is a
> compile-time true.  __snd_ctl_add_replace() validates kcontrol->count and
> never inspects elem->channels.
> 
> Installing an oversized map needs CAP_SYS_RAWIO, but the control outlives
> the hwdep descriptor that created it, so the out-of-bounds stores are
> issued by any process able to read controls on /dev/snd/controlC0.
> 
> KASAN on 7.2.0-rc5 (arm64), triggered by an unprivileged control read:
> 
>   BUG: KASAN: slab-out-of-bounds in fcp_meter_ctl_get
>   Write of size 8 at addr ffff000017af04c8 by task fcp_trigger/185
>    __asan_store8
>    fcp_meter_ctl_get
>    snd_ctl_elem_read
>    snd_ctl_ioctl
>   Allocated by task 185:
>    memdup_user
>    snd_ctl_ioctl
>   The buggy address is located 0 bytes to the right of
>    allocated 1224-byte region [ffff000017af0000, ffff000017af04c8)
> 
> Bound the map size by the ABI limit rather than by 255, and bound the
> store loop at the sink so it cannot run past the value array whatever
> elem->channels holds.
> 
> Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>
> 
> Fixes: 46757a3e7d50 ("ALSA: FCP: Add Focusrite Control Protocol driver")
> Reported-by: Federico Kirschbaum <federico.kirschbaum@xbow.com>
> Reported-by: Baul Lee <baul.lee@xbow.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Baul Lee <baul.lee@xbow.com>

Applied now.  Thanks.


Takashi