ALSA: compat_ioctl: avoid compat_alloc_user_space

Using compat_alloc_user_space() tends to add complexity
to the ioctl handling, so I am trying to remove it everywhere.

The two callers in sound/core can rewritten to just call
the same code that operates on a kernel pointer as the
native handler.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Link: https://lore.kernel.org/r/20200918095642.1446243-1-arnd@arndb.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/core/control.c b/sound/core/control.c
index aa0c0cf..e014598 100644 (file)
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -717,22 +717,19 @@ static int snd_ctl_card_info(struct snd_card *card, struct snd_ctl_file * ctl,
 }
 
 static int snd_ctl_elem_list(struct snd_card *card,
-                            struct snd_ctl_elem_list __user *_list)
+                            struct snd_ctl_elem_list *list)
 {
-       struct snd_ctl_elem_list list;
        struct snd_kcontrol *kctl;
        struct snd_ctl_elem_id id;
        unsigned int offset, space, jidx;
        int err = 0;
 
-       if (copy_from_user(&list, _list, sizeof(list)))
-               return -EFAULT;
-       offset = list.offset;
-       space = list.space;
+       offset = list->offset;
+       space = list->space;
 
        down_read(&card->controls_rwsem);
-       list.count = card->controls_count;
-       list.used = 0;
+       list->count = card->controls_count;
+       list->used = 0;
        if (space > 0) {
                list_for_each_entry(kctl, &card->controls, list) {
                        if (offset >= kctl->count) {
@@ -741,12 +738,12 @@ static int snd_ctl_elem_list(struct snd_card *card,
                        }
                        for (jidx = offset; jidx < kctl->count; jidx++) {
                                snd_ctl_build_ioff(&id, kctl, jidx);
-                               if (copy_to_user(list.pids + list.used, &id,
+                               if (copy_to_user(list->pids + list->used, &id,
                                                 sizeof(id))) {
                                        err = -EFAULT;
                                        goto out;
                                }
-                               list.used++;
+                               list->used++;
                                if (!--space)
                                        goto out;
                        }
@@ -755,11 +752,26 @@ static int snd_ctl_elem_list(struct snd_card *card,
        }
  out:
        up_read(&card->controls_rwsem);
-       if (!err && copy_to_user(_list, &list, sizeof(list)))
-               err = -EFAULT;
        return err;
 }
 
+static int snd_ctl_elem_list_user(struct snd_card *card,
+                                 struct snd_ctl_elem_list __user *_list)
+{
+       struct snd_ctl_elem_list list;
+       int err;
+
+       if (copy_from_user(&list, _list, sizeof(list)))
+               return -EFAULT;
+       err = snd_ctl_elem_list(card, &list);
+       if (err)
+               return err;
+       if (copy_to_user(_list, &list, sizeof(list)))
+               return -EFAULT;
+
+       return 0;
+}
+
 /* Check whether the given kctl info is valid */
 static int snd_ctl_check_elem_info(struct snd_card *card,
                                   const struct snd_ctl_elem_info *info)
@@ -1703,7 +1715,7 @@ static long snd_ctl_ioctl(struct file *file, unsigned int cmd, unsigned long arg
        case SNDRV_CTL_IOCTL_CARD_INFO:
                return snd_ctl_card_info(card, ctl, cmd, argp);
        case SNDRV_CTL_IOCTL_ELEM_LIST:
-               return snd_ctl_elem_list(card, argp);
+               return snd_ctl_elem_list_user(card, argp);
        case SNDRV_CTL_IOCTL_ELEM_INFO:
                return snd_ctl_elem_info_user(ctl, argp);
        case SNDRV_CTL_IOCTL_ELEM_READ:

diff --git a/sound/core/control_compat.c b/sound/core/control_compat.c
index 02df1d7..1d708aa 100644 (file)
--- a/sound/core/control_compat.c
+++ b/sound/core/control_compat.c
@@ -22,24 +22,22 @@ struct snd_ctl_elem_list32 {
 static int snd_ctl_elem_list_compat(struct snd_card *card,
                                    struct snd_ctl_elem_list32 __user *data32)
 {
-       struct snd_ctl_elem_list __user *data;
+       struct snd_ctl_elem_list data = {};
        compat_caddr_t ptr;
        int err;
 
-       data = compat_alloc_user_space(sizeof(*data));
-
        /* offset, space, used, count */
-       if (copy_in_user(data, data32, 4 * sizeof(u32)))
+       if (copy_from_user(&data, data32, 4 * sizeof(u32)))
                return -EFAULT;
        /* pids */
-       if (get_user(ptr, &data32->pids) ||
-           put_user(compat_ptr(ptr), &data->pids))
+       if (get_user(ptr, &data32->pids))
                return -EFAULT;
-       err = snd_ctl_elem_list(card, data);
+       data.pids = compat_ptr(ptr);
+       err = snd_ctl_elem_list(card, &data);
        if (err < 0)
                return err;
        /* copy the result */
-       if (copy_in_user(data32, data, 4 * sizeof(u32)))
+       if (copy_to_user(data32, &data, 4 * sizeof(u32)))
                return -EFAULT;
        return 0;
 }

diff --git a/sound/core/hwdep.c b/sound/core/hwdep.c
index 21edb8a..0c02989 100644 (file)
--- a/sound/core/hwdep.c
+++ b/sound/core/hwdep.c
@@ -203,28 +203,35 @@ static int snd_hwdep_dsp_status(struct snd_hwdep *hw,
 }
 
 static int snd_hwdep_dsp_load(struct snd_hwdep *hw,
-                             struct snd_hwdep_dsp_image __user *_info)
+                             struct snd_hwdep_dsp_image *info)
 {
-       struct snd_hwdep_dsp_image info;
        int err;
        
        if (! hw->ops.dsp_load)
                return -ENXIO;
-       memset(&info, 0, sizeof(info));
-       if (copy_from_user(&info, _info, sizeof(info)))
-               return -EFAULT;
-       if (info.index >= 32)
+       if (info->index >= 32)
                return -EINVAL;
        /* check whether the dsp was already loaded */
-       if (hw->dsp_loaded & (1u << info.index))
+       if (hw->dsp_loaded & (1u << info->index))
                return -EBUSY;
-       err = hw->ops.dsp_load(hw, &info);
+       err = hw->ops.dsp_load(hw, info);
        if (err < 0)
                return err;
-       hw->dsp_loaded |= (1u << info.index);
+       hw->dsp_loaded |= (1u << info->index);
        return 0;
 }
 
+static int snd_hwdep_dsp_load_user(struct snd_hwdep *hw,
+                                  struct snd_hwdep_dsp_image __user *_info)
+{
+       struct snd_hwdep_dsp_image info = {};
+
+       if (copy_from_user(&info, _info, sizeof(info)))
+               return -EFAULT;
+       return snd_hwdep_dsp_load(hw, &info);
+}
+
+
 static long snd_hwdep_ioctl(struct file * file, unsigned int cmd,
                            unsigned long arg)
 {
@@ -238,7 +245,7 @@ static long snd_hwdep_ioctl(struct file * file, unsigned int cmd,
        case SNDRV_HWDEP_IOCTL_DSP_STATUS:
                return snd_hwdep_dsp_status(hw, argp);
        case SNDRV_HWDEP_IOCTL_DSP_LOAD:
-               return snd_hwdep_dsp_load(hw, argp);
+               return snd_hwdep_dsp_load_user(hw, argp);
        }
        if (hw->ops.ioctl)
                return hw->ops.ioctl(hw, file, cmd, arg);

diff --git a/sound/core/hwdep_compat.c b/sound/core/hwdep_compat.c
index bc81db9..a0b7670 100644 (file)
--- a/sound/core/hwdep_compat.c
+++ b/sound/core/hwdep_compat.c
@@ -19,26 +19,17 @@ struct snd_hwdep_dsp_image32 {
 static int snd_hwdep_dsp_load_compat(struct snd_hwdep *hw,
                                     struct snd_hwdep_dsp_image32 __user *src)
 {
-       struct snd_hwdep_dsp_image __user *dst;
+       struct snd_hwdep_dsp_image info = {};
        compat_caddr_t ptr;
-       u32 val;
 
-       dst = compat_alloc_user_space(sizeof(*dst));
-
-       /* index and name */
-       if (copy_in_user(dst, src, 4 + 64))
-               return -EFAULT;
-       if (get_user(ptr, &src->image) ||
-           put_user(compat_ptr(ptr), &dst->image))
-               return -EFAULT;
-       if (get_user(val, &src->length) ||
-           put_user(val, &dst->length))
-               return -EFAULT;
-       if (get_user(val, &src->driver_data) ||
-           put_user(val, &dst->driver_data))
+       if (copy_from_user(&info, src, 4 + 64) ||
+           get_user(ptr, &src->image) ||
+           get_user(info.length, &src->length) ||
+           get_user(info.driver_data, &src->driver_data))
                return -EFAULT;
+       info.image = compat_ptr(ptr);
 
-       return snd_hwdep_dsp_load(hw, dst);
+       return snd_hwdep_dsp_load(hw, &info);
 }
 
 enum {