Linux Sound subsystem development
 help / color / mirror / Atom feed
* [PATCH 0/2] ALSA: caiaq: fix S4 OOB read and bound the EP1 input parsers
@ 2026-06-18  6:03 Maoyi Xie
  2026-06-18  6:03 ` [PATCH 1/2] ALSA: caiaq: fix out-of-bounds read in the Traktor Kontrol S4 input parser Maoyi Xie
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Maoyi Xie @ 2026-06-18  6:03 UTC (permalink / raw)
  To: Daniel Mack, Jaroslav Kysela, Takashi Iwai; +Cc: linux-sound, linux-kernel

Hi Takashi,

Thanks for confirming the Traktor Kontrol S4 out-of-bounds read and for
the follow-up on the neighbouring parsers.

Patch 1 is the actual fix. snd_usb_caiaq_tks4_dispatch() loops on the raw
urb->actual_length. That value is controlled by the device and is not
required to be a multiple of the 16-byte message block. Once len drops
below 16 the unsigned "len -= TKS4_MSGBLOCK_SIZE" underflows. The loop
then keeps walking buf past ep4_in_buf[EP4_BUFSIZE]. The fix iterates
only while a full block remains, which also discards any trailing partial
block. The X1 and Maschine arms already floor the length before dispatch,
so only the S4 arm was affected.

Patch 2 adds the length checks you suggested to
snd_caiaq_input_read_erp() and snd_caiaq_input_read_io(). Both are
reachable through snd_usb_caiaq_input_dispatch(). As you noted,
snd_caiaq_input_read_analog() and snd_usb_caiaq_maschine_dispatch()
already have the length floored by their callers, so they are left
unchanged. The two parsers patch 2 touches are not an out-of-bounds
access either. Every offset is a fixed driver constant within the 64-byte
ep1_in_buf. A short reply does make them decode stale data, though, so the
guards drop such replies per device path. Patch 2 carries your
Suggested-by.

Patch 1 carries a Fixes tag and Cc: stable. Patch 2 does not.

Maoyi Xie (2):
  ALSA: caiaq: fix out-of-bounds read in the Traktor Kontrol S4 input
    parser
  ALSA: caiaq: bound the length in the EP1 input parsers

 sound/usb/caiaq/input.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

-- 
2.34.1


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

* [PATCH 1/2] ALSA: caiaq: fix out-of-bounds read in the Traktor Kontrol S4 input parser
  2026-06-18  6:03 [PATCH 0/2] ALSA: caiaq: fix S4 OOB read and bound the EP1 input parsers Maoyi Xie
@ 2026-06-18  6:03 ` Maoyi Xie
  2026-06-18  6:03 ` [PATCH 2/2] ALSA: caiaq: bound the length in the EP1 input parsers Maoyi Xie
  2026-06-18 10:38 ` [PATCH 0/2] ALSA: caiaq: fix S4 OOB read and bound " Takashi Iwai
  2 siblings, 0 replies; 4+ messages in thread
From: Maoyi Xie @ 2026-06-18  6:03 UTC (permalink / raw)
  To: Daniel Mack, Jaroslav Kysela, Takashi Iwai; +Cc: linux-sound, linux-kernel

snd_usb_caiaq_tks4_dispatch() decodes the Traktor Kontrol S4 input
stream in fixed 16-byte (TKS4_MSGBLOCK_SIZE) message blocks. On every
iteration it advances buf and subtracts the block size while looping on
"while (len)".

len is urb->actual_length. That value is supplied by the device and is
not guaranteed to be a multiple of 16. When a final short block leaves
len between 1 and 15, the loop runs once more, reads up to buf[15], and
then does "len -= TKS4_MSGBLOCK_SIZE". As len is unsigned this underflows
to a huge value. The loop then keeps iterating and walking buf far past
the end of the 512-byte ep4_in_buf, reading out of bounds until a bogus
block id happens to be hit.

Iterate only while a full message block is available. This stops the
unsigned underflow and silently drops any trailing partial block, which
carries no complete control value anyway.

The sibling endpoint-4 parsers are not affected. The Traktor Kontrol X1
and Maschine arms in snd_usb_caiaq_ep4_reply_dispatch() floor
urb->actual_length before dispatching.

Fixes: 15c5ab607045 ("ALSA: snd-usb-caiaq: Add support for Traktor Kontrol S4")
Cc: stable@vger.kernel.org
Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
---
 sound/usb/caiaq/input.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/usb/caiaq/input.c b/sound/usb/caiaq/input.c
index 5c70fdf61cc1..2db4d1332df1 100644
--- a/sound/usb/caiaq/input.c
+++ b/sound/usb/caiaq/input.c
@@ -330,7 +330,7 @@ static void snd_usb_caiaq_tks4_dispatch(struct snd_usb_caiaqdev *cdev,
 {
 	struct device *dev = caiaqdev_to_dev(cdev);
 
-	while (len) {
+	while (len >= TKS4_MSGBLOCK_SIZE) {
 		unsigned int i, block_id = (buf[0] << 8) | buf[1];
 
 		switch (block_id) {
-- 
2.34.1


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

* [PATCH 2/2] ALSA: caiaq: bound the length in the EP1 input parsers
  2026-06-18  6:03 [PATCH 0/2] ALSA: caiaq: fix S4 OOB read and bound the EP1 input parsers Maoyi Xie
  2026-06-18  6:03 ` [PATCH 1/2] ALSA: caiaq: fix out-of-bounds read in the Traktor Kontrol S4 input parser Maoyi Xie
@ 2026-06-18  6:03 ` Maoyi Xie
  2026-06-18 10:38 ` [PATCH 0/2] ALSA: caiaq: fix S4 OOB read and bound " Takashi Iwai
  2 siblings, 0 replies; 4+ messages in thread
From: Maoyi Xie @ 2026-06-18  6:03 UTC (permalink / raw)
  To: Daniel Mack, Jaroslav Kysela, Takashi Iwai; +Cc: linux-sound, linux-kernel

snd_caiaq_input_read_erp() and snd_caiaq_input_read_io() can be reached
from snd_usb_caiaq_input_dispatch(). They read fixed byte offsets from
the reply buffer without checking the reported length. On a short reply
they decode stale bytes left from a previous, longer report and feed them
to the input layer.

This is not an out-of-bounds access. Every offset is a compile-time
driver constant. The largest is buf[21] in the Maschine ERP case. The
EP1 transfer buffer ep1_in_buf is EP1_BUFSIZE (64) bytes, and the USB
core caps actual_length at 64, so a short reply only reads in-bounds
stale data. Acting on data the device did not send is still wrong, so
bail out per usb_id case when the reply is shorter than the bytes that
case consumes.

  read_erp: AK1 needs 2 bytes, Kore needs 16, Maschine needs 22.
  read_io:  the Kore case needs 5 bytes (buf[4]) and the Traktor Kontrol
            X1 case needs 7 (buf[5]/buf[6]). The preceding key bit loop
            is already bounded by "i < len * 8" and is left untouched.

snd_caiaq_input_read_analog() and snd_usb_caiaq_maschine_dispatch() are
not changed. Their callers already floor the reply length.

Suggested-by: Takashi Iwai <tiwai@suse.com>
Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
---
 sound/usb/caiaq/input.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/sound/usb/caiaq/input.c b/sound/usb/caiaq/input.c
index 2db4d1332df1..eabbf41fdfb2 100644
--- 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);
-- 
2.34.1


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

* Re: [PATCH 0/2] ALSA: caiaq: fix S4 OOB read and bound the EP1 input parsers
  2026-06-18  6:03 [PATCH 0/2] ALSA: caiaq: fix S4 OOB read and bound the EP1 input parsers Maoyi Xie
  2026-06-18  6:03 ` [PATCH 1/2] ALSA: caiaq: fix out-of-bounds read in the Traktor Kontrol S4 input parser Maoyi Xie
  2026-06-18  6:03 ` [PATCH 2/2] ALSA: caiaq: bound the length in the EP1 input parsers Maoyi Xie
@ 2026-06-18 10:38 ` Takashi Iwai
  2 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2026-06-18 10:38 UTC (permalink / raw)
  To: Maoyi Xie
  Cc: Daniel Mack, Jaroslav Kysela, Takashi Iwai, linux-sound,
	linux-kernel

On Thu, 18 Jun 2026 08:03:15 +0200,
Maoyi Xie wrote:
> 
> Hi Takashi,
> 
> Thanks for confirming the Traktor Kontrol S4 out-of-bounds read and for
> the follow-up on the neighbouring parsers.
> 
> Patch 1 is the actual fix. snd_usb_caiaq_tks4_dispatch() loops on the raw
> urb->actual_length. That value is controlled by the device and is not
> required to be a multiple of the 16-byte message block. Once len drops
> below 16 the unsigned "len -= TKS4_MSGBLOCK_SIZE" underflows. The loop
> then keeps walking buf past ep4_in_buf[EP4_BUFSIZE]. The fix iterates
> only while a full block remains, which also discards any trailing partial
> block. The X1 and Maschine arms already floor the length before dispatch,
> so only the S4 arm was affected.
> 
> Patch 2 adds the length checks you suggested to
> snd_caiaq_input_read_erp() and snd_caiaq_input_read_io(). Both are
> reachable through snd_usb_caiaq_input_dispatch(). As you noted,
> snd_caiaq_input_read_analog() and snd_usb_caiaq_maschine_dispatch()
> already have the length floored by their callers, so they are left
> unchanged. The two parsers patch 2 touches are not an out-of-bounds
> access either. Every offset is a fixed driver constant within the 64-byte
> ep1_in_buf. A short reply does make them decode stale data, though, so the
> guards drop such replies per device path. Patch 2 carries your
> Suggested-by.
> 
> Patch 1 carries a Fixes tag and Cc: stable. Patch 2 does not.
> 
> Maoyi Xie (2):
>   ALSA: caiaq: fix out-of-bounds read in the Traktor Kontrol S4 input
>     parser
>   ALSA: caiaq: bound the length in the EP1 input parsers

Applied both patches now.  Thanks.


Takashi

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

end of thread, other threads:[~2026-06-18 10:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-18  6:03 [PATCH 0/2] ALSA: caiaq: fix S4 OOB read and bound the EP1 input parsers Maoyi Xie
2026-06-18  6:03 ` [PATCH 1/2] ALSA: caiaq: fix out-of-bounds read in the Traktor Kontrol S4 input parser Maoyi Xie
2026-06-18  6:03 ` [PATCH 2/2] ALSA: caiaq: bound the length in the EP1 input parsers Maoyi Xie
2026-06-18 10:38 ` [PATCH 0/2] ALSA: caiaq: fix S4 OOB read and bound " Takashi Iwai

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