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

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

[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 <arteme@gmail.com>
---
 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(-)

diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
index 59faa5a9a714..4dbe7bce3ee8 100644
--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -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;
diff --git a/sound/usb/line6/midi.c b/sound/usb/line6/midi.c
index ba0e2b7e8fe1..335f8d531548 100644
--- a/sound/usb/line6/midi.c
+++ b/sound/usb/line6/midi.c
@@ -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;
diff --git a/sound/usb/line6/midibuf.c b/sound/usb/line6/midibuf.c
index 6a70463f82c4..ba7a2243cf68 100644
--- a/sound/usb/line6/midibuf.c
+++ b/sound/usb/line6/midibuf.c
@@ -9,6 +9,22 @@
 
 #include "midibuf.h"
 
+/* #define DUMP_BUFFERS */
+
+#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");
+}
+#else
+#define dump_buffer(rx, data, length) /* nothing */
+#endif
+
+
 static int midibuf_message_length(unsigned char code)
 {
 	int message_length;
@@ -20,12 +36,7 @@ static int midibuf_message_length(unsigned char code)
 
 		message_length = length[(code >> 4) - 8];
 	} else {
-		/*
-		   Note that according to the MIDI specification 0xf2 is
-		   the "Song Position Pointer", but this is used by Line 6
-		   to send sysex messages to the host.
-		 */
-		static const int length[] = { -1, 2, -1, 2, -1, -1, 1, 1, 1, 1,
+		static const int length[] = { -1, 2, 2, 2, -1, -1, 1, 1, 1, -1,
 			1, 1, 1, -1, 1, 1
 		};
 		message_length = length[code & 0x0f];
@@ -125,7 +136,7 @@ int line6_midibuf_write(struct midi_buffer *this, unsigned char *data,
 }
 
 int line6_midibuf_read(struct midi_buffer *this, unsigned char *data,
-		       int length)
+		       int length, char rx)
 {
 	int bytes_used;
 	int length1, length2;
@@ -148,9 +159,22 @@ int line6_midibuf_read(struct midi_buffer *this, unsigned char *data,
 
 	length1 = this->size - this->pos_read;
 
-	/* check MIDI command length */
 	command = this->buf[this->pos_read];
+	/*
+	   PODxt always has status byte lower nibble set to 0010,
+	   when it means to send 0000, so we correct if here so
+	   that control/program changes come on channel 1 and
+	   sysex message status byte is correct
+	 */
+	if (rx) {
+		if (command == 0xb2 || command == 0xc2 || command == 0xf2) {
+			unsigned char fixed = command & 0xf0;
+			this->buf[this->pos_read] = fixed;
+			command = fixed;
+		}
+	}
 
+	/* check MIDI command length */
 	if (command & 0x80) {
 		midi_length = midibuf_message_length(command);
 		this->command_prev = command;
@@ -222,6 +246,8 @@ int line6_midibuf_read(struct midi_buffer *this, unsigned char *data,
 		this->pos_read = length2;
 	}
 
+	dump_buffer(rx, data, length + repeat);
+
 	if (repeat)
 		data[0] = this->command_prev;
 
diff --git a/sound/usb/line6/midibuf.h b/sound/usb/line6/midibuf.h
index 124a8f9f7e96..8342e45046b0 100644
--- a/sound/usb/line6/midibuf.h
+++ b/sound/usb/line6/midibuf.h
@@ -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);
diff --git a/sound/usb/line6/pod.c b/sound/usb/line6/pod.c
index cd41aa7f0385..d173971e5f02 100644
--- a/sound/usb/line6/pod.c
+++ b/sound/usb/line6/pod.c
@@ -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,
-- 
2.38.1

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

[Artem Egorkine]

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 <arteme@gmail.com>
---
 sound/usb/line6/midi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/usb/line6/midi.c b/sound/usb/line6/midi.c
index 335f8d531548..fbcb55b6e8b0 100644
--- a/sound/usb/line6/midi.c
+++ b/sound/usb/line6/midi.c
@@ -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)
-- 
2.38.1

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

[Greg KH]

On Fri, Dec 16, 2022 at 12:02:38PM +0200, Artem Egorkine wrote:
> 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 <arteme@gmail.com>
> ---
>  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 <perex@perex.cz> (maintainer:SOUND)
Takashi Iwai <tiwai@suse.com> (maintainer:SOUND)
alsa-devel@alsa-project.org (moderated list:SOUND)
linux-kernel@vger.kernel.org (open list)

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

[Greg KH]

On Fri, Dec 16, 2022 at 12:02:38PM +0200, Artem Egorkine wrote:
> 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 <arteme@gmail.com>
> ---
>  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(-)
> 
> diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
> index 59faa5a9a714..4dbe7bce3ee8 100644
> --- a/sound/usb/line6/driver.c
> +++ b/sound/usb/line6/driver.c
> @@ -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;
> diff --git a/sound/usb/line6/midi.c b/sound/usb/line6/midi.c
> index ba0e2b7e8fe1..335f8d531548 100644
> --- a/sound/usb/line6/midi.c
> +++ b/sound/usb/line6/midi.c
> @@ -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.

>  
>  		if (done == 0)
>  			break;
> diff --git a/sound/usb/line6/midibuf.c b/sound/usb/line6/midibuf.c
> index 6a70463f82c4..ba7a2243cf68 100644
> --- a/sound/usb/line6/midibuf.c
> +++ b/sound/usb/line6/midibuf.c
> @@ -9,6 +9,22 @@
>  
>  #include "midibuf.h"
>  
> +/* #define DUMP_BUFFERS */
> +
> +#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

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

[Greg KH]

On Fri, Dec 16, 2022 at 12:02:38PM +0200, Artem Egorkine wrote:
> 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 <arteme@gmail.com>

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

thanks,

greg k-h