lkml.org 
[lkml]   [2018]   [Dec]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.9 30/50] ALSA: control: Fix race between adding and removing a user element
    Date
    4.9-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Takashi Iwai <tiwai@suse.de>

    commit e1a7bfe3807974e66f971f2589d4e0197ec0fced upstream.

    The procedure for adding a user control element has some window opened
    for race against the concurrent removal of a user element. This was
    caught by syzkaller, hitting a KASAN use-after-free error.

    This patch addresses the bug by wrapping the whole procedure to add a
    user control element with the card->controls_rwsem, instead of only
    around the increment of card->user_ctl_count.

    This required a slight code refactoring, too. The function
    snd_ctl_add() is split to two parts: a core function to add the
    control element and a part calling it. The former is called from the
    function for adding a user control element inside the controls_rwsem.

    One change to be noted is that snd_ctl_notify() for adding a control
    element gets called inside the controls_rwsem as well while it was
    called outside the rwsem. But this should be OK, as snd_ctl_notify()
    takes another (finer) rwlock instead of rwsem, and the call of
    snd_ctl_notify() inside rwsem is already done in another code path.

    Reported-by: syzbot+dc09047bce3820621ba2@syzkaller.appspotmail.com
    Cc: <stable@vger.kernel.org>
    Signed-off-by: Takashi Iwai <tiwai@suse.de>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    sound/core/control.c | 80 ++++++++++++++++++++++++++++-----------------------
    1 file changed, 45 insertions(+), 35 deletions(-)

    --- a/sound/core/control.c
    +++ b/sound/core/control.c
    @@ -346,6 +346,40 @@ static int snd_ctl_find_hole(struct snd_
    return 0;
    }

    +/* add a new kcontrol object; call with card->controls_rwsem locked */
    +static int __snd_ctl_add(struct snd_card *card, struct snd_kcontrol *kcontrol)
    +{
    + struct snd_ctl_elem_id id;
    + unsigned int idx;
    + unsigned int count;
    +
    + id = kcontrol->id;
    + if (id.index > UINT_MAX - kcontrol->count)
    + return -EINVAL;
    +
    + if (snd_ctl_find_id(card, &id)) {
    + dev_err(card->dev,
    + "control %i:%i:%i:%s:%i is already present\n",
    + id.iface, id.device, id.subdevice, id.name, id.index);
    + return -EBUSY;
    + }
    +
    + if (snd_ctl_find_hole(card, kcontrol->count) < 0)
    + return -ENOMEM;
    +
    + list_add_tail(&kcontrol->list, &card->controls);
    + card->controls_count += kcontrol->count;
    + kcontrol->id.numid = card->last_numid + 1;
    + card->last_numid += kcontrol->count;
    +
    + id = kcontrol->id;
    + count = kcontrol->count;
    + for (idx = 0; idx < count; idx++, id.index++, id.numid++)
    + snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_ADD, &id);
    +
    + return 0;
    +}
    +
    /**
    * snd_ctl_add - add the control instance to the card
    * @card: the card instance
    @@ -362,45 +396,18 @@ static int snd_ctl_find_hole(struct snd_
    */
    int snd_ctl_add(struct snd_card *card, struct snd_kcontrol *kcontrol)
    {
    - struct snd_ctl_elem_id id;
    - unsigned int idx;
    - unsigned int count;
    int err = -EINVAL;

    if (! kcontrol)
    return err;
    if (snd_BUG_ON(!card || !kcontrol->info))
    goto error;
    - id = kcontrol->id;
    - if (id.index > UINT_MAX - kcontrol->count)
    - goto error;

    down_write(&card->controls_rwsem);
    - if (snd_ctl_find_id(card, &id)) {
    - up_write(&card->controls_rwsem);
    - dev_err(card->dev, "control %i:%i:%i:%s:%i is already present\n",
    - id.iface,
    - id.device,
    - id.subdevice,
    - id.name,
    - id.index);
    - err = -EBUSY;
    - goto error;
    - }
    - if (snd_ctl_find_hole(card, kcontrol->count) < 0) {
    - up_write(&card->controls_rwsem);
    - err = -ENOMEM;
    - goto error;
    - }
    - list_add_tail(&kcontrol->list, &card->controls);
    - card->controls_count += kcontrol->count;
    - kcontrol->id.numid = card->last_numid + 1;
    - card->last_numid += kcontrol->count;
    - id = kcontrol->id;
    - count = kcontrol->count;
    + err = __snd_ctl_add(card, kcontrol);
    up_write(&card->controls_rwsem);
    - for (idx = 0; idx < count; idx++, id.index++, id.numid++)
    - snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_ADD, &id);
    + if (err < 0)
    + goto error;
    return 0;

    error:
    @@ -1354,9 +1361,12 @@ static int snd_ctl_elem_add(struct snd_c
    kctl->tlv.c = snd_ctl_elem_user_tlv;

    /* This function manage to free the instance on failure. */
    - err = snd_ctl_add(card, kctl);
    - if (err < 0)
    - return err;
    + down_write(&card->controls_rwsem);
    + err = __snd_ctl_add(card, kctl);
    + if (err < 0) {
    + snd_ctl_free_one(kctl);
    + goto unlock;
    + }
    offset = snd_ctl_get_ioff(kctl, &info->id);
    snd_ctl_build_ioff(&info->id, kctl, offset);
    /*
    @@ -1367,10 +1377,10 @@ static int snd_ctl_elem_add(struct snd_c
    * which locks the element.
    */

    - down_write(&card->controls_rwsem);
    card->user_ctl_count++;
    - up_write(&card->controls_rwsem);

    + unlock:
    + up_write(&card->controls_rwsem);
    return 0;
    }


    \
     
     \ /
      Last update: 2018-12-04 12:09    [W:6.266 / U:0.280 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site