From fc1c428eb46af8183be771d2c78b3902acbeffe3 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 28 Dec 2017 14:43:06 -0500 Subject: usx2y: don't bother with access_ok() in ->dsp_load() memdup_user() checks it, so the only effect would be failing with -EINVAL instead of -EFAULT in case when access_ok() is false. However, the caller has already checked access_ok() itself (and would have buggered off with -EFAULT), so the check is completely pointless. Removing it both simplifies the only instance of ->dsp_load() and allows to get rid of the check in caller - its sole effect used to be in preventing a bogus error value from access_ok() in the instance. Let memdup_user() do the right thing instead... Signed-off-by: Al Viro --- sound/usb/usx2y/usX2Yhwdep.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) (limited to 'sound/usb/usx2y') diff --git a/sound/usb/usx2y/usX2Yhwdep.c b/sound/usb/usx2y/usX2Yhwdep.c index f4b3cda412fc..2bbcf4af06dd 100644 --- a/sound/usb/usx2y/usX2Yhwdep.c +++ b/sound/usb/usx2y/usX2Yhwdep.c @@ -198,24 +198,22 @@ static int snd_usX2Y_hwdep_dsp_load(struct snd_hwdep *hw, struct snd_hwdep_dsp_image *dsp) { struct usX2Ydev *priv = hw->private_data; - int lret, err = -EINVAL; - snd_printdd( "dsp_load %s\n", dsp->name); + struct usb_device* dev = priv->dev; + int lret, err; + char *buf; - if (access_ok(VERIFY_READ, dsp->image, dsp->length)) { - struct usb_device* dev = priv->dev; - char *buf; + snd_printdd( "dsp_load %s\n", dsp->name); - buf = memdup_user(dsp->image, dsp->length); - if (IS_ERR(buf)) - return PTR_ERR(buf); + buf = memdup_user(dsp->image, dsp->length); + if (IS_ERR(buf)) + return PTR_ERR(buf); - err = usb_set_interface(dev, 0, 1); - if (err) - snd_printk(KERN_ERR "usb_set_interface error \n"); - else - err = usb_bulk_msg(dev, usb_sndbulkpipe(dev, 2), buf, dsp->length, &lret, 6000); - kfree(buf); - } + err = usb_set_interface(dev, 0, 1); + if (err) + snd_printk(KERN_ERR "usb_set_interface error \n"); + else + err = usb_bulk_msg(dev, usb_sndbulkpipe(dev, 2), buf, dsp->length, &lret, 6000); + kfree(buf); if (err) return err; if (dsp->index == 1) { -- cgit v1.2.3 From 3d46d7108dd3ff8b1d477bc2b7b061b12690e83c Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 28 Dec 2017 17:22:51 -0500 Subject: usx2y: don't bother with memdup_user() for 16-byte structure ... when it can bloody well go into a local variable. Signed-off-by: Al Viro --- sound/usb/usx2y/us122l.c | 43 ++++++++++++++++++------------------------- 1 file changed, 18 insertions(+), 25 deletions(-) (limited to 'sound/usb/usx2y') diff --git a/sound/usb/usx2y/us122l.c b/sound/usb/usx2y/us122l.c index 159da1f3924e..8c394178a385 100644 --- a/sound/usb/usx2y/us122l.c +++ b/sound/usb/usx2y/us122l.c @@ -378,7 +378,7 @@ out: static int usb_stream_hwdep_ioctl(struct snd_hwdep *hw, struct file *file, unsigned cmd, unsigned long arg) { - struct usb_stream_config *cfg; + struct usb_stream_config cfg; struct us122l *us122l = hw->private_data; struct usb_stream *s; unsigned min_period_frames; @@ -388,24 +388,21 @@ static int usb_stream_hwdep_ioctl(struct snd_hwdep *hw, struct file *file, if (cmd != SNDRV_USB_STREAM_IOCTL_SET_PARAMS) return -ENOTTY; - cfg = memdup_user((void *)arg, sizeof(*cfg)); - if (IS_ERR(cfg)) - return PTR_ERR(cfg); + if (copy_from_user(&cfg, (void __user *)arg, sizeof(cfg))) + return -EFAULT; + + if (cfg.version != USB_STREAM_INTERFACE_VERSION) + return -ENXIO; - if (cfg->version != USB_STREAM_INTERFACE_VERSION) { - err = -ENXIO; - goto free; - } high_speed = us122l->dev->speed == USB_SPEED_HIGH; - if ((cfg->sample_rate != 44100 && cfg->sample_rate != 48000 && + if ((cfg.sample_rate != 44100 && cfg.sample_rate != 48000 && (!high_speed || - (cfg->sample_rate != 88200 && cfg->sample_rate != 96000))) || - cfg->frame_size != 6 || - cfg->period_frames > 0x3000) { - err = -EINVAL; - goto free; - } - switch (cfg->sample_rate) { + (cfg.sample_rate != 88200 && cfg.sample_rate != 96000))) || + cfg.frame_size != 6 || + cfg.period_frames > 0x3000) + return -EINVAL; + + switch (cfg.sample_rate) { case 44100: min_period_frames = 48; break; @@ -418,10 +415,8 @@ static int usb_stream_hwdep_ioctl(struct snd_hwdep *hw, struct file *file, } if (!high_speed) min_period_frames <<= 1; - if (cfg->period_frames < min_period_frames) { - err = -EINVAL; - goto free; - } + if (cfg.period_frames < min_period_frames) + return -EINVAL; snd_power_wait(hw->card, SNDRV_CTL_POWER_D0); @@ -430,24 +425,22 @@ static int usb_stream_hwdep_ioctl(struct snd_hwdep *hw, struct file *file, if (!us122l->master) us122l->master = file; else if (us122l->master != file) { - if (!s || memcmp(cfg, &s->cfg, sizeof(*cfg))) { + if (!s || memcmp(&cfg, &s->cfg, sizeof(cfg))) { err = -EIO; goto unlock; } us122l->slave = file; } - if (!s || memcmp(cfg, &s->cfg, sizeof(*cfg)) || + if (!s || memcmp(&cfg, &s->cfg, sizeof(cfg)) || s->state == usb_stream_xrun) { us122l_stop(us122l); - if (!us122l_start(us122l, cfg->sample_rate, cfg->period_frames)) + if (!us122l_start(us122l, cfg.sample_rate, cfg.period_frames)) err = -EIO; else err = 1; } unlock: mutex_unlock(&us122l->mutex); -free: - kfree(cfg); wake_up_all(&us122l->sk.sleep); return err; } -- cgit v1.2.3