mirror of
https://github.com/edk2-porting/linux-next.git
synced 2024-12-21 03:33:59 +08:00
ALSA: control: Fix race between adding and removing a user element
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>
This commit is contained in:
parent
9a20332ab3
commit
e1a7bfe380
@ -348,6 +348,40 @@ static int snd_ctl_find_hole(struct snd_card *card, unsigned int count)
|
||||
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
|
||||
@ -364,45 +398,18 @@ static int snd_ctl_find_hole(struct snd_card *card, unsigned int count)
|
||||
*/
|
||||
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:
|
||||
@ -1361,9 +1368,12 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file,
|
||||
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);
|
||||
/*
|
||||
@ -1374,10 +1384,10 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file,
|
||||
* 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;
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user