netdev - Re: [PATCH net-next] ipv6: lockless IPV6_ADDR

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: Mon, 18 Sep 2023 20:48:26 -0600
From: David Ahern <dsahern@...nel.org>
To: Eric Dumazet <edumazet@...gle.com>, "David S . Miller"
 <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, eric.dumazet@...il.com
Subject: Re: [PATCH net-next] ipv6: lockless IPV6_ADDR_PREFERENCES
 implementation

On 9/18/23 8:23 AM, Eric Dumazet wrote:
> We have data-races while reading np->srcprefs
> 
> Switch the field to a plain byte, add READ_ONCE()
> and WRITE_ONCE() annotations where needed,
> and IPV6_ADDR_PREFERENCES setsockopt() can now be lockless.
> 
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> ---
>  include/linux/ipv6.h     |  2 +-
>  include/net/ip6_route.h  |  5 ++---
>  include/net/ipv6.h       | 20 +++++++-------------
>  net/ipv6/ip6_output.c    |  2 +-
>  net/ipv6/ipv6_sockglue.c | 19 ++++++++++---------
>  net/ipv6/route.c         |  2 +-
>  6 files changed, 22 insertions(+), 28 deletions(-)
> 
> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> index 09253825c99c7a94c4c8a3f176f0ceecd0b166bc..e400ff757f136e72e81277d48063551e445b4970 100644
> --- a/include/linux/ipv6.h
> +++ b/include/linux/ipv6.h
> @@ -243,7 +243,7 @@ struct ipv6_pinfo {
>  	} rxopt;
>  
>  	/* sockopt flags */
> -	__u8			srcprefs:3;	/* 001: prefer temporary address
> +	__u8			srcprefs;	/* 001: prefer temporary address
>  						 * 010: prefer public address
>  						 * 100: prefer care-of address
>  						 */
> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
> index b1ea49900b4ae17cb3436f884e26f5ae3a7a761c..28b0657902615157c4cbd836cc70e0767cf49a4d 100644
> --- a/include/net/ip6_route.h
> +++ b/include/net/ip6_route.h
> @@ -53,13 +53,12 @@ struct route_info {
>   */
>  static inline int rt6_srcprefs2flags(unsigned int srcprefs)
>  {
> -	/* No need to bitmask because srcprefs have only 3 bits. */
> -	return srcprefs << 3;
> +	return (srcprefs & IPV6_PREFER_SRC_MASK) << 3;
>  }
>  
>  static inline unsigned int rt6_flags2srcprefs(int flags)
>  {
> -	return (flags >> 3) & 7;
> +	return (flags >> 3) & IPV6_PREFER_SRC_MASK;
>  }
>  
>  static inline bool rt6_need_strict(const struct in6_addr *daddr)
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index bd115980809f386a7d38a5471d8d636f25ce1eba..b3444c8a6f744c17052a9fa1c85d54c6b08a1889 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -1306,10 +1306,13 @@ static inline void ip6_sock_set_recverr(struct sock *sk)
>  	inet6_set_bit(RECVERR6, sk);
>  }
>  
> -static inline int __ip6_sock_set_addr_preferences(struct sock *sk, int val)
> +#define IPV6_PREFER_SRC_MASK (IPV6_PREFER_SRC_TMP | IPV6_PREFER_SRC_PUBLIC | \
> +			      IPV6_PREFER_SRC_COA)
> +
> +static inline int ip6_sock_set_addr_preferences(struct sock *sk, int val)
>  {
> +	unsigned int prefmask = ~IPV6_PREFER_SRC_MASK;
>  	unsigned int pref = 0;
> -	unsigned int prefmask = ~0;
>  
>  	/* check PUBLIC/TMP/PUBTMP_DEFAULT conflicts */
>  	switch (val & (IPV6_PREFER_SRC_PUBLIC |

Unfortunate that address preference details are spread across 3 non-uapi
header files, but that is a change for a different patch.

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