ALSA: control: Use automatic cleanup of kfree()

There are common patterns where a temporary buffer is allocated and
freed at the exit, and those can be simplified with the recent cleanup
mechanism via __free(kfree).

A caveat is that some allocations are memdup_user() and they return an
error pointer instead of NULL.  Those need special cares and the value
has to be cleared with no_free_ptr() at the allocation error path.

Other than that, the conversions are straightforward.

No functional changes, only code refactoring.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20240222111509.28390-3-tiwai@suse.de
This commit is contained in:
Takashi Iwai 2024-02-22 12:15:02 +01:00
parent ae92139848
commit 1052d98822
2 changed files with 34 additions and 58 deletions

View File

@ -932,7 +932,7 @@ EXPORT_SYMBOL(snd_ctl_find_id);
static int snd_ctl_card_info(struct snd_card *card, struct snd_ctl_file * ctl,
unsigned int cmd, void __user *arg)
{
struct snd_ctl_card_info *info;
struct snd_ctl_card_info *info __free(kfree) = NULL;
info = kzalloc(sizeof(*info), GFP_KERNEL);
if (! info)
@ -946,11 +946,8 @@ static int snd_ctl_card_info(struct snd_card *card, struct snd_ctl_file * ctl,
strscpy(info->mixername, card->mixername, sizeof(info->mixername));
strscpy(info->components, card->components, sizeof(info->components));
up_read(&snd_ioctl_rwsem);
if (copy_to_user(arg, info, sizeof(struct snd_ctl_card_info))) {
kfree(info);
if (copy_to_user(arg, info, sizeof(struct snd_ctl_card_info)))
return -EFAULT;
}
kfree(info);
return 0;
}
@ -1339,12 +1336,10 @@ static int snd_ctl_elem_read_user(struct snd_card *card,
result = snd_ctl_elem_read(card, control);
if (result < 0)
goto error;
return result;
if (copy_to_user(_control, control, sizeof(*control)))
result = -EFAULT;
error:
kfree(control);
return -EFAULT;
return result;
}
@ -1406,23 +1401,21 @@ static int snd_ctl_elem_write(struct snd_card *card, struct snd_ctl_file *file,
static int snd_ctl_elem_write_user(struct snd_ctl_file *file,
struct snd_ctl_elem_value __user *_control)
{
struct snd_ctl_elem_value *control;
struct snd_ctl_elem_value *control __free(kfree) = NULL;
struct snd_card *card;
int result;
control = memdup_user(_control, sizeof(*control));
if (IS_ERR(control))
return PTR_ERR(control);
return PTR_ERR(no_free_ptr(control));
card = file->card;
result = snd_ctl_elem_write(card, file, control);
if (result < 0)
goto error;
return result;
if (copy_to_user(_control, control, sizeof(*control)))
result = -EFAULT;
error:
kfree(control);
return -EFAULT;
return result;
}

View File

@ -79,61 +79,56 @@ struct snd_ctl_elem_info32 {
static int snd_ctl_elem_info_compat(struct snd_ctl_file *ctl,
struct snd_ctl_elem_info32 __user *data32)
{
struct snd_ctl_elem_info *data;
struct snd_ctl_elem_info *data __free(kfree) = NULL;
int err;
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (! data)
return -ENOMEM;
err = -EFAULT;
/* copy id */
if (copy_from_user(&data->id, &data32->id, sizeof(data->id)))
goto error;
return -EFAULT;
/* we need to copy the item index.
* hope this doesn't break anything..
*/
if (get_user(data->value.enumerated.item, &data32->value.enumerated.item))
goto error;
return -EFAULT;
err = snd_ctl_elem_info(ctl, data);
if (err < 0)
goto error;
return err;
/* restore info to 32bit */
err = -EFAULT;
/* id, type, access, count */
if (copy_to_user(&data32->id, &data->id, sizeof(data->id)) ||
copy_to_user(&data32->type, &data->type, 3 * sizeof(u32)))
goto error;
return -EFAULT;
if (put_user(data->owner, &data32->owner))
goto error;
return -EFAULT;
switch (data->type) {
case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
case SNDRV_CTL_ELEM_TYPE_INTEGER:
if (put_user(data->value.integer.min, &data32->value.integer.min) ||
put_user(data->value.integer.max, &data32->value.integer.max) ||
put_user(data->value.integer.step, &data32->value.integer.step))
goto error;
return -EFAULT;
break;
case SNDRV_CTL_ELEM_TYPE_INTEGER64:
if (copy_to_user(&data32->value.integer64,
&data->value.integer64,
sizeof(data->value.integer64)))
goto error;
return -EFAULT;
break;
case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
if (copy_to_user(&data32->value.enumerated,
&data->value.enumerated,
sizeof(data->value.enumerated)))
goto error;
return -EFAULT;
break;
default:
break;
}
err = 0;
error:
kfree(data);
return err;
return 0;
}
/* read / write */
@ -169,7 +164,7 @@ static int get_ctl_type(struct snd_card *card, struct snd_ctl_elem_id *id,
int *countp)
{
struct snd_kcontrol *kctl;
struct snd_ctl_elem_info *info;
struct snd_ctl_elem_info *info __free(kfree) = NULL;
int err;
down_read(&card->controls_rwsem);
@ -193,7 +188,6 @@ static int get_ctl_type(struct snd_card *card, struct snd_ctl_elem_id *id,
err = info->type;
*countp = info->count;
}
kfree(info);
return err;
}
@ -289,7 +283,7 @@ static int copy_ctl_value_to_user(void __user *userdata,
static int ctl_elem_read_user(struct snd_card *card,
void __user *userdata, void __user *valuep)
{
struct snd_ctl_elem_value *data;
struct snd_ctl_elem_value *data __free(kfree) = NULL;
int err, type, count;
data = kzalloc(sizeof(*data), GFP_KERNEL);
@ -299,21 +293,18 @@ static int ctl_elem_read_user(struct snd_card *card,
err = copy_ctl_value_from_user(card, data, userdata, valuep,
&type, &count);
if (err < 0)
goto error;
return err;
err = snd_ctl_elem_read(card, data);
if (err < 0)
goto error;
err = copy_ctl_value_to_user(userdata, valuep, data, type, count);
error:
kfree(data);
return err;
return err;
return copy_ctl_value_to_user(userdata, valuep, data, type, count);
}
static int ctl_elem_write_user(struct snd_ctl_file *file,
void __user *userdata, void __user *valuep)
{
struct snd_ctl_elem_value *data;
struct snd_ctl_elem_value *data __free(kfree) = NULL;
struct snd_card *card = file->card;
int err, type, count;
@ -324,15 +315,12 @@ static int ctl_elem_write_user(struct snd_ctl_file *file,
err = copy_ctl_value_from_user(card, data, userdata, valuep,
&type, &count);
if (err < 0)
goto error;
return err;
err = snd_ctl_elem_write(card, file, data);
if (err < 0)
goto error;
err = copy_ctl_value_to_user(userdata, valuep, data, type, count);
error:
kfree(data);
return err;
return err;
return copy_ctl_value_to_user(userdata, valuep, data, type, count);
}
static int snd_ctl_elem_read_user_compat(struct snd_card *card,
@ -366,49 +354,44 @@ static int snd_ctl_elem_add_compat(struct snd_ctl_file *file,
struct snd_ctl_elem_info32 __user *data32,
int replace)
{
struct snd_ctl_elem_info *data;
int err;
struct snd_ctl_elem_info *data __free(kfree) = NULL;
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (! data)
return -ENOMEM;
err = -EFAULT;
/* id, type, access, count */ \
if (copy_from_user(&data->id, &data32->id, sizeof(data->id)) ||
copy_from_user(&data->type, &data32->type, 3 * sizeof(u32)))
goto error;
return -EFAULT;
if (get_user(data->owner, &data32->owner))
goto error;
return -EFAULT;
switch (data->type) {
case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
case SNDRV_CTL_ELEM_TYPE_INTEGER:
if (get_user(data->value.integer.min, &data32->value.integer.min) ||
get_user(data->value.integer.max, &data32->value.integer.max) ||
get_user(data->value.integer.step, &data32->value.integer.step))
goto error;
return -EFAULT;
break;
case SNDRV_CTL_ELEM_TYPE_INTEGER64:
if (copy_from_user(&data->value.integer64,
&data32->value.integer64,
sizeof(data->value.integer64)))
goto error;
return -EFAULT;
break;
case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
if (copy_from_user(&data->value.enumerated,
&data32->value.enumerated,
sizeof(data->value.enumerated)))
goto error;
return -EFAULT;
data->value.enumerated.names_ptr =
(uintptr_t)compat_ptr(data->value.enumerated.names_ptr);
break;
default:
break;
}
err = snd_ctl_elem_add(file, data, replace);
error:
kfree(data);
return err;
return snd_ctl_elem_add(file, data, replace);
}
enum {