| From: Takashi Iwai <tiwai@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@google.com> |
| Signed-off-by: Takashi Iwai <tiwai@suse.de> |
| [bwh: Backported to 3.2: adjust context] |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| sound/drivers/dummy.c | 37 +++++++++++++++++++------------------ |
| 1 file changed, 19 insertions(+), 18 deletions(-) |
| |
| --- a/sound/drivers/dummy.c |
| +++ b/sound/drivers/dummy.c |
| @@ -87,7 +87,7 @@ MODULE_PARM_DESC(pcm_substreams, "PCM su |
| 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); |
| @@ -134,7 +137,6 @@ struct snd_dummy { |
| spinlock_t mixer_lock; |
| int mixer_volume[MIXER_ADDR_LAST+1][2]; |
| int capture_source[MIXER_ADDR_LAST+1][2]; |
| - const struct dummy_timer_ops *timer_ops; |
| }; |
| |
| /* |
| @@ -228,6 +230,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; |
| @@ -365,6 +369,8 @@ static struct dummy_timer_ops dummy_syst |
| */ |
| |
| 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; |
| @@ -491,31 +497,25 @@ static struct dummy_timer_ops dummy_hrti |
| |
| 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 = { |
| @@ -561,17 +561,19 @@ static int dummy_pcm_open(struct snd_pcm |
| 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) { |
| @@ -593,7 +595,7 @@ static int dummy_pcm_open(struct snd_pcm |
| err = model->capture_constraints(substream->runtime); |
| } |
| if (err < 0) { |
| - dummy->timer_ops->free(substream); |
| + get_dummy_ops(substream)->free(substream); |
| return err; |
| } |
| return 0; |
| @@ -601,8 +603,7 @@ static int dummy_pcm_open(struct snd_pcm |
| |
| 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; |
| } |
| |