Commit 57db0e40 authored by Takashi Iwai's avatar Takashi Iwai Committed by Luis Henriques

ALSA: dummy: Implement timer backend switching more safely

commit ddce57a6 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 [7ee96216: 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.comReported-by: default avatarDmitry Vyukov <dvyukov@google.com>
Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
Signed-off-by: default avatarLuis Henriques <luis.henriques@canonical.com>
parent d5dc8862
...@@ -87,7 +87,7 @@ MODULE_PARM_DESC(pcm_substreams, "PCM substreams # (1-128) for dummy driver."); ...@@ -87,7 +87,7 @@ MODULE_PARM_DESC(pcm_substreams, "PCM substreams # (1-128) for dummy driver.");
module_param(fake_buffer, bool, 0444); module_param(fake_buffer, bool, 0444);
MODULE_PARM_DESC(fake_buffer, "Fake buffer allocations."); MODULE_PARM_DESC(fake_buffer, "Fake buffer allocations.");
#ifdef CONFIG_HIGH_RES_TIMERS #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."); MODULE_PARM_DESC(hrtimer, "Use hrtimer as the timer source.");
#endif #endif
...@@ -109,6 +109,9 @@ struct dummy_timer_ops { ...@@ -109,6 +109,9 @@ struct dummy_timer_ops {
snd_pcm_uframes_t (*pointer)(struct snd_pcm_substream *); 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 { struct dummy_model {
const char *name; const char *name;
int (*playback_constraints)(struct snd_pcm_runtime *runtime); int (*playback_constraints)(struct snd_pcm_runtime *runtime);
...@@ -137,7 +140,6 @@ struct snd_dummy { ...@@ -137,7 +140,6 @@ struct snd_dummy {
int iobox; int iobox;
struct snd_kcontrol *cd_volume_ctl; struct snd_kcontrol *cd_volume_ctl;
struct snd_kcontrol *cd_switch_ctl; struct snd_kcontrol *cd_switch_ctl;
const struct dummy_timer_ops *timer_ops;
}; };
/* /*
...@@ -231,6 +233,8 @@ struct dummy_model *dummy_models[] = { ...@@ -231,6 +233,8 @@ struct dummy_model *dummy_models[] = {
*/ */
struct dummy_systimer_pcm { struct dummy_systimer_pcm {
/* ops must be the first item */
const struct dummy_timer_ops *timer_ops;
spinlock_t lock; spinlock_t lock;
struct timer_list timer; struct timer_list timer;
unsigned long base_time; unsigned long base_time;
...@@ -368,6 +372,8 @@ static struct dummy_timer_ops dummy_systimer_ops = { ...@@ -368,6 +372,8 @@ static struct dummy_timer_ops dummy_systimer_ops = {
*/ */
struct dummy_hrtimer_pcm { struct dummy_hrtimer_pcm {
/* ops must be the first item */
const struct dummy_timer_ops *timer_ops;
ktime_t base_time; ktime_t base_time;
ktime_t period_time; ktime_t period_time;
atomic_t running; atomic_t running;
...@@ -494,31 +500,25 @@ static struct dummy_timer_ops dummy_hrtimer_ops = { ...@@ -494,31 +500,25 @@ static struct dummy_timer_ops dummy_hrtimer_ops = {
static int dummy_pcm_trigger(struct snd_pcm_substream *substream, int cmd) static int dummy_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
{ {
struct snd_dummy *dummy = snd_pcm_substream_chip(substream);
switch (cmd) { switch (cmd) {
case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_RESUME: 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_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_SUSPEND:
return dummy->timer_ops->stop(substream); return get_dummy_ops(substream)->stop(substream);
} }
return -EINVAL; return -EINVAL;
} }
static int dummy_pcm_prepare(struct snd_pcm_substream *substream) static int dummy_pcm_prepare(struct snd_pcm_substream *substream)
{ {
struct snd_dummy *dummy = snd_pcm_substream_chip(substream); return get_dummy_ops(substream)->prepare(substream);
return dummy->timer_ops->prepare(substream);
} }
static snd_pcm_uframes_t dummy_pcm_pointer(struct snd_pcm_substream *substream) static snd_pcm_uframes_t dummy_pcm_pointer(struct snd_pcm_substream *substream)
{ {
struct snd_dummy *dummy = snd_pcm_substream_chip(substream); return get_dummy_ops(substream)->pointer(substream);
return dummy->timer_ops->pointer(substream);
} }
static struct snd_pcm_hardware dummy_pcm_hardware = { static struct snd_pcm_hardware dummy_pcm_hardware = {
...@@ -564,17 +564,19 @@ static int dummy_pcm_open(struct snd_pcm_substream *substream) ...@@ -564,17 +564,19 @@ static int dummy_pcm_open(struct snd_pcm_substream *substream)
struct snd_dummy *dummy = snd_pcm_substream_chip(substream); struct snd_dummy *dummy = snd_pcm_substream_chip(substream);
struct dummy_model *model = dummy->model; struct dummy_model *model = dummy->model;
struct snd_pcm_runtime *runtime = substream->runtime; struct snd_pcm_runtime *runtime = substream->runtime;
const struct dummy_timer_ops *ops;
int err; int err;
dummy->timer_ops = &dummy_systimer_ops; ops = &dummy_systimer_ops;
#ifdef CONFIG_HIGH_RES_TIMERS #ifdef CONFIG_HIGH_RES_TIMERS
if (hrtimer) if (hrtimer)
dummy->timer_ops = &dummy_hrtimer_ops; ops = &dummy_hrtimer_ops;
#endif #endif
err = dummy->timer_ops->create(substream); err = ops->create(substream);
if (err < 0) if (err < 0)
return err; return err;
get_dummy_ops(substream) = ops;
runtime->hw = dummy->pcm_hw; runtime->hw = dummy->pcm_hw;
if (substream->pcm->device & 1) { if (substream->pcm->device & 1) {
...@@ -596,7 +598,7 @@ static int dummy_pcm_open(struct snd_pcm_substream *substream) ...@@ -596,7 +598,7 @@ static int dummy_pcm_open(struct snd_pcm_substream *substream)
err = model->capture_constraints(substream->runtime); err = model->capture_constraints(substream->runtime);
} }
if (err < 0) { if (err < 0) {
dummy->timer_ops->free(substream); get_dummy_ops(substream)->free(substream);
return err; return err;
} }
return 0; return 0;
...@@ -604,8 +606,7 @@ static int dummy_pcm_open(struct snd_pcm_substream *substream) ...@@ -604,8 +606,7 @@ static int dummy_pcm_open(struct snd_pcm_substream *substream)
static int dummy_pcm_close(struct snd_pcm_substream *substream) static int dummy_pcm_close(struct snd_pcm_substream *substream)
{ {
struct snd_dummy *dummy = snd_pcm_substream_chip(substream); get_dummy_ops(substream)->free(substream);
dummy->timer_ops->free(substream);
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment