All 3 upstream commits apply cleanly: * 5fcfb6d0bfcd ("hso: fix bailout in error case of probe") is a support patch needed for context * a6ecfb39ba9d ("usb: hso: fix error handling code of hso_create_net_device") is the actual fix * dcb713d53e2e ("usb: hso: remove the bailout parameter") is a follow up cleanup commit
Dongliang Mu (2): usb: hso: fix error handling code of hso_create_net_device usb: hso: remove the bailout parameter
Oliver Neukum (1): hso: fix bailout in error case of probe
drivers/net/usb/hso.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-)
From: Oliver Neukum oneukum@suse.com
commit 5fcfb6d0bfcda17f0d0656e4e5b3710af2bbaae5 upstream.
The driver tries to reuse code for disconnect in case of a failed probe. If resources need to be freed after an error in probe, the netdev must not be freed because it has never been registered. Fix it by telling the helper which path we are in.
Signed-off-by: Oliver Neukum oneukum@suse.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com --- drivers/net/usb/hso.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c index 22450c4a9225..fb8827dd5671 100644 --- a/drivers/net/usb/hso.c +++ b/drivers/net/usb/hso.c @@ -2354,7 +2354,7 @@ static int remove_net_device(struct hso_device *hso_dev) }
/* Frees our network device */ -static void hso_free_net_device(struct hso_device *hso_dev) +static void hso_free_net_device(struct hso_device *hso_dev, bool bailout) { int i; struct hso_net *hso_net = dev2net(hso_dev); @@ -2377,7 +2377,7 @@ static void hso_free_net_device(struct hso_device *hso_dev) kfree(hso_net->mux_bulk_tx_buf); hso_net->mux_bulk_tx_buf = NULL;
- if (hso_net->net) + if (hso_net->net && !bailout) free_netdev(hso_net->net);
kfree(hso_dev); @@ -2553,7 +2553,7 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
return hso_dev; exit: - hso_free_net_device(hso_dev); + hso_free_net_device(hso_dev, true); return NULL; }
@@ -3122,7 +3122,7 @@ static void hso_free_interface(struct usb_interface *interface) rfkill_unregister(rfk); rfkill_destroy(rfk); } - hso_free_net_device(network_table[i]); + hso_free_net_device(network_table[i], false); } } }
From: Dongliang Mu mudongliangabcd@gmail.com
commit a6ecfb39ba9d7316057cea823b196b734f6b18ca upstream.
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=66eff8d49af1b28370ad342787413e35bbe76ef...
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 Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com --- 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 fb8827dd5671..762f7a4db809 100644 --- a/drivers/net/usb/hso.c +++ b/drivers/net/usb/hso.c @@ -2497,7 +2497,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); @@ -2510,13 +2510,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); @@ -2525,18 +2525,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);
@@ -2544,7 +2544,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); @@ -2552,8 +2552,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; }
From: Dongliang Mu mudongliangabcd@gmail.com
commit dcb713d53e2eadf42b878c12a471e74dc6ed3145 upstream.
There are two invocation sites of hso_free_net_device. After refactoring hso_create_net_device, this parameter is useless. Remove the bailout in the hso_free_net_device and change the invocation sites of this function.
Signed-off-by: Dongliang Mu mudongliangabcd@gmail.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com --- drivers/net/usb/hso.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c index 762f7a4db809..95da2576a221 100644 --- a/drivers/net/usb/hso.c +++ b/drivers/net/usb/hso.c @@ -2354,7 +2354,7 @@ static int remove_net_device(struct hso_device *hso_dev) }
/* Frees our network device */ -static void hso_free_net_device(struct hso_device *hso_dev, bool bailout) +static void hso_free_net_device(struct hso_device *hso_dev) { int i; struct hso_net *hso_net = dev2net(hso_dev); @@ -2377,7 +2377,7 @@ static void hso_free_net_device(struct hso_device *hso_dev, bool bailout) kfree(hso_net->mux_bulk_tx_buf); hso_net->mux_bulk_tx_buf = NULL;
- if (hso_net->net && !bailout) + if (hso_net->net) free_netdev(hso_net->net);
kfree(hso_dev); @@ -3135,7 +3135,7 @@ static void hso_free_interface(struct usb_interface *interface) rfkill_unregister(rfk); rfkill_destroy(rfk); } - hso_free_net_device(network_table[i], false); + hso_free_net_device(network_table[i]); } } }
Hi Ovidiu
On Tue, Sep 28, 2021 at 04:15:20PM +0300, Ovidiu Panait wrote:
All 3 upstream commits apply cleanly:
- 5fcfb6d0bfcd ("hso: fix bailout in error case of probe") is a support patch needed for context
- a6ecfb39ba9d ("usb: hso: fix error handling code of hso_create_net_device") is the actual fix
- dcb713d53e2e ("usb: hso: remove the bailout parameter") is a follow up cleanup commit
Dongliang Mu (2): usb: hso: fix error handling code of hso_create_net_device usb: hso: remove the bailout parameter
Oliver Neukum (1): hso: fix bailout in error case of probe
drivers/net/usb/hso.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-)
Noticing you sent this patch series for 4.14, 4.19 and 5.4 but am I right that the last commit dcb713d53e2e ("usb: hso: remove the bailout parameter") as cleanup commit should ideally as well be applied to 5.10.y and 5.14.y?
Whilst it's probably not strictly needed it would otherwise leave the upper 5.10.y and 5.14.y inconsistent with those where these series are applied.
Regards, Salvatore
Hi Salvatore,
On 9/28/21 10:29 PM, Salvatore Bonaccorso wrote:
[Please note: This e-mail is from an EXTERNAL e-mail address]
Hi Ovidiu
On Tue, Sep 28, 2021 at 04:15:20PM +0300, Ovidiu Panait wrote:
All 3 upstream commits apply cleanly: * 5fcfb6d0bfcd ("hso: fix bailout in error case of probe") is a support patch needed for context * a6ecfb39ba9d ("usb: hso: fix error handling code of hso_create_net_device") is the actual fix * dcb713d53e2e ("usb: hso: remove the bailout parameter") is a follow up cleanup commit
Dongliang Mu (2): usb: hso: fix error handling code of hso_create_net_device usb: hso: remove the bailout parameter
Oliver Neukum (1): hso: fix bailout in error case of probe
drivers/net/usb/hso.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-)
Noticing you sent this patch series for 4.14, 4.19 and 5.4 but am I right that the last commit dcb713d53e2e ("usb: hso: remove the bailout parameter") as cleanup commit should ideally as well be applied to 5.10.y and 5.14.y?
Whilst it's probably not strictly needed it would otherwise leave the upper 5.10.y and 5.14.y inconsistent with those where these series are applied.
You're right, I have sent the cleanup patch for 5.10/5.14 integration as well:
https://lore.kernel.org/stable/20210929075940.1961832-1-ovidiu.panait@windri...
Thanks!
Ovidiu
Regards, Salvatore
On Wed, Sep 29, 2021 at 11:03:19AM +0300, Ovidiu Panait wrote:
Hi Salvatore,
On 9/28/21 10:29 PM, Salvatore Bonaccorso wrote:
[Please note: This e-mail is from an EXTERNAL e-mail address]
Hi Ovidiu
On Tue, Sep 28, 2021 at 04:15:20PM +0300, Ovidiu Panait wrote:
All 3 upstream commits apply cleanly:
- 5fcfb6d0bfcd ("hso: fix bailout in error case of probe") is a support patch needed for context
- a6ecfb39ba9d ("usb: hso: fix error handling code of hso_create_net_device") is the actual fix
- dcb713d53e2e ("usb: hso: remove the bailout parameter") is a follow up cleanup commit
Dongliang Mu (2): usb: hso: fix error handling code of hso_create_net_device usb: hso: remove the bailout parameter
Oliver Neukum (1): hso: fix bailout in error case of probe
drivers/net/usb/hso.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-)
Noticing you sent this patch series for 4.14, 4.19 and 5.4 but am I right that the last commit dcb713d53e2e ("usb: hso: remove the bailout parameter") as cleanup commit should ideally as well be applied to 5.10.y and 5.14.y?
Whilst it's probably not strictly needed it would otherwise leave the upper 5.10.y and 5.14.y inconsistent with those where these series are applied.
You're right, I have sent the cleanup patch for 5.10/5.14 integration as well:
https://lore.kernel.org/stable/20210929075940.1961832-1-ovidiu.panait@windri...
Why do we need that cleanup commit in <=5.4 to begin with? Does it actually fix anything?
Hi Sasha,
On 10/1/21 7:55 PM, Sasha Levin wrote:
[Please note: This e-mail is from an EXTERNAL e-mail address]
On Wed, Sep 29, 2021 at 11:03:19AM +0300, Ovidiu Panait wrote:
Hi Salvatore,
On 9/28/21 10:29 PM, Salvatore Bonaccorso wrote:
[Please note: This e-mail is from an EXTERNAL e-mail address]
Hi Ovidiu
On Tue, Sep 28, 2021 at 04:15:20PM +0300, Ovidiu Panait wrote:
All 3 upstream commits apply cleanly: * 5fcfb6d0bfcd ("hso: fix bailout in error case of probe") is a support patch needed for context * a6ecfb39ba9d ("usb: hso: fix error handling code of hso_create_net_device") is the actual fix * dcb713d53e2e ("usb: hso: remove the bailout parameter") is a follow up cleanup commit
Dongliang Mu (2): usb: hso: fix error handling code of hso_create_net_device usb: hso: remove the bailout parameter
Oliver Neukum (1): hso: fix bailout in error case of probe
drivers/net/usb/hso.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-)
Noticing you sent this patch series for 4.14, 4.19 and 5.4 but am I right that the last commit dcb713d53e2e ("usb: hso: remove the bailout parameter") as cleanup commit should ideally as well be applied to 5.10.y and 5.14.y?
Whilst it's probably not strictly needed it would otherwise leave the upper 5.10.y and 5.14.y inconsistent with those where these series are applied.
You're right, I have sent the cleanup patch for 5.10/5.14 integration as well:
https://lore.kernel.org/stable/20210929075940.1961832-1-ovidiu.panait@windri...
Why do we need that cleanup commit in <=5.4 to begin with? Does it actually fix anything?
The cleanup patch was part of the same patchset with a6ecfb39ba9d ("usb: hso: fix error handling code of hso_create_net_device") fix .
I think it can be dropped, as it doesn't seem to fix anything. Can only the first two commits be cherry-picked for <=5.4, or should I resend?
Ovidiu
-- Thanks, Sasha
Hi,
On Sat, Oct 02, 2021 at 03:36:21PM +0300, Ovidiu Panait wrote:
Hi Sasha,
On 10/1/21 7:55 PM, Sasha Levin wrote:
[Please note: This e-mail is from an EXTERNAL e-mail address]
On Wed, Sep 29, 2021 at 11:03:19AM +0300, Ovidiu Panait wrote:
Hi Salvatore,
On 9/28/21 10:29 PM, Salvatore Bonaccorso wrote:
[Please note: This e-mail is from an EXTERNAL e-mail address]
Hi Ovidiu
On Tue, Sep 28, 2021 at 04:15:20PM +0300, Ovidiu Panait wrote:
All 3 upstream commits apply cleanly: * 5fcfb6d0bfcd ("hso: fix bailout in error case of probe") is a support patch needed for context * a6ecfb39ba9d ("usb: hso: fix error handling code of hso_create_net_device") is the actual fix * dcb713d53e2e ("usb: hso: remove the bailout parameter") is a follow up cleanup commit
Dongliang Mu (2): usb: hso: fix error handling code of hso_create_net_device usb: hso: remove the bailout parameter
Oliver Neukum (1): hso: fix bailout in error case of probe
drivers/net/usb/hso.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-)
Noticing you sent this patch series for 4.14, 4.19 and 5.4 but am I right that the last commit dcb713d53e2e ("usb: hso: remove the bailout parameter") as cleanup commit should ideally as well be applied to 5.10.y and 5.14.y?
Whilst it's probably not strictly needed it would otherwise leave the upper 5.10.y and 5.14.y inconsistent with those where these series are applied.
You're right, I have sent the cleanup patch for 5.10/5.14 integration as well:
https://lore.kernel.org/stable/20210929075940.1961832-1-ovidiu.panait@windri...
Why do we need that cleanup commit in <=5.4 to begin with? Does it actually fix anything?
The cleanup patch was part of the same patchset with a6ecfb39ba9d ("usb: hso: fix error handling code of hso_create_net_device") fix .
I think it can be dropped, as it doesn't seem to fix anything. Can only the first two commits be cherry-picked for <=5.4, or should I resend?
Probably the right thing to do, Sasha and Ovidiu. Picking it would have the small advantage of future commits to backport which would conflict around the changed lines.
But I have no voice on that matter, I was really only going thorugh some stable commits backports request covering CVEs and noticed the submission and it's discrepancy.
For Debian I have for now picked all three commits on top of 4.19.208.
Regards, Salvatore
On Tue, Sep 28, 2021 at 04:15:20PM +0300, Ovidiu Panait wrote:
All 3 upstream commits apply cleanly:
- 5fcfb6d0bfcd ("hso: fix bailout in error case of probe") is a support patch needed for context
- a6ecfb39ba9d ("usb: hso: fix error handling code of hso_create_net_device") is the actual fix
- dcb713d53e2e ("usb: hso: remove the bailout parameter") is a follow up cleanup commit
Dongliang Mu (2): usb: hso: fix error handling code of hso_create_net_device usb: hso: remove the bailout parameter
Oliver Neukum (1): hso: fix bailout in error case of probe
drivers/net/usb/hso.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-)
-- 2.25.1
All now queued up, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Ovidiu Panait -
Salvatore Bonaccorso -
Sasha Levin