[PATCH net] net: sit: unregister_netdevice on newlink's error path

[PATCH net] net: sit: unregister_netdevice on newlink's error path

[Jakub Kicinski]

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

Re: [PATCH net] net: sit: unregister_netdevice on newlink's error path

[Nicolas Dichtel]

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;
> 

Re: [PATCH net] net: sit: unregister_netdevice on newlink's error path

[Jakub Kicinski]

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!