netdev - Re: [PATCH net 3/5] nexthop: Expand nexthop_is

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]

Date:   Tue, 26 May 2020 18:08:15 +0300
From:   Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To:     David Ahern <dsahern@...nel.org>, netdev@...r.kernel.org
Cc:     davem@...emloft.net, kuba@...nel.org,
        David Ahern <dsahern@...il.com>
Subject: Re: [PATCH net 3/5] nexthop: Expand nexthop_is_multipath in a few
 places

On 26/05/2020 18:01, David Ahern wrote:
> From: David Ahern <dsahern@...il.com>
> 
> I got too fancy consolidating checks on multipath type. The result
> is that path lookups can access 2 different nh_grp structs as exposed
> by Nik's torture tests. Expand nexthop_is_multipath within nexthop.h to
> avoid multiple, nh_grp dereferences and make decisions based on the
> consistent struct.
> 
> Only 2 places left using nexthop_is_multipath are within IPv6, both
> only check that the nexthop is a multipath for a branching decision
> which are acceptable.
> 
> Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
> Signed-off-by: David Ahern <dsahern@...il.com>
> ---
>  include/net/nexthop.h | 41 +++++++++++++++++++++++++----------------
>  1 file changed, 25 insertions(+), 16 deletions(-)
> 

Acked-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>

> diff --git a/include/net/nexthop.h b/include/net/nexthop.h
> index 8a343519ed7a..f09e8d7d9886 100644
> --- a/include/net/nexthop.h
> +++ b/include/net/nexthop.h
> @@ -137,21 +137,20 @@ static inline unsigned int nexthop_num_path(const struct nexthop *nh)
>  {
>  	unsigned int rc = 1;
>  
> -	if (nexthop_is_multipath(nh)) {
> +	if (nh->is_group) {
>  		struct nh_group *nh_grp;
>  
>  		nh_grp = rcu_dereference_rtnl(nh->nh_grp);
> -		rc = nh_grp->num_nh;
> +		if (nh_grp->mpath)
> +			rc = nh_grp->num_nh;
>  	}
>  
>  	return rc;
>  }
>  
>  static inline
> -struct nexthop *nexthop_mpath_select(const struct nexthop *nh, int nhsel)
> +struct nexthop *nexthop_mpath_select(const struct nh_group *nhg, int nhsel)
>  {
> -	const struct nh_group *nhg = rcu_dereference_rtnl(nh->nh_grp);
> -
>  	/* for_nexthops macros in fib_semantics.c grabs a pointer to
>  	 * the nexthop before checking nhsel
>  	 */
> @@ -186,12 +185,14 @@ static inline bool nexthop_is_blackhole(const struct nexthop *nh)
>  {
>  	const struct nh_info *nhi;
>  
> -	if (nexthop_is_multipath(nh)) {
> -		if (nexthop_num_path(nh) > 1)
> -			return false;
> -		nh = nexthop_mpath_select(nh, 0);
> -		if (!nh)
> +	if (nh->is_group) {
> +		struct nh_group *nh_grp;
> +
> +		nh_grp = rcu_dereference_rtnl(nh->nh_grp);
> +		if (nh_grp->num_nh > 1)
>  			return false;
> +
> +		nh = nh_grp->nh_entries[0].nh;
>  	}
>  
>  	nhi = rcu_dereference_rtnl(nh->nh_info);
> @@ -217,10 +218,15 @@ struct fib_nh_common *nexthop_fib_nhc(struct nexthop *nh, int nhsel)
>  	BUILD_BUG_ON(offsetof(struct fib_nh, nh_common) != 0);
>  	BUILD_BUG_ON(offsetof(struct fib6_nh, nh_common) != 0);
>  
> -	if (nexthop_is_multipath(nh)) {
> -		nh = nexthop_mpath_select(nh, nhsel);
> -		if (!nh)
> -			return NULL;
> +	if (nh->is_group) {
> +		struct nh_group *nh_grp;
> +
> +		nh_grp = rcu_dereference_rtnl(nh->nh_grp);
> +		if (nh_grp->mpath) {
> +			nh = nexthop_mpath_select(nh_grp, nhsel);
> +			if (!nh)
> +				return NULL;
> +		}
>  	}
>  
>  	nhi = rcu_dereference_rtnl(nh->nh_info);
> @@ -264,8 +270,11 @@ static inline struct fib6_nh *nexthop_fib6_nh(struct nexthop *nh)
>  {
>  	struct nh_info *nhi;
>  
> -	if (nexthop_is_multipath(nh)) {
> -		nh = nexthop_mpath_select(nh, 0);
> +	if (nh->is_group) {
> +		struct nh_group *nh_grp;
> +
> +		nh_grp = rcu_dereference_rtnl(nh->nh_grp);
> +		nh = nexthop_mpath_select(nh_grp, 0);
>  		if (!nh)
>  			return NULL;
>  	}
>