Linux Sound subsystem development
 help / color / mirror / Atom feed
* ALSA: caiaq: possible out of bounds read in Traktor Kontrol S4 dispatch
@ 2026-06-17 12:35 Maoyi Xie
  2026-06-17 12:45 ` Takashi Iwai
  0 siblings, 1 reply; 3+ messages in thread
From: Maoyi Xie @ 2026-06-17 12:35 UTC (permalink / raw)
  To: Daniel Mack, Jaroslav Kysela, Takashi Iwai; +Cc: linux-sound, linux-kernel

Hi all,

I think the Traktor Kontrol S4 input path in sound/usb/caiaq/input.c can read
past the end of the URB buffer when the device sends a reply whose length is
not a multiple of 16. I would appreciate it if you could take a look.

The dispatch loop walks the reply in fixed 16 byte blocks.

	while (len) {
		unsigned int i, block_id = (buf[0] << 8) | buf[1];
		...
		len -= TKS4_MSGBLOCK_SIZE;
		buf += TKS4_MSGBLOCK_SIZE;
	}

Each pass reads up to buf[15] and then does len -= 16. But len is the raw
device length. It comes straight from urb->actual_length in
snd_usb_caiaq_ep4_reply_dispatch and there is no check that it is a multiple
of 16. The X1 and Maschine arms in that same handler do bound check the
length, but the S4 arm does not.

If len is not a multiple of 16, say 8, the final len -= 16 underflows because
len is unsigned. The loop condition while (len) then stays true and the walk
keeps consuming 16 byte blocks far past the end of the buffer. That is an out
of bounds read. The buffer is cdev->ep4_in_buf of size EP4_BUFSIZE.

The attacker model is a malicious or emulated USB device that claims the
Native Instruments Traktor Kontrol S4 id and returns a short reply on its bulk
in endpoint once the input device is opened.

I reproduced this under KASAN on 7.1-rc7. A length of one whole block runs the
loop once and reads nothing past the buffer. A length of 8 underflows and
KASAN reports a slab out of bounds read just past the allocation.

The fix I tried is to only consume a block when a whole block is present.

	-	while (len) {
	+	while (len >= TKS4_MSGBLOCK_SIZE) {

That stops the decrement from underflowing and drops any trailing partial
block.

Does this look like a real bug to you? If the direction is right I am happy to
send a proper patch with a Fixes tag pointing at 15c5ab607045 ("ALSA:
snd-usb-caiaq: Add support for Traktor Kontrol S4").

Thanks,
Maoyi
https://maoyixie.com/

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: ALSA: caiaq: possible out of bounds read in Traktor Kontrol S4 dispatch
  2026-06-17 12:35 ALSA: caiaq: possible out of bounds read in Traktor Kontrol S4 dispatch Maoyi Xie
@ 2026-06-17 12:45 ` Takashi Iwai
  2026-06-17 13:58   ` Takashi Iwai
  0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2026-06-17 12:45 UTC (permalink / raw)
  To: Maoyi Xie
  Cc: Daniel Mack, Jaroslav Kysela, Takashi Iwai, linux-sound,
	linux-kernel

On Wed, 17 Jun 2026 14:35:00 +0200,
Maoyi Xie wrote:
> 
> Hi all,
> 
> I think the Traktor Kontrol S4 input path in sound/usb/caiaq/input.c can read
> past the end of the URB buffer when the device sends a reply whose length is
> not a multiple of 16. I would appreciate it if you could take a look.
> 
> The dispatch loop walks the reply in fixed 16 byte blocks.
> 
> 	while (len) {
> 		unsigned int i, block_id = (buf[0] << 8) | buf[1];
> 		...
> 		len -= TKS4_MSGBLOCK_SIZE;
> 		buf += TKS4_MSGBLOCK_SIZE;
> 	}
> 
> Each pass reads up to buf[15] and then does len -= 16. But len is the raw
> device length. It comes straight from urb->actual_length in
> snd_usb_caiaq_ep4_reply_dispatch and there is no check that it is a multiple
> of 16. The X1 and Maschine arms in that same handler do bound check the
> length, but the S4 arm does not.
> 
> If len is not a multiple of 16, say 8, the final len -= 16 underflows because
> len is unsigned. The loop condition while (len) then stays true and the walk
> keeps consuming 16 byte blocks far past the end of the buffer. That is an out
> of bounds read. The buffer is cdev->ep4_in_buf of size EP4_BUFSIZE.
> 
> The attacker model is a malicious or emulated USB device that claims the
> Native Instruments Traktor Kontrol S4 id and returns a short reply on its bulk
> in endpoint once the input device is opened.
> 
> I reproduced this under KASAN on 7.1-rc7. A length of one whole block runs the
> loop once and reads nothing past the buffer. A length of 8 underflows and
> KASAN reports a slab out of bounds read just past the allocation.
> 
> The fix I tried is to only consume a block when a whole block is present.
> 
> 	-	while (len) {
> 	+	while (len >= TKS4_MSGBLOCK_SIZE) {
> 
> That stops the decrement from underflowing and drops any trailing partial
> block.
> 
> Does this look like a real bug to you? If the direction is right I am happy to
> send a proper patch with a Fixes tag pointing at 15c5ab607045 ("ALSA:
> snd-usb-caiaq: Add support for Traktor Kontrol S4").

Yes, feel free to submit the patch.

BTW, it seems that there are lots of other places missing the length
check, and we'd need to paper over them, too:
snd_caiaq_input_read_analog(), snd_caiaq_input_read_erp(),
snd_caiaq_input_read_io(), and snd_usb_caiaq_maschine_dispatch().


thanks,

Takashi

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: ALSA: caiaq: possible out of bounds read in Traktor Kontrol S4 dispatch
  2026-06-17 12:45 ` Takashi Iwai
@ 2026-06-17 13:58   ` Takashi Iwai
  0 siblings, 0 replies; 3+ messages in thread
From: Takashi Iwai @ 2026-06-17 13:58 UTC (permalink / raw)
  To: Maoyi Xie
  Cc: Daniel Mack, Jaroslav Kysela, Takashi Iwai, linux-sound,
	linux-kernel

On Wed, 17 Jun 2026 14:45:58 +0200,
Takashi Iwai wrote:
> 
> On Wed, 17 Jun 2026 14:35:00 +0200,
> Maoyi Xie wrote:
> > 
> > Hi all,
> > 
> > I think the Traktor Kontrol S4 input path in sound/usb/caiaq/input.c can read
> > past the end of the URB buffer when the device sends a reply whose length is
> > not a multiple of 16. I would appreciate it if you could take a look.
> > 
> > The dispatch loop walks the reply in fixed 16 byte blocks.
> > 
> > 	while (len) {
> > 		unsigned int i, block_id = (buf[0] << 8) | buf[1];
> > 		...
> > 		len -= TKS4_MSGBLOCK_SIZE;
> > 		buf += TKS4_MSGBLOCK_SIZE;
> > 	}
> > 
> > Each pass reads up to buf[15] and then does len -= 16. But len is the raw
> > device length. It comes straight from urb->actual_length in
> > snd_usb_caiaq_ep4_reply_dispatch and there is no check that it is a multiple
> > of 16. The X1 and Maschine arms in that same handler do bound check the
> > length, but the S4 arm does not.
> > 
> > If len is not a multiple of 16, say 8, the final len -= 16 underflows because
> > len is unsigned. The loop condition while (len) then stays true and the walk
> > keeps consuming 16 byte blocks far past the end of the buffer. That is an out
> > of bounds read. The buffer is cdev->ep4_in_buf of size EP4_BUFSIZE.
> > 
> > The attacker model is a malicious or emulated USB device that claims the
> > Native Instruments Traktor Kontrol S4 id and returns a short reply on its bulk
> > in endpoint once the input device is opened.
> > 
> > I reproduced this under KASAN on 7.1-rc7. A length of one whole block runs the
> > loop once and reads nothing past the buffer. A length of 8 underflows and
> > KASAN reports a slab out of bounds read just past the allocation.
> > 
> > The fix I tried is to only consume a block when a whole block is present.
> > 
> > 	-	while (len) {
> > 	+	while (len >= TKS4_MSGBLOCK_SIZE) {
> > 
> > That stops the decrement from underflowing and drops any trailing partial
> > block.
> > 
> > Does this look like a real bug to you? If the direction is right I am happy to
> > send a proper patch with a Fixes tag pointing at 15c5ab607045 ("ALSA:
> > snd-usb-caiaq: Add support for Traktor Kontrol S4").
> 
> Yes, feel free to submit the patch.
> 
> BTW, it seems that there are lots of other places missing the length
> check, and we'd need to paper over them, too:
> snd_caiaq_input_read_analog(), snd_caiaq_input_read_erp(),
> snd_caiaq_input_read_io(), and snd_usb_caiaq_maschine_dispatch().

Actually for snd_caiaq_input_read_analog() and
snd_usb_caiaq_maschine_dispatch(), the callers have already the length
checks, so they are OK.  But snd_caiaq_input_read_erp() and
snd_caiaq_input_read_io() may be called via
snd_usb_caiaq_input_dispatch(), and they need the length check,
something like below (totally untested).


Takashi

-- 8< --
--- a/sound/usb/caiaq/input.c
+++ b/sound/usb/caiaq/input.c
@@ -237,12 +237,16 @@ static void snd_caiaq_input_read_erp(struct snd_usb_caiaqdev *cdev,
 
 	switch (cdev->chip.usb_id) {
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_AK1):
+		if (len < 2)
+			return;
 		i = decode_erp(buf[0], buf[1]);
 		input_report_abs(input_dev, ABS_X, i);
 		input_sync(input_dev);
 		break;
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_KORECONTROLLER):
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_KORECONTROLLER2):
+		if (len < 16)
+			return;
 		i = decode_erp(buf[7], buf[5]);
 		input_report_abs(input_dev, ABS_HAT0X, i);
 		i = decode_erp(buf[12], buf[14]);
@@ -263,6 +267,8 @@ static void snd_caiaq_input_read_erp(struct snd_usb_caiaqdev *cdev,
 		break;
 
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_MASCHINECONTROLLER):
+		if (len < 22)
+			return;
 		/* 4 under the left screen */
 		input_report_abs(input_dev, ABS_HAT0X, decode_erp(buf[21], buf[20]));
 		input_report_abs(input_dev, ABS_HAT0Y, decode_erp(buf[15], buf[14]));
@@ -308,9 +314,13 @@ static void snd_caiaq_input_read_io(struct snd_usb_caiaqdev *cdev,
 	switch (cdev->chip.usb_id) {
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_KORECONTROLLER):
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_KORECONTROLLER2):
+		if (len < 5)
+			return;
 		input_report_abs(cdev->input_dev, ABS_MISC, 255 - buf[4]);
 		break;
 	case USB_ID(USB_VID_NATIVEINSTRUMENTS, USB_PID_TRAKTORKONTROLX1):
+		if (len < 7)
+			return;
 		/* rotary encoders */
 		input_report_abs(cdev->input_dev, ABS_X, buf[5] & 0xf);
 		input_report_abs(cdev->input_dev, ABS_Y, buf[5] >> 4);

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-06-17 13:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-17 12:35 ALSA: caiaq: possible out of bounds read in Traktor Kontrol S4 dispatch Maoyi Xie
2026-06-17 12:45 ` Takashi Iwai
2026-06-17 13:58   ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox