[PATCH] ALSA: 6fire: bound the MIDI event length from the device

[PATCH] ALSA: 6fire: bound the MIDI event length from the device

[Baul Lee]

usb6fire_comm_receiver_handler() forwards a MIDI event using a length
byte the device supplies, with no bound and no check that the transfer
delivered that many bytes:

	if (!urb->status) {
		if (rt->receiver_buffer[0] == 0x10) /* midi in event */
			if (midi_rt)
				midi_rt->in_received(midi_rt,
						rt->receiver_buffer + 2,
						rt->receiver_buffer[1]);
	}

receiver_buffer is a 64-byte kzalloc() buffer (COMM_RECEIVER_BUFSIZE), so
only 62 bytes follow the two-byte header.  receiver_buffer[1] is a u8 the
device chooses, so a device that answers with 0x10 and a length of 0xFF
makes snd_rawmidi_receive() read 255 bytes starting two bytes into a
64-byte object.  The bytes past the buffer are handed to userspace
through the rawmidi read path.

urb->actual_length is not consulted either, so a short transfer leaves
both the type byte and the length byte at their previous values and the
handler acts on stale data.

The receiver URB is submitted from usb6fire_comm_init() at probe, so the
read happens on plug with no user action; forwarding to userspace also
needs a MIDI input substream open, since usb6fire_midi_in_received()
only calls snd_rawmidi_receive() when rt->in is set.

KASAN on 7.2.0-rc5 (arm64), single packet from an emulated device:

  BUG: KASAN: slab-out-of-bounds in snd_rawmidi_receive
  Read of size 255 at addr ffff000009f64682 by task bash/183
   __asan_memcpy
   snd_rawmidi_receive
   usb6fire_midi_in_received [snd_usb_6fire]
   usb6fire_comm_receiver_handler [snd_usb_6fire]
  Allocated by task 11:
   usb6fire_comm_init [snd_usb_6fire]
   usb6fire_chip_probe [snd_usb_6fire]
  The buggy address is located 2 bytes inside of
   allocated 64-byte region [ffff000009f64680, ffff000009f646c0)

Reject the event when the length exceeds the bytes that follow the
header, and require the transfer to have delivered the header plus that
many bytes.  The receiver URB is submitted with a 64-byte
transfer_buffer_length, so a genuine device cannot deliver an event
longer than those 62 bytes and nothing valid is dropped.

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

Fixes: c6d43ba816d1 ("ALSA: usb/6fire - Driver for TerraTec DMX 6Fire USB")
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>
---
 sound/usb/6fire/comm.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/sound/usb/6fire/comm.c b/sound/usb/6fire/comm.c
index 9a0b85653c61..d3b7cab85699 100644
--- a/sound/usb/6fire/comm.c
+++ b/sound/usb/6fire/comm.c
@@ -33,17 +33,20 @@ static void usb6fire_comm_init_urb(struct comm_runtime *rt, struct urb *urb,
 static void usb6fire_comm_receiver_handler(struct urb *urb)
 {
 	struct comm_runtime *rt = urb->context;
 	struct midi_runtime *midi_rt = rt->chip->midi;
 
 	if (!urb->status) {
-		if (rt->receiver_buffer[0] == 0x10) /* midi in event */
+		u8 len = rt->receiver_buffer[1];
+
+		if (rt->receiver_buffer[0] == 0x10 && /* midi in event */
+		    len <= COMM_RECEIVER_BUFSIZE - 2 &&
+		    urb->actual_length >= len + 2)
 			if (midi_rt)
 				midi_rt->in_received(midi_rt,
-						rt->receiver_buffer + 2,
-						rt->receiver_buffer[1]);
+						rt->receiver_buffer + 2, len);
 	}
 
 	if (!rt->chip->shutdown) {
 		urb->status = 0;
 		urb->actual_length = 0;
 		if (usb_submit_urb(urb, GFP_ATOMIC) < 0)
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] ALSA: 6fire: bound the MIDI event length from the device

[Takashi Iwai]

On Wed, 05 Aug 2026 03:34:23 +0200,
Baul Lee wrote:
> 
> usb6fire_comm_receiver_handler() forwards a MIDI event using a length
> byte the device supplies, with no bound and no check that the transfer
> delivered that many bytes:
> 
> 	if (!urb->status) {
> 		if (rt->receiver_buffer[0] == 0x10) /* midi in event */
> 			if (midi_rt)
> 				midi_rt->in_received(midi_rt,
> 						rt->receiver_buffer + 2,
> 						rt->receiver_buffer[1]);
> 	}
> 
> receiver_buffer is a 64-byte kzalloc() buffer (COMM_RECEIVER_BUFSIZE), so
> only 62 bytes follow the two-byte header.  receiver_buffer[1] is a u8 the
> device chooses, so a device that answers with 0x10 and a length of 0xFF
> makes snd_rawmidi_receive() read 255 bytes starting two bytes into a
> 64-byte object.  The bytes past the buffer are handed to userspace
> through the rawmidi read path.
> 
> urb->actual_length is not consulted either, so a short transfer leaves
> both the type byte and the length byte at their previous values and the
> handler acts on stale data.
> 
> The receiver URB is submitted from usb6fire_comm_init() at probe, so the
> read happens on plug with no user action; forwarding to userspace also
> needs a MIDI input substream open, since usb6fire_midi_in_received()
> only calls snd_rawmidi_receive() when rt->in is set.
> 
> KASAN on 7.2.0-rc5 (arm64), single packet from an emulated device:
> 
>   BUG: KASAN: slab-out-of-bounds in snd_rawmidi_receive
>   Read of size 255 at addr ffff000009f64682 by task bash/183
>    __asan_memcpy
>    snd_rawmidi_receive
>    usb6fire_midi_in_received [snd_usb_6fire]
>    usb6fire_comm_receiver_handler [snd_usb_6fire]
>   Allocated by task 11:
>    usb6fire_comm_init [snd_usb_6fire]
>    usb6fire_chip_probe [snd_usb_6fire]
>   The buggy address is located 2 bytes inside of
>    allocated 64-byte region [ffff000009f64680, ffff000009f646c0)
> 
> Reject the event when the length exceeds the bytes that follow the
> header, and require the transfer to have delivered the header plus that
> many bytes.  The receiver URB is submitted with a 64-byte
> transfer_buffer_length, so a genuine device cannot deliver an event
> longer than those 62 bytes and nothing valid is dropped.
> 
> Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>
> 
> Fixes: c6d43ba816d1 ("ALSA: usb/6fire - Driver for TerraTec DMX 6Fire USB")
> 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 to for-next branch now.  Thanks.


Takashi