Hi,
while fuzzing a USB gadget that emulates a Tascam US-16x08 we found that get_meter_levels_from_urb() in mixer_us16x08.c uses a channel index taken directly from the 64-byte meter packet to index meter_level[], comp_level[] and master_level[] without any bounds checking. A malformed packet can therefore cause out-of-bounds writes in the snd_us16x08_meter_store.
A malicious USB audio device (or USB gadget implementation) that pretends to be a US-16x08-compatible interface can trigger this by sending crafted meter packets. We have a small USB gadget-based PoC for this behaviour and can share it if that would be helpful.
This driver is used by common distributions (e.g. Ubuntu) when a US-16x08 or compatible USB audio device is present. The same pattern is present in current mainline kernels.
This issue was first reported via security@kernel.org. The kernel security team explained that, in the upstream threat model, USB endpoints are expected to be trusted (i.e. only trusted devices should be bound to drivers), so they consider this a normal bug rather than a security vulnerability, and asked us to send a fix to the development lists. The patch below adds simple range checks before updating these arrays.
Suggested patch:
--- sound/usb/mixer_us16x08.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/sound/usb/mixer_us16x08.c b/sound/usb/mixer_us16x08.c index XXXXXXXX..YYYYYYYY 100644 --- a/sound/usb/mixer_us16x08.c +++ b/sound/usb/mixer_us16x08.c @@ -647,15 +647,26 @@ static int snd_get_meter_comp_index(struct snd_us16x08_meter_store *store) /* retrieve the meter level values from URB message */ static void get_meter_levels_from_urb(int s, struct snd_us16x08_meter_store *store, u8 *meter_urb) { int val = MUC2(meter_urb, s) + (MUC3(meter_urb, s) << 8); + int ch = MUB2(meter_urb, s) - 1; + + if (ch < 0) + return; if (MUA0(meter_urb, s) == 0x61 && MUA1(meter_urb, s) == 0x02 && - MUA2(meter_urb, s) == 0x04 && MUB0(meter_urb, s) == 0x62) { - if (MUC0(meter_urb, s) == 0x72) - store->meter_level[MUB2(meter_urb, s) - 1] = val; - if (MUC0(meter_urb, s) == 0xb2) - store->comp_level[MUB2(meter_urb, s) - 1] = val; - } + MUA2(meter_urb, s) == 0x04 && MUB0(meter_urb, s) == 0x62) { + if (ch < SND_US16X08_MAX_CHANNELS) { + if (MUC0(meter_urb, s) == 0x72) + store->meter_level[ch] = val; + if (MUC0(meter_urb, s) == 0xb2) + store->comp_level[ch] = val; + } + } if (MUA0(meter_urb, s) == 0x61 && MUA1(meter_urb, s) == 0x02 && - MUA2(meter_urb, s) == 0x02 && MUB0(meter_urb, s) == 0x62) - store->master_level[MUB2(meter_urb, s) - 1] = val; + MUA2(meter_urb, s) == 0x02 && MUB0(meter_urb, s) == 0x62) { + if (ch < ARRAY_SIZE(store->master_level)) + store->master_level[ch] = val; + } }
Reported-by: DARKNAVY (@DarkNavyOrg) <vr@darknavy.com> Signed-off-by: DARKNAVY (@DarkNavyOrg) <vr@darknavy.com>