netdev - Re: [PATCH net] ipv6: take care of disable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e398f5ae-6dad-2d98-aa29-fd1fda759b28@kernel.org>
Date:   Sun, 26 Jun 2022 11:19:04 -0600
From:   David Ahern <dsahern@...nel.org>
To:     Nicolas Dichtel <nicolas.dichtel@...nd.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Steffen Klassert <klassert@...nel.org>,
        Herbert Xu <herbert@...dor.apana.org.au>
Cc:     netdev@...r.kernel.org, stable@...nel.org,
        David Forster <dforster@...cade.com>,
        Siwar Zitouni <siwar.zitouni@...nd.com>
Subject: Re: [PATCH net] ipv6: take care of disable_policy when restoring
 routes

On 6/23/22 6:00 AM, Nicolas Dichtel wrote:
> When routes corresponding to addresses are restored by
> fixup_permanent_addr(), the dst_nopolicy parameter was not set.
> The typical use case is a user that configures an address on a down
> interface and then put this interface up.
> 
> Let's take care of this flag in addrconf_f6i_alloc(), so that every callers
> benefit ont it.
> 
> CC: stable@...nel.org
> CC: David Forster <dforster@...cade.com>
> Fixes: df789fe75206 ("ipv6: Provide ipv6 version of "disable_policy" sysctl")
> Reported-by: Siwar Zitouni <siwar.zitouni@...nd.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@...nd.com>
> ---
> 
> I choose to be conservative in my patch, thus I filter anycast addresses. But
> I don't see why this flag is not set on anycast routes. Any thoughts
> about this?

no strong opinions here.

> 
>  net/ipv6/addrconf.c | 4 ----
>  net/ipv6/route.c    | 9 ++++++++-
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 1b1932502e9e..5864cbc30db6 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -1109,10 +1109,6 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
>  		goto out;
>  	}
>  
> -	if (net->ipv6.devconf_all->disable_policy ||
> -	    idev->cnf.disable_policy)
> -		f6i->dst_nopolicy = true;
> -
>  	neigh_parms_data_state_setall(idev->nd_parms);
>  
>  	ifa->addr = *cfg->pfx;
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index d25dc83bac62..828355710c57 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -4569,8 +4569,15 @@ struct fib6_info *addrconf_f6i_alloc(struct net *net,
>  	}
>  
>  	f6i = ip6_route_info_create(&cfg, gfp_flags, NULL);
> -	if (!IS_ERR(f6i))
> +	if (!IS_ERR(f6i)) {
>  		f6i->dst_nocount = true;
> +
> +		if (!anycast &&
> +		    (net->ipv6.devconf_all->disable_policy ||
> +		     idev->cnf.disable_policy))
> +			f6i->dst_nopolicy = true;
> +	}
> +
>  	return f6i;
>  }
>  

Reviewed-by: David Ahern <dsahern@...nel.org>