[08/10] sound: line6: move to use usb_control_msg_send() and usb_control_msg_recv()

Message ID 20200902110115.1994491-11-gregkh@linuxfoundation.org
State Superseded
Headers show
Series
  • USB: new USB control message helper functions
Related show

Commit Message

Greg Kroah-Hartman Sept. 2, 2020, 11:01 a.m.
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 <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Vasily Khoruzhick <anarsoul@gmail.com>
Cc: alsa-devel@alsa-project.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 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(-)

Comments

Takashi Iwai Sept. 2, 2020, 2:38 p.m. | #1
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 <perex@perex.cz>
> Cc: Takashi Iwai <tiwai@suse.com>
> Cc: Vasily Khoruzhick <anarsoul@gmail.com>
> Cc: alsa-devel@alsa-project.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Reviewed-by: Takashi Iwai <tiwai@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
>

Patch

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;
 	}