diff mbox series

net/usb: kalmia: Don't pass act_len in usb_bulk_msg error path

Message ID 2f74aab82a40e4c11c91ccba40f5b620f6cb209c.camel@gmail.com
State New
Headers
Series net/usb: kalmia: Don't pass act_len in usb_bulk_msg error path |

Commit Message

Miko Larsson Feb. 10, 2023, 8:13 a.m. UTC 
  syzbot reported that act_len in kalmia_send_init_packet() is
uninitialized when passing it to the first usb_bulk_msg error path. Jiri
Pirko noted that it's pointless to pass it in the error path, and that
the value that would be printed in the second error path would be the
value of act_len from the first call to usb_bulk_msg.[1]

With this in mind, let's just not pass act_len to the usb_bulk_msg error
paths.

1: https://lore.kernel.org/lkml/Y9pY61y1nwTuzMOa@nanopsycho/

Fixes: d40261236e8e ("net/usb: Add Samsung Kalmia driver for Samsung GT-B3730")
Reported-and-tested-by: syzbot+cd80c5ef5121bfe85b55@syzkaller.appspotmail.com
Signed-off-by: Miko Larsson <mikoxyzzz@gmail.com>
---
 drivers/net/usb/kalmia.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Alexander Duyck Feb. 10, 2023, 4:08 p.m. UTC | #1 
On Fri, 2023-02-10 at 09:13 +0100, Miko Larsson wrote:
> syzbot reported that act_len in kalmia_send_init_packet() is
> uninitialized when passing it to the first usb_bulk_msg error path. Jiri
> Pirko noted that it's pointless to pass it in the error path, and that
> the value that would be printed in the second error path would be the
> value of act_len from the first call to usb_bulk_msg.[1]
> 
> With this in mind, let's just not pass act_len to the usb_bulk_msg error
> paths.
> 
> 1: https://lore.kernel.org/lkml/Y9pY61y1nwTuzMOa@nanopsycho/
> 
> Fixes: d40261236e8e ("net/usb: Add Samsung Kalmia driver for Samsung GT-B3730")
> Reported-and-tested-by: syzbot+cd80c5ef5121bfe85b55@syzkaller.appspotmail.com
> Signed-off-by: Miko Larsson <mikoxyzzz@gmail.com>
> ---
>  drivers/net/usb/kalmia.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/usb/kalmia.c b/drivers/net/usb/kalmia.c
> index 9f2b70ef39aa..613fc6910f14 100644
> --- a/drivers/net/usb/kalmia.c
> +++ b/drivers/net/usb/kalmia.c
> @@ -65,8 +65,8 @@ kalmia_send_init_packet(struct usbnet *dev, u8 *init_msg, u8 init_msg_len,
>  		init_msg, init_msg_len, &act_len, KALMIA_USB_TIMEOUT);
>  	if (status != 0) {
>  		netdev_err(dev->net,
> -			"Error sending init packet. Status %i, length %i\n",
> -			status, act_len);
> +			"Error sending init packet. Status %i\n",
> +			status);
>  		return status;
>  	}
>  	else if (act_len != init_msg_len) {
> @@ -83,8 +83,8 @@ kalmia_send_init_packet(struct usbnet *dev, u8 *init_msg, u8 init_msg_len,
>  
>  	if (status != 0)
>  		netdev_err(dev->net,
> -			"Error receiving init result. Status %i, length %i\n",
> -			status, act_len);
> +			"Error receiving init result. Status %i\n",
> +			status);
>  	else if (act_len != expected_len)
>  		netdev_err(dev->net, "Unexpected init result length: %i\n",
>  			act_len);

Makes sense to me since the only possible return values for act_len
appear to be either uninitialized or 0 depending on where it fails.

Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
patchwork-bot+netdevbpf@kernel.org Feb. 13, 2023, 9:50 a.m. UTC | #2 
Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri, 10 Feb 2023 09:13:44 +0100 you wrote:
> syzbot reported that act_len in kalmia_send_init_packet() is
> uninitialized when passing it to the first usb_bulk_msg error path. Jiri
> Pirko noted that it's pointless to pass it in the error path, and that
> the value that would be printed in the second error path would be the
> value of act_len from the first call to usb_bulk_msg.[1]
> 
> With this in mind, let's just not pass act_len to the usb_bulk_msg error
> paths.
> 
> [...]

Here is the summary with links:
  - net/usb: kalmia: Don't pass act_len in usb_bulk_msg error path
    https://git.kernel.org/netdev/net/c/c68f345b7c42

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/usb/kalmia.c b/drivers/net/usb/kalmia.c
index 9f2b70ef39aa..613fc6910f14 100644
--- a/drivers/net/usb/kalmia.c
+++ b/drivers/net/usb/kalmia.c
@@ -65,8 +65,8 @@  kalmia_send_init_packet(struct usbnet *dev, u8 *init_msg, u8 init_msg_len,
 		init_msg, init_msg_len, &act_len, KALMIA_USB_TIMEOUT);
 	if (status != 0) {
 		netdev_err(dev->net,
-			"Error sending init packet. Status %i, length %i\n",
-			status, act_len);
+			"Error sending init packet. Status %i\n",
+			status);
 		return status;
 	}
 	else if (act_len != init_msg_len) {
@@ -83,8 +83,8 @@  kalmia_send_init_packet(struct usbnet *dev, u8 *init_msg, u8 init_msg_len,
 
 	if (status != 0)
 		netdev_err(dev->net,
-			"Error receiving init result. Status %i, length %i\n",
-			status, act_len);
+			"Error receiving init result. Status %i\n",
+			status);
 	else if (act_len != expected_len)
 		netdev_err(dev->net, "Unexpected init result length: %i\n",
 			act_len);