diff mbox series

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

Message ID 20210714091327.677458-1-mudongliangabcd@gmail.com
State Superseded
Headers show
Series [v3,1/2] usb: hso: fix error handling code of hso_create_net_device | expand

Commit Message

Dongliang Mu July 14, 2021, 9:13 a.m. UTC
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 <mudongliangabcd@gmail.com>
---
v1->v2: change labels according to the comment of Dan Carpenter
v2->v3: change the style of error handling labels
 drivers/net/usb/hso.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

Comments

Greg Kroah-Hartman July 21, 2021, 7:36 a.m. UTC | #1
On Wed, Jul 14, 2021 at 05:13:22PM +0800, Dongliang Mu 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.
> 
> [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 <mudongliangabcd@gmail.com>
> ---
> v1->v2: change labels according to the comment of Dan Carpenter
> v2->v3: change the style of error handling labels
>  drivers/net/usb/hso.c | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)

Please resend the whole series, not just one patch of the series.
Otherwise it makes it impossible to determine what patch from what
series should be applied in what order.

All of these are now dropped from my queue, please fix up and resend.

thanks,

greg k-h
Dongliang Mu July 21, 2021, 8:17 a.m. UTC | #2
On Wed, Jul 21, 2021 at 3:36 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>

> On Wed, Jul 14, 2021 at 05:13:22PM +0800, Dongliang Mu 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.

> >

> > [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 <mudongliangabcd@gmail.com>

> > ---

> > v1->v2: change labels according to the comment of Dan Carpenter

> > v2->v3: change the style of error handling labels

> >  drivers/net/usb/hso.c | 33 +++++++++++++++++++++++----------

> >  1 file changed, 23 insertions(+), 10 deletions(-)

>

> Please resend the whole series, not just one patch of the series.

> Otherwise it makes it impossible to determine what patch from what

> series should be applied in what order.

>


Done. Please review the resend v3 patches.

> All of these are now dropped from my queue, please fix up and resend.

>

> thanks,

>

> greg k-h
Johan Hovold July 21, 2021, 2:34 p.m. UTC | #3
On Wed, Jul 21, 2021 at 04:17:01PM +0800, Dongliang Mu wrote:
> On Wed, Jul 21, 2021 at 3:36 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Jul 14, 2021 at 05:13:22PM +0800, Dongliang Mu 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.
> > >
> > > [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 <mudongliangabcd@gmail.com>
> > > ---
> > > v1->v2: change labels according to the comment of Dan Carpenter
> > > v2->v3: change the style of error handling labels
> > >  drivers/net/usb/hso.c | 33 +++++++++++++++++++++++----------
> > >  1 file changed, 23 insertions(+), 10 deletions(-)
> >
> > Please resend the whole series, not just one patch of the series.
> > Otherwise it makes it impossible to determine what patch from what
> > series should be applied in what order.
> >
> 
> Done. Please review the resend v3 patches.
> 
> > All of these are now dropped from my queue, please fix up and resend.

A version of this patch has already been applied to net-next.

No idea which version that was or why the second patch hasn't been
applied yet.

Dongliang, if you're resending something here it should first be rebased
on linux-next (net-next).

Johan
Johan Hovold July 21, 2021, 4:42 p.m. UTC | #4
On Wed, Jul 21, 2021 at 04:34:56PM +0200, Johan Hovold wrote:
> On Wed, Jul 21, 2021 at 04:17:01PM +0800, Dongliang Mu wrote:
> > On Wed, Jul 21, 2021 at 3:36 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Wed, Jul 14, 2021 at 05:13:22PM +0800, Dongliang Mu 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.
> > > >
> > > > [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 <mudongliangabcd@gmail.com>
> > > > ---
> > > > v1->v2: change labels according to the comment of Dan Carpenter
> > > > v2->v3: change the style of error handling labels
> > > >  drivers/net/usb/hso.c | 33 +++++++++++++++++++++++----------
> > > >  1 file changed, 23 insertions(+), 10 deletions(-)
> > >
> > > Please resend the whole series, not just one patch of the series.
> > > Otherwise it makes it impossible to determine what patch from what
> > > series should be applied in what order.
> > >
> > 
> > Done. Please review the resend v3 patches.
> > 
> > > All of these are now dropped from my queue, please fix up and resend.
> 
> A version of this patch has already been applied to net-next.

That was apparently net (not net-next).

> No idea which version that was or why the second patch hasn't been
> applied yet.
> 
> Dongliang, if you're resending something here it should first be rebased
> on linux-next (net-next).

And the resend of v3 of both patches has now also been applied to
net-next.

Hopefully there are no conflicts between v2 and v3 but we'll see soon.

Johan
Dongliang Mu July 22, 2021, 5:32 a.m. UTC | #5
On Thu, Jul 22, 2021 at 12:42 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Wed, Jul 21, 2021 at 04:34:56PM +0200, Johan Hovold wrote:
> > On Wed, Jul 21, 2021 at 04:17:01PM +0800, Dongliang Mu wrote:
> > > On Wed, Jul 21, 2021 at 3:36 PM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Wed, Jul 14, 2021 at 05:13:22PM +0800, Dongliang Mu 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.
> > > > >
> > > > > [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 <mudongliangabcd@gmail.com>
> > > > > ---
> > > > > v1->v2: change labels according to the comment of Dan Carpenter
> > > > > v2->v3: change the style of error handling labels
> > > > >  drivers/net/usb/hso.c | 33 +++++++++++++++++++++++----------
> > > > >  1 file changed, 23 insertions(+), 10 deletions(-)
> > > >
> > > > Please resend the whole series, not just one patch of the series.
> > > > Otherwise it makes it impossible to determine what patch from what
> > > > series should be applied in what order.
> > > >
> > >
> > > Done. Please review the resend v3 patches.
> > >
> > > > All of these are now dropped from my queue, please fix up and resend.
> >
> > A version of this patch has already been applied to net-next.
>
> That was apparently net (not net-next).
>
> > No idea which version that was or why the second patch hasn't been
> > applied yet.

It seems because I only sent the 1/2 patch in the v3. Also due to
this, gregkh asked me to resend the whole patchset again.

> >
> > Dongliang, if you're resending something here it should first be rebased
> > on linux-next (net-next).
>
> And the resend of v3 of both patches has now also been applied to
> net-next.
>
> Hopefully there are no conflicts between v2 and v3 but we'll see soon.

You mean you apply a v2 patch into one tree? This thread already
contains the v3 patch, and there is no v2 patch in the mailing list
due to one incomplete email subject.

BTW, v2->v3 only some label change due to naming style.

>
> Johan
Johan Hovold July 22, 2021, 7:23 a.m. UTC | #6
On Thu, Jul 22, 2021 at 01:32:48PM +0800, Dongliang Mu wrote:
> On Thu, Jul 22, 2021 at 12:42 AM Johan Hovold <johan@kernel.org> wrote:

> > > A version of this patch has already been applied to net-next.
> >
> > That was apparently net (not net-next).
> >
> > > No idea which version that was or why the second patch hasn't been
> > > applied yet.
> 
> It seems because I only sent the 1/2 patch in the v3. Also due to
> this, gregkh asked me to resend the whole patchset again.

Yeah, it's hard to keep track of submissions sometimes, especially if
not updating the whole series.

> > > Dongliang, if you're resending something here it should first be rebased
> > > on linux-next (net-next).
> >
> > And the resend of v3 of both patches has now also been applied to
> > net-next.
> >
> > Hopefully there are no conflicts between v2 and v3 but we'll see soon.
> 
> You mean you apply a v2 patch into one tree? This thread already
> contains the v3 patch, and there is no v2 patch in the mailing list
> due to one incomplete email subject.
> 
> BTW, v2->v3 only some label change due to naming style.

Ok, I can't keep track of this either. I just noticed that this patch
shows up in both net (for 5.14):

	https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=a6ecfb39ba9d7316057cea823b196b734f6b18ca

and net-next (for 5.15):

	https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=788e67f18d797abbd48a96143511261f4f3b4f21

The net one was applied on the 15th and the net-next one yesterday. 

Judging from a quick look it appears to be the same diff so no damage
done.

Johan
Dongliang Mu July 22, 2021, 7:51 a.m. UTC | #7
On Thu, Jul 22, 2021 at 3:24 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Thu, Jul 22, 2021 at 01:32:48PM +0800, Dongliang Mu wrote:
> > On Thu, Jul 22, 2021 at 12:42 AM Johan Hovold <johan@kernel.org> wrote:
>
> > > > A version of this patch has already been applied to net-next.
> > >
> > > That was apparently net (not net-next).
> > >
> > > > No idea which version that was or why the second patch hasn't been
> > > > applied yet.
> >
> > It seems because I only sent the 1/2 patch in the v3. Also due to
> > this, gregkh asked me to resend the whole patchset again.
>
> Yeah, it's hard to keep track of submissions sometimes, especially if
> not updating the whole series.
>
> > > > Dongliang, if you're resending something here it should first be rebased
> > > > on linux-next (net-next).
> > >
> > > And the resend of v3 of both patches has now also been applied to
> > > net-next.
> > >
> > > Hopefully there are no conflicts between v2 and v3 but we'll see soon.
> >
> > You mean you apply a v2 patch into one tree? This thread already
> > contains the v3 patch, and there is no v2 patch in the mailing list
> > due to one incomplete email subject.
> >
> > BTW, v2->v3 only some label change due to naming style.
>
> Ok, I can't keep track of this either. I just noticed that this patch
> shows up in both net (for 5.14):
>
>         https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=a6ecfb39ba9d7316057cea823b196b734f6b18ca
>
> and net-next (for 5.15):
>
>         https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=788e67f18d797abbd48a96143511261f4f3b4f21
>
> The net one was applied on the 15th and the net-next one yesterday.

I did not get any notification about this merge operation. So I cannot
help with this. Any chance to notify the developers the patch is
merged? In some subsystems, I will get notified by robots.

In the future, I will keep in mind updating the whole patch set. This
is easier for developers to follow.

>
> Judging from a quick look it appears to be the same diff so no damage
> done.

That's great.

>
> Johan
diff mbox series

Patch

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_free_tx_urb;
 
 	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_free_tx_buf;
 	}
 
 	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_free_tx_buf:
+	remove_net_device(hso_dev);
+	kfree(hso_net->mux_bulk_tx_buf);
+err_free_tx_urb:
+	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;
 }