[3.13.y-ckt stable] Patch "ALSA: dummy: Implement timer backend switching more safely" has been added to the 3.13.y-ckt tree

Kamal Mostafa kamal at canonical.com
Wed Mar 9 23:10:33 UTC 2016


This is a note to let you know that I have just added a patch titled

    ALSA: dummy: Implement timer backend switching more safely

to the linux-3.13.y-queue branch of the 3.13.y-ckt extended stable tree 
which can be found at:

    http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-3.13.y-queue

This patch is scheduled to be released in version 3.13.11-ckt36.

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.13.y-ckt tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Kamal

---8<------------------------------------------------------------

>From d9b501469c94199530d6811cb356f9a7c51320bb Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai at suse.de>
Date: Tue, 2 Feb 2016 15:27:36 +0100
Subject: ALSA: dummy: Implement timer backend switching more safely

commit ddce57a6f0a2d8d1bfacfa77f06043bc760403c2 upstream.

Currently the selected timer backend is referred at any moment from
the running PCM callbacks.  When the backend is switched, it's
possible to lead to inconsistency from the running backend.  This was
pointed by syzkaller fuzzer, and the commit [7ee96216c31a: ALSA:
dummy: Disable switching timer backend via sysfs] disabled the dynamic
switching for avoiding the crash.

This patch improves the handling of timer backend switching.  It keeps
the reference to the selected backend during the whole operation of an
opened stream so that it won't be changed by other streams.

Together with this change, the hrtimer parameter is reenabled as
writable now.

NOTE: this patch also turned out to fix the still remaining race.
Namely, ops was still replaced dynamically at dummy_pcm_open:

  static int dummy_pcm_open(struct snd_pcm_substream *substream)
  {
  ....
          dummy->timer_ops = &dummy_systimer_ops;
          if (hrtimer)
                  dummy->timer_ops = &dummy_hrtimer_ops;

Since dummy->timer_ops is common among all streams, and when the
replacement happens during accesses of other streams, it may lead to a
crash.  This was actually triggered by syzkaller fuzzer and KASAN.

This patch rewrites the code not to use the ops shared by all streams
any longer, too.

BugLink: http://lkml.kernel.org/r/CACT4Y+aZ+xisrpuM6cOXbL21DuM0yVxPYXf4cD4Md9uw0C3dBQ@mail.gmail.com
Reported-by: Dmitry Vyukov <dvyukov at google.com>
Signed-off-by: Takashi Iwai <tiwai at suse.de>
Signed-off-by: Kamal Mostafa <kamal at canonical.com>
---
 sound/drivers/dummy.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
index f4d626c..8946cef 100644
--- a/sound/drivers/dummy.c
+++ b/sound/drivers/dummy.c
@@ -87,7 +87,7 @@ MODULE_PARM_DESC(pcm_substreams, "PCM substreams # (1-128) for dummy driver.");
 module_param(fake_buffer, bool, 0444);
 MODULE_PARM_DESC(fake_buffer, "Fake buffer allocations.");
 #ifdef CONFIG_HIGH_RES_TIMERS
-module_param(hrtimer, bool, 0444);
+module_param(hrtimer, bool, 0644);
 MODULE_PARM_DESC(hrtimer, "Use hrtimer as the timer source.");
 #endif

@@ -109,6 +109,9 @@ struct dummy_timer_ops {
 	snd_pcm_uframes_t (*pointer)(struct snd_pcm_substream *);
 };

+#define get_dummy_ops(substream) \
+	(*(const struct dummy_timer_ops **)(substream)->runtime->private_data)
+
 struct dummy_model {
 	const char *name;
 	int (*playback_constraints)(struct snd_pcm_runtime *runtime);
@@ -137,7 +140,6 @@ struct snd_dummy {
 	int iobox;
 	struct snd_kcontrol *cd_volume_ctl;
 	struct snd_kcontrol *cd_switch_ctl;
-	const struct dummy_timer_ops *timer_ops;
 };

 /*
@@ -231,6 +233,8 @@ struct dummy_model *dummy_models[] = {
  */

 struct dummy_systimer_pcm {
+	/* ops must be the first item */
+	const struct dummy_timer_ops *timer_ops;
 	spinlock_t lock;
 	struct timer_list timer;
 	unsigned long base_time;
@@ -368,6 +372,8 @@ static struct dummy_timer_ops dummy_systimer_ops = {
  */

 struct dummy_hrtimer_pcm {
+	/* ops must be the first item */
+	const struct dummy_timer_ops *timer_ops;
 	ktime_t base_time;
 	ktime_t period_time;
 	atomic_t running;
@@ -494,31 +500,25 @@ static struct dummy_timer_ops dummy_hrtimer_ops = {

 static int dummy_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
 {
-	struct snd_dummy *dummy = snd_pcm_substream_chip(substream);
-
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
-		return dummy->timer_ops->start(substream);
+		return get_dummy_ops(substream)->start(substream);
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
-		return dummy->timer_ops->stop(substream);
+		return get_dummy_ops(substream)->stop(substream);
 	}
 	return -EINVAL;
 }

 static int dummy_pcm_prepare(struct snd_pcm_substream *substream)
 {
-	struct snd_dummy *dummy = snd_pcm_substream_chip(substream);
-
-	return dummy->timer_ops->prepare(substream);
+	return get_dummy_ops(substream)->prepare(substream);
 }

 static snd_pcm_uframes_t dummy_pcm_pointer(struct snd_pcm_substream *substream)
 {
-	struct snd_dummy *dummy = snd_pcm_substream_chip(substream);
-
-	return dummy->timer_ops->pointer(substream);
+	return get_dummy_ops(substream)->pointer(substream);
 }

 static struct snd_pcm_hardware dummy_pcm_hardware = {
@@ -564,17 +564,19 @@ static int dummy_pcm_open(struct snd_pcm_substream *substream)
 	struct snd_dummy *dummy = snd_pcm_substream_chip(substream);
 	struct dummy_model *model = dummy->model;
 	struct snd_pcm_runtime *runtime = substream->runtime;
+	const struct dummy_timer_ops *ops;
 	int err;

-	dummy->timer_ops = &dummy_systimer_ops;
+	ops = &dummy_systimer_ops;
 #ifdef CONFIG_HIGH_RES_TIMERS
 	if (hrtimer)
-		dummy->timer_ops = &dummy_hrtimer_ops;
+		ops = &dummy_hrtimer_ops;
 #endif

-	err = dummy->timer_ops->create(substream);
+	err = ops->create(substream);
 	if (err < 0)
 		return err;
+	get_dummy_ops(substream) = ops;

 	runtime->hw = dummy->pcm_hw;
 	if (substream->pcm->device & 1) {
@@ -596,7 +598,7 @@ static int dummy_pcm_open(struct snd_pcm_substream *substream)
 			err = model->capture_constraints(substream->runtime);
 	}
 	if (err < 0) {
-		dummy->timer_ops->free(substream);
+		get_dummy_ops(substream)->free(substream);
 		return err;
 	}
 	return 0;
@@ -604,8 +606,7 @@ static int dummy_pcm_open(struct snd_pcm_substream *substream)

 static int dummy_pcm_close(struct snd_pcm_substream *substream)
 {
-	struct snd_dummy *dummy = snd_pcm_substream_chip(substream);
-	dummy->timer_ops->free(substream);
+	get_dummy_ops(substream)->free(substream);
 	return 0;
 }

--
2.7.0





More information about the kernel-team mailing list