Commit 1052d988 authored by Takashi Iwai's avatar Takashi Iwai
Browse files

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: default avatarTakashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20240222111509.28390-3-tiwai@suse.de
parent ae921398

sound/core/control.c

+8 −15
Original line number Diff line number Diff line
@@ -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;

}

sound/core/control_compat.c

+26 −43
Original line number Diff line number Diff line
@@ -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 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 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 {