Commit 20ce9ded authored by Takashi Iwai's avatar Takashi Iwai
Browse files

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 0179488c ("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: default avatarKun 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: default avatarTakashi Iwai <tiwai@suse.de>
parent 41d11d6e

sound/core/seq/oss/seq_oss_device.h

+0 −1
Original line number Diff line number Diff line
@@ -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;

sound/core/seq/oss/seq_oss_synth.c

+14 −66
Original line number Diff line number Diff line
@@ -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,66 +469,28 @@ 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;



	info = snd_seq_oss_synth_info(dp, dev);

	if (!info)

		return -ENXIO;



	guard(mutex)(&sysex_mutex);

	sysex = info->sysex;

	if (sysex == NULL) {

		sysex = kzalloc(sizeof(*sysex), GFP_KERNEL);

		if (sysex == NULL)

			return -ENOMEM;

		info->sysex = sysex;

	}

	unsigned char *p;

	int len = 6;



	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;

		}

	}

	p = memchr(buf, 0xff, 6);

	if (p)

		len = p - buf + 1;



	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;

	ev->flags = SNDRV_SEQ_EVENT_LENGTH_VARIABLE;

	ev->data.ext.len = len;

	ev->data.ext.ptr = buf;

	return 0;

}



	return -EINVAL; /* skip */

}



/*

 * fill the event source/destination addresses

 */