Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1307434 > unrolled thread

Re: sound: use-after-free in snd_timer_stop

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: sound: use-after-free in snd_timer_stop Takashi Iwai <tiwai@suse.de> - 2016-01-12 15:10 +0100
    Re: sound: use-after-free in snd_timer_stop Takashi Iwai <tiwai@suse.de> - 2016-01-12 15:20 +0100
      Re: sound: use-after-free in snd_timer_stop Dmitry Vyukov <dvyukov@google.com> - 2016-01-12 17:50 +0100
        Re: sound: use-after-free in snd_timer_stop Takashi Iwai <tiwai@suse.de> - 2016-01-12 18:00 +0100

#1307434 — Re: sound: use-after-free in snd_timer_stop

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?


thanks,

Takashi

---
diff --git a/sound/core/seq/seq_queue.c b/sound/core/seq/seq_queue.c
index 7dfd0f429410..8ea17d0a4299 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... */
@@ -470,7 +472,9 @@ int snd_seq_queue_timer_close(int queueid)
 	queue = queueptr(queueid);
 	if (queue == NULL)
 		return -EINVAL;
+	mutex_lock(&queue->timer_mutex);
 	snd_seq_timer_close(queue);
+	mutex_unlock(&queue->timer_mutex);
 	queuefree(queue);
 	return result;
 }

[toc] | [next] | [standalone]

#1307461

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).


Takashi

---
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... */

[toc] | [prev] | [next] | [standalone]

#1307617

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>


> ---
> 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... */

[toc] | [prev] | [next] | [standalone]

#1307633

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... */
-- 
2.7.0

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web