Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net] net: sit: unregister_netdevice on newlink's error path
@ 2021-01-14  1:29 Jakub Kicinski
  2021-01-14  8:49 ` Nicolas Dichtel
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2021-01-14  1:29 UTC (permalink / raw)
  To: davem
  Cc: netdev, nicolas.dichtel, Jakub Kicinski, syzbot+2393580080a2da190f04

We need to unregister the netdevice if config failed.
.ndo_uninit takes care of most of the heavy lifting.

This was uncovered by recent commit c269a24ce057 ("net: make
free_netdev() more lenient with unregistering devices").
Previously the partially-initialized device would be left
in the system.

Reported-and-tested-by: syzbot+2393580080a2da190f04@syzkaller.appspotmail.com
Fixes: e2f1f072db8d ("sit: allow to configure 6rd tunnels via netlink")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/ipv6/sit.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 2da0ee703779..440175bc2e89 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -1645,8 +1645,11 @@ static int ipip6_newlink(struct net *src_net, struct net_device *dev,
 	}
 
 #ifdef CONFIG_IPV6_SIT_6RD
-	if (ipip6_netlink_6rd_parms(data, &ip6rd))
+	if (ipip6_netlink_6rd_parms(data, &ip6rd)) {
 		err = ipip6_tunnel_update_6rd(nt, &ip6rd);
+		if (err)
+			unregister_netdevice_queue(dev, NULL);
+	}
 #endif
 
 	return err;
-- 
2.26.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: sit: unregister_netdevice on newlink's error path
  2021-01-14  1:29 [PATCH net] net: sit: unregister_netdevice on newlink's error path Jakub Kicinski
@ 2021-01-14  8:49 ` Nicolas Dichtel
  2021-01-14 18:27   ` Jakub Kicinski
  0 siblings, 1 reply; 3+ messages in thread
From: Nicolas Dichtel @ 2021-01-14  8:49 UTC (permalink / raw)
  To: Jakub Kicinski, davem; +Cc: netdev, syzbot+2393580080a2da190f04

Le 14/01/2021 à 02:29, Jakub Kicinski a écrit :
[snip]
> --- a/net/ipv6/sit.c
> +++ b/net/ipv6/sit.c
> @@ -1645,8 +1645,11 @@ static int ipip6_newlink(struct net *src_net, struct net_device *dev,
>  	}
>  
>  #ifdef CONFIG_IPV6_SIT_6RD
> -	if (ipip6_netlink_6rd_parms(data, &ip6rd))
> +	if (ipip6_netlink_6rd_parms(data, &ip6rd)) {
>  		err = ipip6_tunnel_update_6rd(nt, &ip6rd);
> +		if (err)
nit: I would prefer 'if (err < 0)' to be consistent with the rest of the sit
code, but it's purely aesthetic (ipip6_tunnel_update_6rd() returns a negative
value or 0).

With or without this:
Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

> +			unregister_netdevice_queue(dev, NULL);
> +	}
>  #endif
>  
>  	return err;
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: sit: unregister_netdevice on newlink's error path
  2021-01-14  8:49 ` Nicolas Dichtel
@ 2021-01-14 18:27   ` Jakub Kicinski
  0 siblings, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2021-01-14 18:27 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: davem, netdev, syzbot+2393580080a2da190f04

On Thu, 14 Jan 2021 09:49:48 +0100 Nicolas Dichtel wrote:
> Le 14/01/2021 à 02:29, Jakub Kicinski a écrit :
> [snip]
> > --- a/net/ipv6/sit.c
> > +++ b/net/ipv6/sit.c
> > @@ -1645,8 +1645,11 @@ static int ipip6_newlink(struct net *src_net, struct net_device *dev,
> >  	}
> >  
> >  #ifdef CONFIG_IPV6_SIT_6RD
> > -	if (ipip6_netlink_6rd_parms(data, &ip6rd))
> > +	if (ipip6_netlink_6rd_parms(data, &ip6rd)) {
> >  		err = ipip6_tunnel_update_6rd(nt, &ip6rd);
> > +		if (err)  
> nit: I would prefer 'if (err < 0)' to be consistent with the rest of the sit
> code, but it's purely aesthetic (ipip6_tunnel_update_6rd() returns a negative
> value or 0).

Done.

> With or without this:
> Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Thanks for the review, applied!

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-14  1:29 [PATCH net] net: sit: unregister_netdevice on newlink's error path Jakub Kicinski
2021-01-14  8:49 ` Nicolas Dichtel
2021-01-14 18:27   ` Jakub Kicinski

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git