On Wed, 02 Sep 2020 13:01:12 +0200,
Greg Kroah-Hartman wrote:
> 
> The usb_control_msg_send() and usb_control_msg_recv() calls can return
> an error if a "short" write/read happens, and they can handle data off
> of the stack, so move the driver over to using those calls instead,
> saving some logic when dynamically allocating memory.
> 
> Cc: Jaroslav Kysela <pe...@perex.cz>
> Cc: Takashi Iwai <ti...@suse.com>
> Cc: Vasily Khoruzhick <anars...@gmail.com>
> Cc: alsa-de...@alsa-project.org
> Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>

Reviewed-by: Takashi Iwai <ti...@suse.de>


thanks,

Takashi


> ---
>  sound/usb/line6/driver.c   | 69 +++++++++++++++-----------------------
>  sound/usb/line6/podhd.c    | 17 ++++------
>  sound/usb/line6/toneport.c |  8 ++---
>  3 files changed, 37 insertions(+), 57 deletions(-)
> 
> diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c
> index 60674ce4879b..601292c51491 100644
> --- a/sound/usb/line6/driver.c
> +++ b/sound/usb/line6/driver.c
> @@ -337,23 +337,18 @@ int line6_read_data(struct usb_line6 *line6, unsigned 
> address, void *data,
>  {
>       struct usb_device *usbdev = line6->usbdev;
>       int ret;
> -     unsigned char *len;
> +     u8 len;
>       unsigned count;
>  
>       if (address > 0xffff || datalen > 0xff)
>               return -EINVAL;
>  
> -     len = kmalloc(1, GFP_KERNEL);
> -     if (!len)
> -             return -ENOMEM;
> -
>       /* query the serial number: */
> -     ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
> -                           USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> -                           (datalen << 8) | 0x21, address,
> -                           NULL, 0, LINE6_TIMEOUT * HZ);
> -
> -     if (ret < 0) {
> +     ret = usb_control_msg_send(usbdev, 0, 0x67,
> +                                USB_TYPE_VENDOR | USB_RECIP_DEVICE | 
> USB_DIR_OUT,
> +                                (datalen << 8) | 0x21, address, NULL, 0,
> +                                LINE6_TIMEOUT * HZ);
> +     if (ret) {
>               dev_err(line6->ifcdev, "read request failed (error %d)\n", ret);
>               goto exit;
>       }
> @@ -362,45 +357,41 @@ int line6_read_data(struct usb_line6 *line6, unsigned 
> address, void *data,
>       for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
>               mdelay(LINE6_READ_WRITE_STATUS_DELAY);
>  
> -             ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
> -                                   USB_TYPE_VENDOR | USB_RECIP_DEVICE |
> -                                   USB_DIR_IN,
> -                                   0x0012, 0x0000, len, 1,
> -                                   LINE6_TIMEOUT * HZ);
> -             if (ret < 0) {
> +             ret = usb_control_msg_recv(usbdev, 0, 0x67,
> +                                        USB_TYPE_VENDOR | USB_RECIP_DEVICE | 
> USB_DIR_IN,
> +                                        0x0012, 0x0000, &len, 1,
> +                                        LINE6_TIMEOUT * HZ);
> +             if (ret) {
>                       dev_err(line6->ifcdev,
>                               "receive length failed (error %d)\n", ret);
>                       goto exit;
>               }
>  
> -             if (*len != 0xff)
> +             if (len != 0xff)
>                       break;
>       }
>  
>       ret = -EIO;
> -     if (*len == 0xff) {
> +     if (len == 0xff) {
>               dev_err(line6->ifcdev, "read failed after %d retries\n",
>                       count);
>               goto exit;
> -     } else if (*len != datalen) {
> +     } else if (len != datalen) {
>               /* should be equal or something went wrong */
>               dev_err(line6->ifcdev,
>                       "length mismatch (expected %d, got %d)\n",
> -                     (int)datalen, (int)*len);
> +                     (int)datalen, len);
>               goto exit;
>       }
>  
>       /* receive the result: */
> -     ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
> -                           USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
> -                           0x0013, 0x0000, data, datalen,
> -                           LINE6_TIMEOUT * HZ);
> -
> -     if (ret < 0)
> +     ret = usb_control_msg_recv(usbdev, 0, 0x67,
> +                                USB_TYPE_VENDOR | USB_RECIP_DEVICE | 
> USB_DIR_IN,
> +                                0x0013, 0x0000, data, datalen, LINE6_TIMEOUT 
> * HZ);
> +     if (ret)
>               dev_err(line6->ifcdev, "read failed (error %d)\n", ret);
>  
>  exit:
> -     kfree(len);
>       return ret;
>  }
>  EXPORT_SYMBOL_GPL(line6_read_data);
> @@ -423,12 +414,10 @@ int line6_write_data(struct usb_line6 *line6, unsigned 
> address, void *data,
>       if (!status)
>               return -ENOMEM;
>  
> -     ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
> -                           USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> -                           0x0022, address, data, datalen,
> -                           LINE6_TIMEOUT * HZ);
> -
> -     if (ret < 0) {
> +     ret = usb_control_msg_send(usbdev, 0, 0x67,
> +                                USB_TYPE_VENDOR | USB_RECIP_DEVICE | 
> USB_DIR_OUT,
> +                                0x0022, address, data, datalen, 
> LINE6_TIMEOUT * HZ);
> +     if (ret) {
>               dev_err(line6->ifcdev,
>                       "write request failed (error %d)\n", ret);
>               goto exit;
> @@ -437,14 +426,10 @@ int line6_write_data(struct usb_line6 *line6, unsigned 
> address, void *data,
>       for (count = 0; count < LINE6_READ_WRITE_MAX_RETRIES; count++) {
>               mdelay(LINE6_READ_WRITE_STATUS_DELAY);
>  
> -             ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
> -                                   0x67,
> -                                   USB_TYPE_VENDOR | USB_RECIP_DEVICE |
> -                                   USB_DIR_IN,
> -                                   0x0012, 0x0000,
> -                                   status, 1, LINE6_TIMEOUT * HZ);
> -
> -             if (ret < 0) {
> +             ret = usb_control_msg_recv(usbdev, 0, 0x67,
> +                                        USB_TYPE_VENDOR | USB_RECIP_DEVICE | 
> USB_DIR_IN,
> +                                        0x0012, 0x0000, status, 1, 
> LINE6_TIMEOUT * HZ);
> +             if (ret) {
>                       dev_err(line6->ifcdev,
>                               "receiving status failed (error %d)\n", ret);
>                       goto exit;
> diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c
> index eef45f7fef0d..a1261f55d62b 100644
> --- a/sound/usb/line6/podhd.c
> +++ b/sound/usb/line6/podhd.c
> @@ -183,29 +183,25 @@ static const struct attribute_group 
> podhd_dev_attr_group = {
>  static int podhd_dev_start(struct usb_line6_podhd *pod)
>  {
>       int ret;
> -     u8 *init_bytes;
> +     u8 init_bytes[8];
>       int i;
>       struct usb_device *usbdev = pod->line6.usbdev;
>  
> -     init_bytes = kmalloc(8, GFP_KERNEL);
> -     if (!init_bytes)
> -             return -ENOMEM;
> -
> -     ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
> +     ret = usb_control_msg_send(usbdev, 0,
>                                       0x67, USB_TYPE_VENDOR | 
> USB_RECIP_DEVICE | USB_DIR_OUT,
>                                       0x11, 0,
>                                       NULL, 0, LINE6_TIMEOUT * HZ);
> -     if (ret < 0) {
> +     if (ret) {
>               dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", 
> ret);
>               goto exit;
>       }
>  
>       /* NOTE: looks like some kind of ping message */
> -     ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0), 0x67,
> +     ret = usb_control_msg_recv(usbdev, 0, 0x67,
>                                       USB_TYPE_VENDOR | USB_RECIP_DEVICE | 
> USB_DIR_IN,
>                                       0x11, 0x0,
>                                       init_bytes, 3, LINE6_TIMEOUT * HZ);
> -     if (ret < 0) {
> +     if (ret) {
>               dev_err(pod->line6.ifcdev,
>                       "receive length failed (error %d)\n", ret);
>               goto exit;
> @@ -220,13 +216,12 @@ static int podhd_dev_start(struct usb_line6_podhd *pod)
>                       goto exit;
>       }
>  
> -     ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
> +     ret = usb_control_msg_send(usbdev, 0,
>                                       USB_REQ_SET_FEATURE,
>                                       USB_TYPE_STANDARD | USB_RECIP_DEVICE | 
> USB_DIR_OUT,
>                                       1, 0,
>                                       NULL, 0, LINE6_TIMEOUT * HZ);
>  exit:
> -     kfree(init_bytes);
>       return ret;
>  }
>  
> diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c
> index 94dd5e7ab2e6..a9b56085b76a 100644
> --- a/sound/usb/line6/toneport.c
> +++ b/sound/usb/line6/toneport.c
> @@ -126,11 +126,11 @@ static int toneport_send_cmd(struct usb_device *usbdev, 
> int cmd1, int cmd2)
>  {
>       int ret;
>  
> -     ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x67,
> -                           USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
> -                           cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
> +     ret = usb_control_msg_send(usbdev, 0, 0x67,
> +                                USB_TYPE_VENDOR | USB_RECIP_DEVICE | 
> USB_DIR_OUT,
> +                                cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ);
>  
> -     if (ret < 0) {
> +     if (ret) {
>               dev_err(&usbdev->dev, "send failed (error %d)\n", ret);
>               return ret;
>       }
> -- 
> 2.28.0
> 

Reply via email to