lkml.org 
[lkml]   [2016]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.12 048/142] ALSA: dummy: Implement timer backend switching more safely
    Date
    From: Takashi Iwai <tiwai@suse.de>

    3.12-stable review patch. If anyone has any objections, please let me know.

    ===============

    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>
    Signed-off-by: Jiri Slaby <jslaby@suse.cz>
    ---
    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 f4d626c05071..8946cef245fc 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.1
    \
     
     \ /
      Last update: 2016-02-24 12:41    [W:4.192 / U:25.896 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site