On Tue, 12 Jan 2016 17:46:15 +0100, Dmitry Vyukov wrote:
On Tue, Jan 12, 2016 at 3:16 PM, Takashi Iwai tiwai@suse.de wrote:
On Tue, 12 Jan 2016 15:05:24 +0100, Takashi Iwai wrote:
On Tue, 12 Jan 2016 11:22:09 +0100, Dmitry Vyukov wrote:
Hello,
I've hit the following use-after-free while running syzkaller fuzzer. It is followed by a splat of other reports and finally kernel death. I wasn't able to reproduce it with a standalone C program (there is probably some global state involved). But it reproduces by replaying fuzzer logs in a loop (you will need Go toolchain):
$ go get github.com/google/syzkaller $ cd $GOPATH/src/github.com/google/syzkaller $ make executor execprog
Just a note: I had to run "mkdir bin" beforehand. Also for my distro, I had to remove -static.
$ scp bin/syz-executor bin/syz-execprog your@machine $ scp snd_timer_stop your@machine # the attached file on test machine: $ ./syz-execprog -executor ./syz-executor -cover=0 -repeat=0 -procs=16 snd_timer_stop
I tried this but couldn't reproduce the issue on my machine, unfortunately. (Instead I hit another kernel panic regarding apparmor :)
But looking through your log, it seems like a missing race protection. Does the patch below work for you?
Gah, scratch this. The second hunk causes a deadlock. The revised patch is below (just containing the first hunk).
Yes, this patch fixes the crashes for me. Reproduction may require running syz-execprog for several minutes.
Tested-by: Dmitry Vyukov dvyukov@google.com
Good to hear. FWIW, below is the final patch I'm going to queue. Thanks!
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: seq: Fix race at timer setup and close
ALSA sequencer code has an open race between the timer setup ioctl and the close of the client. This was triggered by syzkaller fuzzer, and a use-after-free was caught there as a result.
This patch papers over it by adding a proper queue->timer_mutex lock around the timer-related calls in the relevant code path.
Reported-by: Dmitry Vyukov dvyukov@google.com Tested-by: Dmitry Vyukov dvyukov@google.com Cc: stable@vger.kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/seq/seq_queue.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/sound/core/seq/seq_queue.c b/sound/core/seq/seq_queue.c index 7dfd0f429410..0bec02e89d51 100644 --- a/sound/core/seq/seq_queue.c +++ b/sound/core/seq/seq_queue.c @@ -142,8 +142,10 @@ static struct snd_seq_queue *queue_new(int owner, int locked) static void queue_delete(struct snd_seq_queue *q) { /* stop and release the timer */ + mutex_lock(&q->timer_mutex); snd_seq_timer_stop(q->timer); snd_seq_timer_close(q); + mutex_unlock(&q->timer_mutex); /* wait until access free */ snd_use_lock_sync(&q->use_lock); /* release resources... */