Re: BUG: unable to handle kernel paging request in snd_seq_oss_readq_puts

[Takashi Iwai]

On Thu, 02 Nov 2017 09:58:04 +0100,
Takashi Iwai wrote:
> 
> On Wed, 01 Nov 2017 20:49:10 +0100,
> Takashi Iwai wrote:
> > 
> > On Wed, 01 Nov 2017 19:39:46 +0100,
> > Dmitry Vyukov wrote:
> > > 
> > > On Wed, Nov 1, 2017 at 9:38 PM, syzbot
> > > <bot+31681772ec7a18dde8d3f8caf475f361a89b9514@xxxxxxxxxxxxxxxxxxxxxxxxx>
> > > wrote:
> > > > Hello,
> > > >
> > > > syzkaller hit the following crash on
> > > > fc2e8b1a47c14b22c33eb087fca0db58e1f4ed0e
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master
> > > > compiler: gcc (GCC) 7.1.1 20170620
> > > > .config is attached
> > > > Raw console output is attached.
> > > > C reproducer is attached
> > > > syzkaller reproducer is attached. See https://goo.gl/kgGztJ
> > > > for information about syzkaller reproducers
> > > 
> > > This also happened on more recent commits, including upstream
> > > 9c323bff13f92832e03657cabdd70d731408d621 (Oct 20):
> > 
> > Could you try the patch below with CONFIG_SND_DEBUG=y and see whether
> > it catches any bad calls?  It's already in for-next branch for 4.15.
> 
> And now I remember that it's the exactly same bug Mark Salyzyn
> reported sometime ago, msgid
>  <20171006171731.88889-1-salyzyn@xxxxxxxxxxx>
> 
> So this is likely hitting an invalid page, if Mark's analysis is
> correct.

I figured out that it's the badly handled SYSEX event delivery in OSS
sequencer code.  The fix patch is below.

#syz fix: ALSA: seq: Fix OSS sysex delivery in OSS emulation


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@xxxxxxx>
Subject: [PATCH] ALSA: seq: Fix OSS sysex delivery in OSS emulation

The SYSEX event delivery in OSS sequencer emulation assumed that the
event is encoded in the variable-length data with the straight
buffering.  This was the normal behavior in the past, but during the
development, the chained buffers were introduced for carrying more
data, while the OSS code was left intact.  As a result, when a SYSEX
event with the chained buffer data is passed to OSS sequencer port,
it may end up with the wrong memory access, as if it were having a too
large buffer.

This patch addresses the bug, by applying the buffer data expansion by
the generic snd_seq_dump_var_event() helper function.

Reported-by: syzbot <syzkaller@xxxxxxxxxxxxxxxx>
Reported-by: Mark Salyzyn <salyzyn@xxxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
---
 sound/core/seq/oss/seq_oss_midi.c  |  4 +---
 sound/core/seq/oss/seq_oss_readq.c | 29 +++++++++++++++++++++++++++++
 sound/core/seq/oss/seq_oss_readq.h |  2 ++
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/sound/core/seq/oss/seq_oss_midi.c b/sound/core/seq/oss/seq_oss_midi.c
index aaff9ee32695..b30b2139e3f0 100644
--- a/sound/core/seq/oss/seq_oss_midi.c
+++ b/sound/core/seq/oss/seq_oss_midi.c
@@ -612,9 +612,7 @@ send_midi_event(struct seq_oss_devinfo *dp, struct snd_seq_event *ev, struct seq
 	if (!dp->timer->running)
 		len = snd_seq_oss_timer_start(dp->timer);
 	if (ev->type == SNDRV_SEQ_EVENT_SYSEX) {
-		if ((ev->flags & SNDRV_SEQ_EVENT_LENGTH_MASK) == SNDRV_SEQ_EVENT_LENGTH_VARIABLE)
-			snd_seq_oss_readq_puts(dp->readq, mdev->seq_device,
-					       ev->data.ext.ptr, ev->data.ext.len);
+		snd_seq_oss_readq_sysex(dp->readq, mdev->seq_device, ev);
 	} else {
 		len = snd_midi_event_decode(mdev->coder, msg, sizeof(msg), ev);
 		if (len > 0)
diff --git a/sound/core/seq/oss/seq_oss_readq.c b/sound/core/seq/oss/seq_oss_readq.c
index 046cb586fb2f..06b21226b4e7 100644
--- a/sound/core/seq/oss/seq_oss_readq.c
+++ b/sound/core/seq/oss/seq_oss_readq.c
@@ -117,6 +117,35 @@ snd_seq_oss_readq_puts(struct seq_oss_readq *q, int dev, unsigned char *data, in
 	return 0;
 }
 
+/*
+ * put MIDI sysex bytes; the event buffer may be chained, thus it has
+ * to be expanded via snd_seq_dump_var_event().
+ */
+struct readq_sysex_ctx {
+	struct seq_oss_readq *readq;
+	int dev;
+};
+
+static int readq_dump_sysex(void *ptr, void *buf, int count)
+{
+	struct readq_sysex_ctx *ctx = ptr;
+
+	return snd_seq_oss_readq_puts(ctx->readq, ctx->dev, buf, count);
+}
+
+int snd_seq_oss_readq_sysex(struct seq_oss_readq *q, int dev,
+			    struct snd_seq_event *ev)
+{
+	struct readq_sysex_ctx ctx = {
+		.readq = q,
+		.dev = dev
+	};
+
+	if ((ev->flags & SNDRV_SEQ_EVENT_LENGTH_MASK) != SNDRV_SEQ_EVENT_LENGTH_VARIABLE)
+		return 0;
+	return snd_seq_dump_var_event(ev, readq_dump_sysex, &ctx);
+}
+
 /*
  * copy an event to input queue:
  * return zero if enqueued
diff --git a/sound/core/seq/oss/seq_oss_readq.h b/sound/core/seq/oss/seq_oss_readq.h
index f1463f1f449e..8d033ca2d23f 100644
--- a/sound/core/seq/oss/seq_oss_readq.h
+++ b/sound/core/seq/oss/seq_oss_readq.h
@@ -44,6 +44,8 @@ void snd_seq_oss_readq_delete(struct seq_oss_readq *q);
 void snd_seq_oss_readq_clear(struct seq_oss_readq *readq);
 unsigned int snd_seq_oss_readq_poll(struct seq_oss_readq *readq, struct file *file, poll_table *wait);
 int snd_seq_oss_readq_puts(struct seq_oss_readq *readq, int dev, unsigned char *data, int len);
+int snd_seq_oss_readq_sysex(struct seq_oss_readq *q, int dev,
+			    struct snd_seq_event *ev);
 int snd_seq_oss_readq_put_event(struct seq_oss_readq *readq, union evrec *ev);
 int snd_seq_oss_readq_put_timestamp(struct seq_oss_readq *readq, unsigned long curt, int seq_mode);
 int snd_seq_oss_readq_pick(struct seq_oss_readq *q, union evrec *rec);
-- 
2.14.3