[PATCH 1/2] usb: hso: fix error handling code of hso_create_net_device

Subsystems: hso 3g modem driver, networking drivers, the rest, usb networking drivers

STALE1877d

3 messages, 2 authors, 2021-07-14 · open the first message on its own page

[PATCH 1/2] usb: hso: fix error handling code of hso_create_net_device

From: Dongliang Mu <hidden>
Date: 2021-07-14 08:11:51

The current error handling code of hso_create_net_device is
hso_free_net_device, no matter which errors lead to. For example,
WARNING in hso_free_net_device [1].

Fix this by refactoring the error handling code of
hso_create_net_device by handling different errors by different code.

[1] https://syzkaller.appspot.com/bug?id=66eff8d49af1b28370ad342787413e35bbe76efe

Reported-by: syzbot+44d53c7255bb1aea22d2@syzkaller.appspotmail.com
Fixes: 5fcfb6d0bfcd ("hso: fix bailout in error case of probe")
Signed-off-by: Dongliang Mu <redacted>
---
 drivers/net/usb/hso.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 54ef8492ca01..39c4e88eab62 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2495,7 +2495,7 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
 			   hso_net_init);
 	if (!net) {
 		dev_err(&interface->dev, "Unable to create ethernet device\n");
-		goto exit;
+		goto err_hso_dev;
 	}
 
 	hso_net = netdev_priv(net);
@@ -2508,13 +2508,13 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
 				      USB_DIR_IN);
 	if (!hso_net->in_endp) {
 		dev_err(&interface->dev, "Can't find BULK IN endpoint\n");
-		goto exit;
+		goto err_net;
 	}
 	hso_net->out_endp = hso_get_ep(interface, USB_ENDPOINT_XFER_BULK,
 				       USB_DIR_OUT);
 	if (!hso_net->out_endp) {
 		dev_err(&interface->dev, "Can't find BULK OUT endpoint\n");
-		goto exit;
+		goto err_net;
 	}
 	SET_NETDEV_DEV(net, &interface->dev);
 	SET_NETDEV_DEVTYPE(net, &hso_type);
@@ -2523,18 +2523,18 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
 	for (i = 0; i < MUX_BULK_RX_BUF_COUNT; i++) {
 		hso_net->mux_bulk_rx_urb_pool[i] = usb_alloc_urb(0, GFP_KERNEL);
 		if (!hso_net->mux_bulk_rx_urb_pool[i])
-			goto exit;
+			goto err_mux_bulk_rx;
 		hso_net->mux_bulk_rx_buf_pool[i] = kzalloc(MUX_BULK_RX_BUF_SIZE,
 							   GFP_KERNEL);
 		if (!hso_net->mux_bulk_rx_buf_pool[i])
-			goto exit;
+			goto err_mux_bulk_rx;
 	}
 	hso_net->mux_bulk_tx_urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!hso_net->mux_bulk_tx_urb)
-		goto exit;
+		goto err_mux_bulk_rx;
 	hso_net->mux_bulk_tx_buf = kzalloc(MUX_BULK_TX_BUF_SIZE, GFP_KERNEL);
 	if (!hso_net->mux_bulk_tx_buf)
-		goto exit;
+		goto err_mux_bulk_tx;
 
 	add_net_device(hso_dev);
 
@@ -2542,7 +2542,7 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
 	result = register_netdev(net);
 	if (result) {
 		dev_err(&interface->dev, "Failed to register device\n");
-		goto exit;
+		goto err_register;
 	}
 
 	hso_log_port(hso_dev);
@@ -2550,8 +2550,21 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
 	hso_create_rfkill(hso_dev, interface);
 
 	return hso_dev;
-exit:
-	hso_free_net_device(hso_dev, true);
+
+err_register:
+	remove_net_device(hso_dev);
+	kfree(hso_net->mux_bulk_tx_buf);
+err_mux_bulk_tx:
+	usb_free_urb(hso_net->mux_bulk_tx_urb);
+err_mux_bulk_rx:
+	for (i = 0; i < MUX_BULK_RX_BUF_COUNT; i++) {
+		usb_free_urb(hso_net->mux_bulk_rx_urb_pool[i]);
+		kfree(hso_net->mux_bulk_rx_buf_pool[i]);
+	}
+err_net:
+	free_netdev(net);
+err_hso_dev:
+	kfree(hso_dev);
 	return NULL;
 }
 
-- 
2.25.1

Re: [PATCH 1/2] usb: hso: fix error handling code of hso_create_net_device

From: Dongliang Mu <hidden>
Date: 2021-07-14 08:14:50

On Wed, Jul 14, 2021 at 4:11 PM Dongliang Mu [off-list ref] wrote:
The current error handling code of hso_create_net_device is
hso_free_net_device, no matter which errors lead to. For example,
WARNING in hso_free_net_device [1].

Fix this by refactoring the error handling code of
hso_create_net_device by handling different errors by different code.
Hi Dan,

Please take a look at this version. I forget about the changelog about
this patch. I will send a version v3 with your further comment if you
have.
quoted hunk
[1] https://syzkaller.appspot.com/bug?id=66eff8d49af1b28370ad342787413e35bbe76efe

Reported-by: syzbot+44d53c7255bb1aea22d2@syzkaller.appspotmail.com
Fixes: 5fcfb6d0bfcd ("hso: fix bailout in error case of probe")
Signed-off-by: Dongliang Mu <redacted>
---
 drivers/net/usb/hso.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 54ef8492ca01..39c4e88eab62 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2495,7 +2495,7 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
                           hso_net_init);
        if (!net) {
                dev_err(&interface->dev, "Unable to create ethernet device\n");
-               goto exit;
+               goto err_hso_dev;
        }

        hso_net = netdev_priv(net);
@@ -2508,13 +2508,13 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
                                      USB_DIR_IN);
        if (!hso_net->in_endp) {
                dev_err(&interface->dev, "Can't find BULK IN endpoint\n");
-               goto exit;
+               goto err_net;
        }
        hso_net->out_endp = hso_get_ep(interface, USB_ENDPOINT_XFER_BULK,
                                       USB_DIR_OUT);
        if (!hso_net->out_endp) {
                dev_err(&interface->dev, "Can't find BULK OUT endpoint\n");
-               goto exit;
+               goto err_net;
        }
        SET_NETDEV_DEV(net, &interface->dev);
        SET_NETDEV_DEVTYPE(net, &hso_type);
@@ -2523,18 +2523,18 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
        for (i = 0; i < MUX_BULK_RX_BUF_COUNT; i++) {
                hso_net->mux_bulk_rx_urb_pool[i] = usb_alloc_urb(0, GFP_KERNEL);
                if (!hso_net->mux_bulk_rx_urb_pool[i])
-                       goto exit;
+                       goto err_mux_bulk_rx;
                hso_net->mux_bulk_rx_buf_pool[i] = kzalloc(MUX_BULK_RX_BUF_SIZE,
                                                           GFP_KERNEL);
                if (!hso_net->mux_bulk_rx_buf_pool[i])
-                       goto exit;
+                       goto err_mux_bulk_rx;
        }
        hso_net->mux_bulk_tx_urb = usb_alloc_urb(0, GFP_KERNEL);
        if (!hso_net->mux_bulk_tx_urb)
-               goto exit;
+               goto err_mux_bulk_rx;
        hso_net->mux_bulk_tx_buf = kzalloc(MUX_BULK_TX_BUF_SIZE, GFP_KERNEL);
        if (!hso_net->mux_bulk_tx_buf)
-               goto exit;
+               goto err_mux_bulk_tx;

        add_net_device(hso_dev);
@@ -2542,7 +2542,7 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
        result = register_netdev(net);
        if (result) {
                dev_err(&interface->dev, "Failed to register device\n");
-               goto exit;
+               goto err_register;
        }

        hso_log_port(hso_dev);
@@ -2550,8 +2550,21 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
        hso_create_rfkill(hso_dev, interface);

        return hso_dev;
-exit:
-       hso_free_net_device(hso_dev, true);
+
+err_register:
+       remove_net_device(hso_dev);
+       kfree(hso_net->mux_bulk_tx_buf);
+err_mux_bulk_tx:
+       usb_free_urb(hso_net->mux_bulk_tx_urb);
+err_mux_bulk_rx:
+       for (i = 0; i < MUX_BULK_RX_BUF_COUNT; i++) {
+               usb_free_urb(hso_net->mux_bulk_rx_urb_pool[i]);
+               kfree(hso_net->mux_bulk_rx_buf_pool[i]);
+       }
+err_net:
+       free_netdev(net);
+err_hso_dev:
+       kfree(hso_dev);
        return NULL;
 }

--
2.25.1

Re: [PATCH 1/2] usb: hso: fix error handling code of hso_create_net_device

From: Dan Carpenter <hidden>
Date: 2021-07-14 08:31:45

On Wed, Jul 14, 2021 at 04:14:18PM +0800, Dongliang Mu wrote:
quoted
@@ -2523,18 +2523,18 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
        for (i = 0; i < MUX_BULK_RX_BUF_COUNT; i++) {
                hso_net->mux_bulk_rx_urb_pool[i] = usb_alloc_urb(0, GFP_KERNEL);
                if (!hso_net->mux_bulk_rx_urb_pool[i])
-                       goto exit;
+                       goto err_mux_bulk_rx;
                hso_net->mux_bulk_rx_buf_pool[i] = kzalloc(MUX_BULK_RX_BUF_SIZE,
                                                           GFP_KERNEL);
                if (!hso_net->mux_bulk_rx_buf_pool[i])
-                       goto exit;
+                       goto err_mux_bulk_rx;
        }
        hso_net->mux_bulk_tx_urb = usb_alloc_urb(0, GFP_KERNEL);
        if (!hso_net->mux_bulk_tx_urb)
-               goto exit;
+               goto err_mux_bulk_rx;
        hso_net->mux_bulk_tx_buf = kzalloc(MUX_BULK_TX_BUF_SIZE, GFP_KERNEL);
        if (!hso_net->mux_bulk_tx_buf)
-               goto exit;
+               goto err_mux_bulk_tx;
I would probably have called this err free_tx_urb or something like
that.
quoted
        add_net_device(hso_dev);
@@ -2542,7 +2542,7 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
        result = register_netdev(net);
        if (result) {
                dev_err(&interface->dev, "Failed to register device\n");
-               goto exit;
+               goto err_register;
This is still Come From style.  I wouldn't have commented except that
you said you are giong to redo the patch for other reasons.

It looks good.  Straight forward to review now.

regards,
dan carpenter
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help