Skip to content

Commit

Permalink
ALSA: compat_ioctl: avoid compat_alloc_user_space
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Iwai <[email protected]>
  • Loading branch information
@arndb @tiwai
arndb authored and tiwai committed Sep 21, 2020
1 parent 2b98751 commit 18d122c
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 47 deletions.
38 changes: 25 additions & 13 deletions sound/core/control.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}
Expand All @@ -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)
Expand Down Expand Up @@ -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:
Expand Down
14 changes: 6 additions & 8 deletions sound/core/control_compat.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
27 changes: 17 additions & 10 deletions sound/core/hwdep.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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);
Expand Down
23 changes: 7 additions & 16 deletions sound/core/hwdep_compat.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 18d122c

Please sign in to comment.