[PATCH] ALSA: usb-audio: fix OOB write in snd_usbmidi_akai_output()

[PATCH] ALSA: usb-audio: fix OOB write in snd_usbmidi_akai_output()

[Baul Lee]

snd_usbmidi_akai_output() computes its fill-loop bound

	buf_end = ep->max_transfer - MAX_AKAI_SYSEX_LEN - 1;

as a signed int, so a small device-advertised bulk-OUT max_transfer
makes buf_end negative.  The loop guard then compares the u32
urb->transfer_buffer_length against that negative int: the usual
arithmetic conversion turns buf_end into a large unsigned value, so the
guard stays true and each iteration keeps appending SysEx framing and
payload bytes past the end of the URB transfer buffer, which is only
max_transfer bytes long.

A USB device that advertises a tiny bulk-OUT endpoint can therefore
trigger an attacker-length- and content-controlled heap out-of-bounds
write when a process writes to the created /dev/snd/midiC*D* node.

Perform the comparison in signed arithmetic so that a negative buf_end
stops the loop instead of wrapping to a huge unsigned bound.

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

Fixes: 4434ade8c933 ("ALSA: usb-audio: add support for Akai MPD16")
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/midi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/usb/midi.c b/sound/usb/midi.c
index d87e3f357cf7..cc7df77632a1 100644
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -799,7 +799,7 @@ static void snd_usbmidi_akai_output(struct snd_usb_midi_out_endpoint *ep,
 	buf_end = ep->max_transfer - MAX_AKAI_SYSEX_LEN - 1;
 
 	/* only try adding more data when there's space for at least 1 SysEx */
-	while (urb->transfer_buffer_length < buf_end) {
+	while ((int)urb->transfer_buffer_length < buf_end) {
 		count = snd_rawmidi_transmit_peek(substream,
 						  tmp, MAX_AKAI_SYSEX_LEN);
 		if (!count) {
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] ALSA: usb-audio: fix OOB write in snd_usbmidi_akai_output()

[Takashi Iwai]

On Sun, 26 Jul 2026 07:20:40 +0200,
Baul Lee wrote:
> 
> snd_usbmidi_akai_output() computes its fill-loop bound
> 
> 	buf_end = ep->max_transfer - MAX_AKAI_SYSEX_LEN - 1;
> 
> as a signed int, so a small device-advertised bulk-OUT max_transfer
> makes buf_end negative.  The loop guard then compares the u32
> urb->transfer_buffer_length against that negative int: the usual
> arithmetic conversion turns buf_end into a large unsigned value, so the
> guard stays true and each iteration keeps appending SysEx framing and
> payload bytes past the end of the URB transfer buffer, which is only
> max_transfer bytes long.
> 
> A USB device that advertises a tiny bulk-OUT endpoint can therefore
> trigger an attacker-length- and content-controlled heap out-of-bounds
> write when a process writes to the created /dev/snd/midiC*D* node.
> 
> Perform the comparison in signed arithmetic so that a negative buf_end
> stops the loop instead of wrapping to a huge unsigned bound.
> 
> Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>
> 
> Fixes: 4434ade8c933 ("ALSA: usb-audio: add support for Akai MPD16")
> 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/midi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/usb/midi.c b/sound/usb/midi.c
> index d87e3f357cf7..cc7df77632a1 100644
> --- a/sound/usb/midi.c
> +++ b/sound/usb/midi.c
> @@ -799,7 +799,7 @@ static void snd_usbmidi_akai_output(struct snd_usb_midi_out_endpoint *ep,
>  	buf_end = ep->max_transfer - MAX_AKAI_SYSEX_LEN - 1;
>  
>  	/* only try adding more data when there's space for at least 1 SysEx */
> -	while (urb->transfer_buffer_length < buf_end) {
> +	while ((int)urb->transfer_buffer_length < buf_end) {
>  		count = snd_rawmidi_transmit_peek(substream,
>  						  tmp, MAX_AKAI_SYSEX_LEN);
>  		if (!count) {

The code change looks correct, but I believe the intention can be
understandable by a more explicit check like the below.
Could you verify it and submit v2 if it works?


thanks,

Takashi

-- 8< --
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -797,6 +797,8 @@ static void snd_usbmidi_akai_output(struct snd_usb_midi_out_endpoint *ep,
 
 	msg = urb->transfer_buffer + urb->transfer_buffer_length;
 	buf_end = ep->max_transfer - MAX_AKAI_SYSEX_LEN - 1;
+	if (buf_end <= 0)
+		return;
 
 	/* only try adding more data when there's space for at least 1 SysEx */
 	while (urb->transfer_buffer_length < buf_end) {

[PATCH v2] ALSA: usb-audio: fix OOB write in snd_usbmidi_akai_output()

[Baul Lee]

snd_usbmidi_akai_output() computes its fill-loop bound

	buf_end = ep->max_transfer - MAX_AKAI_SYSEX_LEN - 1;

as a signed int, so a small device-advertised bulk-OUT max_transfer
makes buf_end negative.  The loop guard then compares the u32
urb->transfer_buffer_length against that negative int: the usual
arithmetic conversion turns buf_end into a large unsigned value, so the
guard stays true and each iteration keeps appending SysEx framing and
payload bytes past the end of the URB transfer buffer, which is only
max_transfer bytes long.

A USB device that advertises a tiny bulk-OUT endpoint can therefore
trigger an attacker-length- and content-controlled heap out-of-bounds
write when a process writes to the created /dev/snd/midiC*D* node.

Return early when there is no room for even one SysEx, so the loop is
never entered with a bound that would wrap.  The loop is the last
statement of the function, so bailing out is equivalent to it not
running.

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

Fixes: 4434ade8c933 ("ALSA: usb-audio: add support for Akai MPD16")
Suggested-by: Takashi Iwai <tiwai@suse.de>
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>
---
v2: use an explicit "buf_end <= 0" early return instead of casting the
    loop guard to int, as suggested by Takashi Iwai. Verified with the
    reproducer under KASAN on v7.2-rc4: without the patch the kernel
    reports a slab-out-of-bounds write in snd_usbmidi_akai_output(),
    with it the same run is clean.

 sound/usb/midi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/usb/midi.c b/sound/usb/midi.c
index d87e3f357cf7..f8996416c3be 100644
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -797,6 +797,8 @@ static void snd_usbmidi_akai_output(struct snd_usb_midi_out_endpoint *ep,
 
 	msg = urb->transfer_buffer + urb->transfer_buffer_length;
 	buf_end = ep->max_transfer - MAX_AKAI_SYSEX_LEN - 1;
+	if (buf_end <= 0)
+		return;
 
 	/* only try adding more data when there's space for at least 1 SysEx */
 	while (urb->transfer_buffer_length < buf_end) {
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v2] ALSA: usb-audio: fix OOB write in snd_usbmidi_akai_output()

[Takashi Iwai]

On Sun, 26 Jul 2026 09:45:00 +0200,
Baul Lee wrote:
> 
> snd_usbmidi_akai_output() computes its fill-loop bound
> 
> 	buf_end = ep->max_transfer - MAX_AKAI_SYSEX_LEN - 1;
> 
> as a signed int, so a small device-advertised bulk-OUT max_transfer
> makes buf_end negative.  The loop guard then compares the u32
> urb->transfer_buffer_length against that negative int: the usual
> arithmetic conversion turns buf_end into a large unsigned value, so the
> guard stays true and each iteration keeps appending SysEx framing and
> payload bytes past the end of the URB transfer buffer, which is only
> max_transfer bytes long.
> 
> A USB device that advertises a tiny bulk-OUT endpoint can therefore
> trigger an attacker-length- and content-controlled heap out-of-bounds
> write when a process writes to the created /dev/snd/midiC*D* node.
> 
> Return early when there is no room for even one SysEx, so the loop is
> never entered with a bound that would wrap.  The loop is the last
> statement of the function, so bailing out is equivalent to it not
> running.
> 
> Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>
> 
> Fixes: 4434ade8c933 ("ALSA: usb-audio: add support for Akai MPD16")
> Suggested-by: Takashi Iwai <tiwai@suse.de>
> 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>
> ---
> v2: use an explicit "buf_end <= 0" early return instead of casting the
>     loop guard to int, as suggested by Takashi Iwai. Verified with the
>     reproducer under KASAN on v7.2-rc4: without the patch the kernel
>     reports a slab-out-of-bounds write in snd_usbmidi_akai_output(),
>     with it the same run is clean.

Applied now.  Thanks!


Takashi