[PATCH] ALSA: FCP: fix OOB write in fcp_meter_ctl_get()
Baul Lee <[email protected]> Tue, 4 Aug 2026 21:36:11 +0900
| Newsgroups | gmane.linux.sound,gmane.linux.kernel,gmane.linux.kernel.stable |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]>
Fixes: 46757a3e7d50 ("ALSA: FCP: Add Focusrite Control Protocol driver")
Reported-by: Federico Kirschbaum <[email protected]>
Reported-by: Baul Lee <[email protected]>
Cc: [email protected]
Signed-off-by: Baul Lee <[email protected]>
---
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)