ALSA: control: Use automatic cleanup of kfree()
authorTakashi Iwai <tiwai@suse.de>
Thu, 22 Feb 2024 11:15:02 +0000 (12:15 +0100)
committerTakashi Iwai <tiwai@suse.de>
Fri, 23 Feb 2024 09:57:30 +0000 (10:57 +0100)
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
sound/core/control.c
sound/core/control_compat.c

index 59c8658966d4cb37cc571c4d21da8c2dc57a4ee7..c8cd70aed6af4b55698a25d3ae07bed19588d9dd 100644 (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;
 }
 
index 63d787501066c907dd5efcdccea52960e8a7c44b..8392183c77ed3f40efc0c7490973000a5c8d9680 100644 (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 {