* [PATCH v2] net: hso: do not call unregister if not registered
@ 2020-10-02 11:43 Greg KH
2020-10-04 0:00 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2020-10-02 11:43 UTC (permalink / raw)
To: netdev
Cc: Tuba Yavuz, David S. Miller, Jakub Kicinski, Oliver Neukum,
linux-kernel, linux-usb
From: Tuba Yavuz <tuba@ece.ufl.edu>
On an error path inside the hso_create_net_device function of the hso
driver, hso_free_net_device gets called. This causes a use-after-free
and a double-free if register_netdev has not been called yet as
hso_free_net_device calls unregister_netdev regardless. I think the
driver should distinguish these cases and call unregister_netdev only if
register_netdev has been called.
Signed-off-by: Tuba Yavuz <tuba@ece.ufl.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2: format cleaned up based on feedback from previous review
Forward to Greg to submit due to email problems on Tuba's side
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 2bb28db89432..e6b56bdf691d 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2366,7 +2366,8 @@ static void hso_free_net_device(struct hso_device *hso_dev, bool bailout)
remove_net_device(hso_net->parent);
- if (hso_net->net)
+ if (hso_net->net &&
+ hso_net->net->reg_state == NETREG_REGISTERED)
unregister_netdev(hso_net->net);
/* start freeing */
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] net: hso: do not call unregister if not registered 2020-10-02 11:43 [PATCH v2] net: hso: do not call unregister if not registered Greg KH @ 2020-10-04 0:00 ` David Miller 2020-10-04 7:14 ` Greg KH 0 siblings, 1 reply; 6+ messages in thread From: David Miller @ 2020-10-04 0:00 UTC (permalink / raw) To: gregkh; +Cc: netdev, tuba, kuba, oneukum, linux-kernel, linux-usb From: Greg KH <gregkh@linuxfoundation.org> Date: Fri, 2 Oct 2020 13:43:23 +0200 > @@ -2366,7 +2366,8 @@ static void hso_free_net_device(struct hso_device *hso_dev, bool bailout) > > remove_net_device(hso_net->parent); > > - if (hso_net->net) > + if (hso_net->net && > + hso_net->net->reg_state == NETREG_REGISTERED) > unregister_netdev(hso_net->net); > > /* start freeing */ I really want to get out of the habit of drivers testing the internal netdev registration state to make decisions. Instead, please track this internally. You know if you registered the device or not, therefore use that to control whether you try to unregister it or not. Thank you. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] net: hso: do not call unregister if not registered 2020-10-04 0:00 ` David Miller @ 2020-10-04 7:14 ` Greg KH 2021-08-16 6:52 ` Salvatore Bonaccorso 0 siblings, 1 reply; 6+ messages in thread From: Greg KH @ 2020-10-04 7:14 UTC (permalink / raw) To: David Miller, tuba; +Cc: netdev, kuba, oneukum, linux-kernel, linux-usb On Sat, Oct 03, 2020 at 05:00:42PM -0700, David Miller wrote: > From: Greg KH <gregkh@linuxfoundation.org> > Date: Fri, 2 Oct 2020 13:43:23 +0200 > > > @@ -2366,7 +2366,8 @@ static void hso_free_net_device(struct hso_device *hso_dev, bool bailout) > > > > remove_net_device(hso_net->parent); > > > > - if (hso_net->net) > > + if (hso_net->net && > > + hso_net->net->reg_state == NETREG_REGISTERED) > > unregister_netdev(hso_net->net); > > > > /* start freeing */ > > I really want to get out of the habit of drivers testing the internal > netdev registration state to make decisions. > > Instead, please track this internally. You know if you registered the > device or not, therefore use that to control whether you try to > unregister it or not. Fair enough. Tuba, do you want to fix this up in this way, or do you recommend that someone else do it? thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] net: hso: do not call unregister if not registered 2020-10-04 7:14 ` Greg KH @ 2021-08-16 6:52 ` Salvatore Bonaccorso 2021-08-16 7:02 ` Greg KH 2021-08-17 15:19 ` Yavuz, Tuba 0 siblings, 2 replies; 6+ messages in thread From: Salvatore Bonaccorso @ 2021-08-16 6:52 UTC (permalink / raw) To: Greg KH Cc: David Miller, tuba, netdev, kuba, oneukum, linux-kernel, linux-usb Hi Greg, Tuba, On Sun, Oct 04, 2020 at 09:14:33AM +0200, Greg KH wrote: > On Sat, Oct 03, 2020 at 05:00:42PM -0700, David Miller wrote: > > From: Greg KH <gregkh@linuxfoundation.org> > > Date: Fri, 2 Oct 2020 13:43:23 +0200 > > > > > @@ -2366,7 +2366,8 @@ static void hso_free_net_device(struct hso_device *hso_dev, bool bailout) > > > > > > remove_net_device(hso_net->parent); > > > > > > - if (hso_net->net) > > > + if (hso_net->net && > > > + hso_net->net->reg_state == NETREG_REGISTERED) > > > unregister_netdev(hso_net->net); > > > > > > /* start freeing */ > > > > I really want to get out of the habit of drivers testing the internal > > netdev registration state to make decisions. > > > > Instead, please track this internally. You know if you registered the > > device or not, therefore use that to control whether you try to > > unregister it or not. > > Fair enough. Tuba, do you want to fix this up in this way, or do you > recommend that someone else do it? Do I miss something, or did that possibly fall through the cracks? I was checking some open issues on a downstream distro side and found htat this thread did not got a follow-up. Regards, Salvatore ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] net: hso: do not call unregister if not registered 2021-08-16 6:52 ` Salvatore Bonaccorso @ 2021-08-16 7:02 ` Greg KH 2021-08-17 15:19 ` Yavuz, Tuba 1 sibling, 0 replies; 6+ messages in thread From: Greg KH @ 2021-08-16 7:02 UTC (permalink / raw) To: Salvatore Bonaccorso Cc: David Miller, tuba, netdev, kuba, oneukum, linux-kernel, linux-usb On Mon, Aug 16, 2021 at 08:52:58AM +0200, Salvatore Bonaccorso wrote: > Hi Greg, Tuba, > > On Sun, Oct 04, 2020 at 09:14:33AM +0200, Greg KH wrote: > > On Sat, Oct 03, 2020 at 05:00:42PM -0700, David Miller wrote: > > > From: Greg KH <gregkh@linuxfoundation.org> > > > Date: Fri, 2 Oct 2020 13:43:23 +0200 > > > > > > > @@ -2366,7 +2366,8 @@ static void hso_free_net_device(struct hso_device *hso_dev, bool bailout) > > > > > > > > remove_net_device(hso_net->parent); > > > > > > > > - if (hso_net->net) > > > > + if (hso_net->net && > > > > + hso_net->net->reg_state == NETREG_REGISTERED) > > > > unregister_netdev(hso_net->net); > > > > > > > > /* start freeing */ > > > > > > I really want to get out of the habit of drivers testing the internal > > > netdev registration state to make decisions. > > > > > > Instead, please track this internally. You know if you registered the > > > device or not, therefore use that to control whether you try to > > > unregister it or not. > > > > Fair enough. Tuba, do you want to fix this up in this way, or do you > > recommend that someone else do it? > > Do I miss something, or did that possibly fall through the cracks? > > I was checking some open issues on a downstream distro side and found > htat this thread did not got a follow-up. I did not see a follow-up patch :( ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] net: hso: do not call unregister if not registered 2021-08-16 6:52 ` Salvatore Bonaccorso 2021-08-16 7:02 ` Greg KH @ 2021-08-17 15:19 ` Yavuz, Tuba 1 sibling, 0 replies; 6+ messages in thread From: Yavuz, Tuba @ 2021-08-17 15:19 UTC (permalink / raw) To: Salvatore Bonaccorso, Greg KH Cc: David Miller, netdev, kuba, oneukum, linux-kernel, linux-usb Hi Salvatore, I think it would be best if one of the developers of the hso driver could develop a patch along the lines of David Miller's earlier suggestion. Best, Tuba ________________________________________ From: Salvatore Bonaccorso <salvatore.bonaccorso@gmail.com> on behalf of Salvatore Bonaccorso <carnil@debian.org> Sent: Monday, August 16, 2021 2:52 AM To: Greg KH Cc: David Miller; Yavuz, Tuba; netdev@vger.kernel.org; kuba@kernel.org; oneukum@suse.com; linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org Subject: Re: [PATCH v2] net: hso: do not call unregister if not registered [External Email] Hi Greg, Tuba, On Sun, Oct 04, 2020 at 09:14:33AM +0200, Greg KH wrote: > On Sat, Oct 03, 2020 at 05:00:42PM -0700, David Miller wrote: > > From: Greg KH <gregkh@linuxfoundation.org> > > Date: Fri, 2 Oct 2020 13:43:23 +0200 > > > > > @@ -2366,7 +2366,8 @@ static void hso_free_net_device(struct hso_device *hso_dev, bool bailout) > > > > > > remove_net_device(hso_net->parent); > > > > > > - if (hso_net->net) > > > + if (hso_net->net && > > > + hso_net->net->reg_state == NETREG_REGISTERED) > > > unregister_netdev(hso_net->net); > > > > > > /* start freeing */ > > > > I really want to get out of the habit of drivers testing the internal > > netdev registration state to make decisions. > > > > Instead, please track this internally. You know if you registered the > > device or not, therefore use that to control whether you try to > > unregister it or not. > > Fair enough. Tuba, do you want to fix this up in this way, or do you > recommend that someone else do it? Do I miss something, or did that possibly fall through the cracks? I was checking some open issues on a downstream distro side and found htat this thread did not got a follow-up. Regards, Salvatore ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-17 15:47 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-02 11:43 [PATCH v2] net: hso: do not call unregister if not registered Greg KH 2020-10-04 0:00 ` David Miller 2020-10-04 7:14 ` Greg KH 2021-08-16 6:52 ` Salvatore Bonaccorso 2021-08-16 7:02 ` Greg KH 2021-08-17 15:19 ` Yavuz, Tuba
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).