Re: Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

[Kun Hu]

> 
> The only place incrementing sysex->len is that loop and it has already
> the bounce check which resets sysex->len to 0.  That is, the likely
> reason of the buffer overflow is rather the racy calls of this
> function.
> 
> If so, a potential fix would be rather to protect the concurrency,
> e.g. a simplest form is something like below.
> Could you check whether it addresses your problem?
> 
> 
> thanks,
> 
> Takashi
> 
> -- 8< --
> --- a/sound/core/seq/oss/seq_oss_synth.c
> +++ b/sound/core/seq/oss/seq_oss_synth.c
> @@ -66,6 +66,7 @@ static struct seq_oss_synth midi_synth_dev = {
> };
> 
> static DEFINE_SPINLOCK(register_lock);
> +static DEFINE_MUTEX(sysex_mutex);
> 
> /*
>  * prototypes
> @@ -497,6 +498,7 @@ snd_seq_oss_synth_sysex(struct seq_oss_devinfo *dp, int dev, unsigned char *buf,
> if (!info)
> return -ENXIO;
> 
> + guard(mutex)(&sysex_mutex);
> sysex = info->sysex;
> if (sysex == NULL) {
> sysex = kzalloc(sizeof(*sysex), GFP_KERNEL);

Hi,

I did a lot of reproduction testing and it was very easy to reproduce before the lock was added, after the lock was added the error was no longer reproducible.

Thanks,
Kun Hu