ALSA: seq: oss: Send fragmented SysEx messages immediately

The recent bug report spotted on the old OSS sequencer code that tries
to combine incoming SysEx messages to a single sequencer event.  This
is good, per se, but it has more demerits:

- The sysex message delivery is delayed until the very last event
- The use of internal buffer forced the serialization

The recent fix in commit 0179488ca992 ("ALSA: seq: oss: Fix races at
processing SysEx messages") addressed the latter, but a better fix is
to handle the sysex messages immediately, i.e. just send each incoming
fragmented sysex message as is.  And this patch implements that.
This resulted in a significant cleanup as well.

Note that the only caller of snd_seq_oss_synth_sysex() is
snd_seq_oss_process_event(), and all its callers dispatch the event
immediately, so we can just put the passed buffer pointer to the event
record to be handled.

Reported-and-tested-by: Kun Hu <huk23@m.fudan.edu.cn>
Link: https://lore.kernel.org/2B7E93E4-B13A-4AE4-8E87-306A8EE9BBB7@m.fudan.edu.cn
Link: https://patch.msgid.link/20241231115523.15796-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/core/seq/oss/seq_oss_device.h b/sound/core/seq/oss/seq_oss_device.h
index 98dd20b429764ce94dfc3e72542d8f0352a770d4..c0b1cce127a33c33a2d629fdb29f8c95cefe1ab0 100644 (file)
--- a/sound/core/seq/oss/seq_oss_device.h
+++ b/sound/core/seq/oss/seq_oss_device.h
@@ -55,7 +55,6 @@ struct seq_oss_chinfo {
 struct seq_oss_synthinfo {
        struct snd_seq_oss_arg arg;
        struct seq_oss_chinfo *ch;
-       struct seq_oss_synth_sysex *sysex;
        int nr_voices;
        int opened;
        int is_midi;

diff --git a/sound/core/seq/oss/seq_oss_synth.c b/sound/core/seq/oss/seq_oss_synth.c
index 51ee4c00a84310b3f40d056a7c96b6a0a20eb9ee..02a90c960992ed6caf786079da81b42645b94b84 100644 (file)
--- a/sound/core/seq/oss/seq_oss_synth.c
+++ b/sound/core/seq/oss/seq_oss_synth.c
@@ -26,13 +26,6 @@
  * definition of synth info records
  */
 
-/* sysex buffer */
-struct seq_oss_synth_sysex {
-       int len;
-       int skip;
-       unsigned char buf[MAX_SYSEX_BUFLEN];
-};
-
 /* synth info */
 struct seq_oss_synth {
        int seq_device;
@@ -66,7 +59,6 @@ static struct seq_oss_synth midi_synth_dev = {
 };
 
 static DEFINE_SPINLOCK(register_lock);
-static DEFINE_MUTEX(sysex_mutex);
 
 /*
  * prototypes
@@ -319,8 +311,6 @@ snd_seq_oss_synth_cleanup(struct seq_oss_devinfo *dp)
                        }
                        snd_use_lock_free(&rec->use_lock);
                }
-               kfree(info->sysex);
-               info->sysex = NULL;
                kfree(info->ch);
                info->ch = NULL;
        }
@@ -396,8 +386,6 @@ snd_seq_oss_synth_reset(struct seq_oss_devinfo *dp, int dev)
        info = get_synthinfo_nospec(dp, dev);
        if (!info || !info->opened)
                return;
-       if (info->sysex)
-               info->sysex->len = 0; /* reset sysex */
        reset_channels(info);
        if (info->is_midi) {
                if (midi_synth_dev.opened <= 0)
@@ -409,8 +397,6 @@ snd_seq_oss_synth_reset(struct seq_oss_devinfo *dp, int dev)
                                          dp->file_mode) < 0) {
                        midi_synth_dev.opened--;
                        info->opened = 0;
-                       kfree(info->sysex);
-                       info->sysex = NULL;
                        kfree(info->ch);
                        info->ch = NULL;
                }
@@ -483,64 +469,26 @@ snd_seq_oss_synth_info(struct seq_oss_devinfo *dp, int dev)
 
 /*
  * receive OSS 6 byte sysex packet:
- * the full sysex message will be sent if it reaches to the end of data
- * (0xff).
+ * the event is filled and prepared for sending immediately
+ * (i.e. sysex messages are fragmented)
  */
 int
 snd_seq_oss_synth_sysex(struct seq_oss_devinfo *dp, int dev, unsigned char *buf, struct snd_seq_event *ev)
 {
-       int i, send;
-       unsigned char *dest;
-       struct seq_oss_synth_sysex *sysex;
-       struct seq_oss_synthinfo *info;
+       unsigned char *p;
+       int len = 6;
 
-       info = snd_seq_oss_synth_info(dp, dev);
-       if (!info)
-               return -ENXIO;
+       p = memchr(buf, 0xff, 6);
+       if (p)
+               len = p - buf + 1;
 
-       guard(mutex)(&sysex_mutex);
-       sysex = info->sysex;
-       if (sysex == NULL) {
-               sysex = kzalloc(sizeof(*sysex), GFP_KERNEL);
-               if (sysex == NULL)
-                       return -ENOMEM;
-               info->sysex = sysex;
-       }
-
-       send = 0;
-       dest = sysex->buf + sysex->len;
-       /* copy 6 byte packet to the buffer */
-       for (i = 0; i < 6; i++) {
-               if (buf[i] == 0xff) {
-                       send = 1;
-                       break;
-               }
-               dest[i] = buf[i];
-               sysex->len++;
-               if (sysex->len >= MAX_SYSEX_BUFLEN) {
-                       sysex->len = 0;
-                       sysex->skip = 1;
-                       break;
-               }
-       }
-
-       if (sysex->len && send) {
-               if (sysex->skip) {
-                       sysex->skip = 0;
-                       sysex->len = 0;
-                       return -EINVAL; /* skip */
-               }
-               /* copy the data to event record and send it */
-               ev->flags = SNDRV_SEQ_EVENT_LENGTH_VARIABLE;
-               if (snd_seq_oss_synth_addr(dp, dev, ev))
-                       return -EINVAL;
-               ev->data.ext.len = sysex->len;
-               ev->data.ext.ptr = sysex->buf;
-               sysex->len = 0;
-               return 0;
-       }
-
-       return -EINVAL; /* skip */
+       /* copy the data to event record and send it */
+       if (snd_seq_oss_synth_addr(dp, dev, ev))
+               return -EINVAL;
+       ev->flags = SNDRV_SEQ_EVENT_LENGTH_VARIABLE;
+       ev->data.ext.len = len;
+       ev->data.ext.ptr = buf;
+       return 0;
 }
 
 /*