* [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).