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