sound: line6: correct midi status byte when receiving data from podxt

PATCH In review Artem Egorkine 3 participants 5 messages Dec 16, 2022

Subsystems (2)

USB Sound

lore

Share & tools

lore

Artem Egorkine

author

2022-12-16 10:02 UTC #0 lore raw

PATCH 1/2 sound: line6: correct midi status byte when receiving data from podxt

A PODxt device sends 0xb2, 0xc2 or 0xf2 as a status byte for MIDI messages over USB that should otherwise have a 0xb0, 0xc0 or 0xf0 status byte. This is usually corrected by the driver on other OSes.

This fixes MIDI sysex messages sent by PODxt.

Signed-off-by Artem Egorkine <[email protected]>

---
 sound/usb/line6/driver.c  |  2 +-
 sound/usb/line6/midi.c    |  2 +-
 sound/usb/line6/midibuf.c | 42 +++++++++++++++++++++++++++++++--------
 sound/usb/line6/midibuf.h |  2 +-
 sound/usb/line6/pod.c     |  3 ++-
 5 files changed, 39 insertions(+), 12 deletions(-)

sound/usb/line6/driver.c +1 -1

index 59faa5a9a714..4dbe7bce3ee8 100644
@@ -304,7 +304,7 @@ static void line6_data_received(struct urb *urb)
 		for (;;) {
 			done =
 				line6_midibuf_read(mb, line6->buffer_message,
-						LINE6_MIDI_MESSAGE_MAXLEN);
+						LINE6_MIDI_MESSAGE_MAXLEN, 1);
 
 			if (done <= 0)
 				break;

sound/usb/line6/midi.c +1 -1

index ba0e2b7e8fe1..335f8d531548 100644
@@ -56,7 +56,7 @@ static void line6_midi_transmit(struct snd_rawmidi_substream *substream)
 
 	for (;;) {
 		done = line6_midibuf_read(mb, chunk,
-					  LINE6_FALLBACK_MAXPACKETSIZE);
+					  LINE6_FALLBACK_MAXPACKETSIZE, 0);
 
 		if (done == 0)
 			break;

sound/usb/line6/midibuf.c +34 -8

74 lines not on this page · raw message

sound/usb/line6/midibuf.h +1 -1

index 124a8f9f7e96..8342e45046b0 100644
@@ -23,7 +23,7 @@ extern void line6_midibuf_destroy(struct midi_buffer *mb);
 extern int line6_midibuf_ignore(struct midi_buffer *mb, int length);
 extern int line6_midibuf_init(struct midi_buffer *mb, int size, int split);
 extern int line6_midibuf_read(struct midi_buffer *mb, unsigned char *data,
-			      int length);
+			      int length, char rx);
 extern void line6_midibuf_reset(struct midi_buffer *mb);
 extern int line6_midibuf_write(struct midi_buffer *mb, unsigned char *data,
 			       int length);

sound/usb/line6/pod.c +2 -1

index cd41aa7f0385..d173971e5f02 100644
@@ -159,8 +159,9 @@ static struct line6_pcm_properties pod_pcm_properties = {
 	.bytes_per_channel = 3 /* SNDRV_PCM_FMTBIT_S24_3LE */
 };
 
+
 static const char pod_version_header[] = {
-	0xf2, 0x7e, 0x7f, 0x06, 0x02
+	0xf0, 0x7e, 0x7f, 0x06, 0x02
 };
 
 static char *pod_alloc_sysex_buffer(struct usb_line6_pod *pod, int code,

Signature · 1 line

2.38.1

Artem Egorkine

author

2022-12-16 10:03 UTC #1 lore raw

PATCH 2/2 sound: line6: fix stack overflow in line6_midi_transmit

Correctly calculate available space including the size of the chunk buffer. This fixes a buffer overflow when multiple MIDI sysex messages are sent to a PODxt device.

Signed-off-by Artem Egorkine <[email protected]>

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

sound/usb/line6/midi.c +2 -1

index 335f8d531548..fbcb55b6e8b0 100644
@@ -44,7 +44,8 @@ static void line6_midi_transmit(struct snd_rawmidi_substream *substream)
 	int req, done;
 
 	for (;;) {
-		req = min(line6_midibuf_bytes_free(mb), line6->max_packet_size);
+		req = min3(line6_midibuf_bytes_free(mb), line6->max_packet_size,
+			   LINE6_FALLBACK_MAXPACKETSIZE);
 		done = snd_rawmidi_transmit_peek(substream, chunk, req);
 
 		if (done == 0)

Signature · 1 line

2.38.1

Greg KH

maintainer

2022-12-16 10:59 UTC #2 lore raw

In reply to Artem Egorkine

Quoted 6 lines · Artem Egorkine

A PODxt device sends 0xb2, 0xc2 or 0xf2 as a status byte for MIDI messages over USB that should otherwise have a 0xb0, 0xc0 or 0xf0 status byte. This is usually corrected by the driver on other OSes.
This fixes MIDI sysex messages sent by PODxt.

Quoted 8 lines

Signed-off-by Artem Egorkine <[email protected]>

---
 sound/usb/line6/driver.c  |  2 +-
 sound/usb/line6/midi.c    |  2 +-
 sound/usb/line6/midibuf.c | 42 +++++++++++++++++++++++++++++++--------
 sound/usb/line6/midibuf.h |  2 +-
 sound/usb/line6/pod.c     |  3 ++-
 5 files changed, 39 insertions(+), 12 deletions(-)

You sent this to the wrong list and developers:

$ ./scripts/get_maintainer.pl --file sound/usb/line6/midibuf.c
Jaroslav Kysela <[email protected]> (maintainer:SOUND)
Takashi Iwai <[email protected]> (maintainer:SOUND)
[email protected] (moderated list:SOUND)
[email protected] (open list)

Greg KH

2022-12-16 11:00 UTC #3 lore raw

In reply to Artem Egorkine

Quoted 33 lines · Artem Egorkine

A PODxt device sends 0xb2, 0xc2 or 0xf2 as a status byte for MIDI messages over USB that should otherwise have a 0xb0, 0xc0 or 0xf0 status byte. This is usually corrected by the driver on other OSes.
This fixes MIDI sysex messages sent by PODxt.
Signed-off-by Artem Egorkine <[email protected]>
---
 sound/usb/line6/driver.c  |  2 +-
 sound/usb/line6/midi.c    |  2 +-
 sound/usb/line6/midibuf.c | 42 +++++++++++++++++++++++++++++++--------
 sound/usb/line6/midibuf.h |  2 +-
 sound/usb/line6/pod.c     |  3 ++-
 5 files changed, 39 insertions(+), 12 deletions(-)
sound/usb/line6/driver.c +1 -1
index 59faa5a9a714..4dbe7bce3ee8 100644
@@ -304,7 +304,7 @@ static void line6_data_received(struct urb *urb)
 		for (;;) {
 			done =
 				line6_midibuf_read(mb, line6->buffer_message,
-						LINE6_MIDI_MESSAGE_MAXLEN);
+						LINE6_MIDI_MESSAGE_MAXLEN, 1);
 
 			if (done <= 0)
 				break;
sound/usb/line6/midi.c +0 -0
index ba0e2b7e8fe1..335f8d531548 100644

sound/usb/line6/midi.c static void line6_midi_transmit(struct snd_rawmidi_substream *substream) · line 59

Quoted 5 lines

sound/usb/line6/midi.c +1 -1

@@ -56,7 +56,7 @@ static void line6_midi_transmit(struct snd_rawmidi_substream *substream)
 
 	for (;;) {
 		done = line6_midibuf_read(mb, chunk,
-					  LINE6_FALLBACK_MAXPACKETSIZE);
+					  LINE6_FALLBACK_MAXPACKETSIZE, 0);

This random number at the end of the function is not good, please make it more obvious what has to happen here otherwise every time you see this call you have to go look up what it means.

Quoted 12 lines

sound/usb/line6/midi.c +0 -0

 		if (done == 0)
 			break;

sound/usb/line6/midibuf.c +1 -0

index 6a70463f82c4..ba7a2243cf68 100644
@@ -9,6 +9,22 @@
 
 #include "midibuf.h"
 
+/* #define DUMP_BUFFERS */

sound/usb/line6/midibuf.c line 22

Quoted 10 lines

sound/usb/line6/midibuf.c +10 -0

+
+#ifdef DUMP_BUFFERS
+static void dump_buffer(char rx, const u8 *data, int length)
+{
+	const char* type = rx ? "rx" : "tx";
+        printk(KERN_DEBUG "%s packet: [", type);
+        for (; length > 0; ++data, --length)
+                printk(KERN_CONT " %02x", *data);
+        printk(KERN_CONT " ]\n");
+}

Coding style issues :(

Also, there is a "print a buffer to the debug log" command already, why not use that? And you need to use dev_dbg() to show the device that this came from, right?

thanks,

greg k-h

Greg KH

2022-12-16 11:01 UTC #4 lore raw

In reply to Artem Egorkine

Quoted 7 lines · Artem Egorkine

A PODxt device sends 0xb2, 0xc2 or 0xf2 as a status byte for MIDI messages over USB that should otherwise have a 0xb0, 0xc0 or 0xf0 status byte. This is usually corrected by the driver on other OSes.
This fixes MIDI sysex messages sent by PODxt.
Signed-off-by Artem Egorkine <[email protected]>

Note, you also add debug logic here, that should be a separate patch, right?

thanks,

greg k-h