diff mbox series

[net-next,09/10] ipv6: add READ_ONCE(sk->sk_bound_dev_if) in INET6_MATCH()

Message ID 20220511233757.2001218-10-eric.dumazet@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: add annotations for sk->sk_bound_dev_if | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 29 this patch: 29
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 12 this patch: 12
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 29 this patch: 29
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 30 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Eric Dumazet May 11, 2022, 11:37 p.m. UTC 
From: Eric Dumazet <edumazet@google.com>

INET6_MATCH() runs without holding a lock on the socket.

We probably need to annotate most reads.

This patch makes INET6_MATCH() an inline function
to ease our changes.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/inet6_hashtables.h | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

Comments

kernel test robot May 12, 2022, 3:48 p.m. UTC | #1 
Hi Eric,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Eric-Dumazet/net-add-annotations-for-sk-sk_bound_dev_if/20220512-073914
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git b57c7e8b76c646cf77ce4353a779a8b781592209
config: riscv-randconfig-r032-20220512 (https://download.01.org/0day-ci/archive/20220512/202205122338.qp5zlcyC-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 18dd123c56754edf62c7042dcf23185c3727610f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/ba3ce839eb3de33511aa07e29cabb8e7ed4e0cf0
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Eric-Dumazet/net-add-annotations-for-sk-sk_bound_dev_if/20220512-073914
        git checkout ba3ce839eb3de33511aa07e29cabb8e7ed4e0cf0
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash net/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from net/core/filter.c:26:
   In file included from include/linux/sock_diag.h:5:
   In file included from include/linux/netlink.h:7:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from net/core/filter.c:26:
   In file included from include/linux/sock_diag.h:5:
   In file included from include/linux/netlink.h:7:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from net/core/filter.c:26:
   In file included from include/linux/sock_diag.h:5:
   In file included from include/linux/netlink.h:7:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:1024:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
                                                     ~~~~~~~~~~ ^
   In file included from net/core/filter.c:64:
>> include/net/inet6_hashtables.h:119:28: error: no member named 'skc_v6_daddr' in 'struct sock_common'; did you mean 'skc_daddr'?
               !ipv6_addr_equal(&sk->sk_v6_daddr, saddr) ||
                                     ^
   include/net/sock.h:388:34: note: expanded from macro 'sk_v6_daddr'
   #define sk_v6_daddr             __sk_common.skc_v6_daddr
                                               ^
   include/net/sock.h:170:11: note: 'skc_daddr' declared here
                           __be32  skc_daddr;
                                   ^
   In file included from net/core/filter.c:64:
>> include/net/inet6_hashtables.h:120:28: error: no member named 'skc_v6_rcv_saddr' in 'struct sock_common'; did you mean 'skc_rcv_saddr'?
               !ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
                                     ^
   include/net/sock.h:389:37: note: expanded from macro 'sk_v6_rcv_saddr'
   #define sk_v6_rcv_saddr __sk_common.skc_v6_rcv_saddr
                                       ^
   include/net/sock.h:171:11: note: 'skc_rcv_saddr' declared here
                           __be32  skc_rcv_saddr;
                                   ^
   7 warnings and 2 errors generated.


vim +119 include/net/inet6_hashtables.h

   107	
   108	static inline bool INET6_MATCH(const struct sock *sk, struct net *net,
   109				       const struct in6_addr *saddr,
   110				       const struct in6_addr *daddr,
   111				       const __portpair ports,
   112				       const int dif, const int sdif)
   113	{
   114		int bound_dev_if;
   115	
   116		if (!net_eq(sock_net(sk), net) ||
   117		    sk->sk_family != AF_INET6 ||
   118		    sk->sk_portpair != ports ||
 > 119		    !ipv6_addr_equal(&sk->sk_v6_daddr, saddr) ||
 > 120		    !ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
kernel test robot May 13, 2022, 1 a.m. UTC | #2 
Hi Eric,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Eric-Dumazet/net-add-annotations-for-sk-sk_bound_dev_if/20220512-073914
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git b57c7e8b76c646cf77ce4353a779a8b781592209
config: x86_64-randconfig-a015 (https://download.01.org/0day-ci/archive/20220513/202205130816.Wo8OVaxg-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/ba3ce839eb3de33511aa07e29cabb8e7ed4e0cf0
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Eric-Dumazet/net-add-annotations-for-sk-sk_bound_dev_if/20220512-073914
        git checkout ba3ce839eb3de33511aa07e29cabb8e7ed4e0cf0
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/sock_diag.h:8,
                    from net/core/filter.c:26:
   include/net/inet6_hashtables.h: In function 'INET6_MATCH':
>> include/net/sock.h:388:45: error: 'const struct sock_common' has no member named 'skc_v6_daddr'; did you mean 'skc_daddr'?
     388 | #define sk_v6_daddr             __sk_common.skc_v6_daddr
         |                                             ^~~~~~~~~~~~
   include/net/inet6_hashtables.h:119:35: note: in expansion of macro 'sk_v6_daddr'
     119 |             !ipv6_addr_equal(&sk->sk_v6_daddr, saddr) ||
         |                                   ^~~~~~~~~~~
>> include/net/sock.h:389:37: error: 'const struct sock_common' has no member named 'skc_v6_rcv_saddr'; did you mean 'skc_rcv_saddr'?
     389 | #define sk_v6_rcv_saddr __sk_common.skc_v6_rcv_saddr
         |                                     ^~~~~~~~~~~~~~~~
   include/net/inet6_hashtables.h:120:35: note: in expansion of macro 'sk_v6_rcv_saddr'
     120 |             !ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
         |                                   ^~~~~~~~~~~~~~~


vim +388 include/net/sock.h

4dc6dc7162c08b9 Eric Dumazet             2009-07-15  368  
68835aba4d9b74e Eric Dumazet             2010-11-30  369  #define sk_dontcopy_begin	__sk_common.skc_dontcopy_begin
68835aba4d9b74e Eric Dumazet             2010-11-30  370  #define sk_dontcopy_end		__sk_common.skc_dontcopy_end
4dc6dc7162c08b9 Eric Dumazet             2009-07-15  371  #define sk_hash			__sk_common.skc_hash
5080546682bae3d Eric Dumazet             2013-10-02  372  #define sk_portpair		__sk_common.skc_portpair
05dbc7b59481ca8 Eric Dumazet             2013-10-03  373  #define sk_num			__sk_common.skc_num
05dbc7b59481ca8 Eric Dumazet             2013-10-03  374  #define sk_dport		__sk_common.skc_dport
5080546682bae3d Eric Dumazet             2013-10-02  375  #define sk_addrpair		__sk_common.skc_addrpair
5080546682bae3d Eric Dumazet             2013-10-02  376  #define sk_daddr		__sk_common.skc_daddr
5080546682bae3d Eric Dumazet             2013-10-02  377  #define sk_rcv_saddr		__sk_common.skc_rcv_saddr
^1da177e4c3f415 Linus Torvalds           2005-04-16  378  #define sk_family		__sk_common.skc_family
^1da177e4c3f415 Linus Torvalds           2005-04-16  379  #define sk_state		__sk_common.skc_state
^1da177e4c3f415 Linus Torvalds           2005-04-16  380  #define sk_reuse		__sk_common.skc_reuse
055dc21a1d1d219 Tom Herbert              2013-01-22  381  #define sk_reuseport		__sk_common.skc_reuseport
9fe516ba3fb29b6 Eric Dumazet             2014-06-27  382  #define sk_ipv6only		__sk_common.skc_ipv6only
26abe14379f8e2f Eric W. Biederman        2015-05-08  383  #define sk_net_refcnt		__sk_common.skc_net_refcnt
^1da177e4c3f415 Linus Torvalds           2005-04-16  384  #define sk_bound_dev_if		__sk_common.skc_bound_dev_if
^1da177e4c3f415 Linus Torvalds           2005-04-16  385  #define sk_bind_node		__sk_common.skc_bind_node
8feaf0c0a5488b3 Arnaldo Carvalho de Melo 2005-08-09  386  #define sk_prot			__sk_common.skc_prot
07feaebfcc10cd3 Eric W. Biederman        2007-09-12  387  #define sk_net			__sk_common.skc_net
efe4208f47f907b Eric Dumazet             2013-10-03 @388  #define sk_v6_daddr		__sk_common.skc_v6_daddr
efe4208f47f907b Eric Dumazet             2013-10-03 @389  #define sk_v6_rcv_saddr	__sk_common.skc_v6_rcv_saddr
33cf7c90fe2f97a Eric Dumazet             2015-03-11  390  #define sk_cookie		__sk_common.skc_cookie
70da268b569d32a Eric Dumazet             2015-10-08  391  #define sk_incoming_cpu		__sk_common.skc_incoming_cpu
8e5eb54d303b7cb Eric Dumazet             2015-10-08  392  #define sk_flags		__sk_common.skc_flags
ed53d0ab761f5c7 Eric Dumazet             2015-10-08  393  #define sk_rxhash		__sk_common.skc_rxhash
efe4208f47f907b Eric Dumazet             2013-10-03  394  
43f51df41729559 Eric Dumazet             2021-11-15  395  	/* early demux fields */
8b3f91332291fa2 Jakub Kicinski           2021-12-23  396  	struct dst_entry __rcu	*sk_rx_dst;
43f51df41729559 Eric Dumazet             2021-11-15  397  	int			sk_rx_dst_ifindex;
43f51df41729559 Eric Dumazet             2021-11-15  398  	u32			sk_rx_dst_cookie;
43f51df41729559 Eric Dumazet             2021-11-15  399  
^1da177e4c3f415 Linus Torvalds           2005-04-16  400  	socket_lock_t		sk_lock;
9115e8cd2a0c6ea Eric Dumazet             2016-12-03  401  	atomic_t		sk_drops;
9115e8cd2a0c6ea Eric Dumazet             2016-12-03  402  	int			sk_rcvlowat;
9115e8cd2a0c6ea Eric Dumazet             2016-12-03  403  	struct sk_buff_head	sk_error_queue;
b178bb3dfc30d95 Eric Dumazet             2010-11-16  404  	struct sk_buff_head	sk_receive_queue;
fa438ccfdfd3f6d Eric Dumazet             2007-03-04  405  	/*
fa438ccfdfd3f6d Eric Dumazet             2007-03-04  406  	 * The backlog queue is special, it is always used with
fa438ccfdfd3f6d Eric Dumazet             2007-03-04  407  	 * the per-socket spinlock held and requires low latency
fa438ccfdfd3f6d Eric Dumazet             2007-03-04  408  	 * access. Therefore we special case it's implementation.
b178bb3dfc30d95 Eric Dumazet             2010-11-16  409  	 * Note : rmem_alloc is in this structure to fill a hole
b178bb3dfc30d95 Eric Dumazet             2010-11-16  410  	 * on 64bit arches, not because its logically part of
b178bb3dfc30d95 Eric Dumazet             2010-11-16  411  	 * backlog.
fa438ccfdfd3f6d Eric Dumazet             2007-03-04  412  	 */
fa438ccfdfd3f6d Eric Dumazet             2007-03-04  413  	struct {
b178bb3dfc30d95 Eric Dumazet             2010-11-16  414  		atomic_t	rmem_alloc;
b178bb3dfc30d95 Eric Dumazet             2010-11-16  415  		int		len;
fa438ccfdfd3f6d Eric Dumazet             2007-03-04  416  		struct sk_buff	*head;
fa438ccfdfd3f6d Eric Dumazet             2007-03-04  417  		struct sk_buff	*tail;
fa438ccfdfd3f6d Eric Dumazet             2007-03-04  418  	} sk_backlog;
f35f821935d8df7 Eric Dumazet             2021-11-15  419
diff mbox series

Patch

diff --git a/include/net/inet6_hashtables.h b/include/net/inet6_hashtables.h
index 81b96595303680dee9c43f8c64d97b71fb4c4977..a7523e2e7ce50564c3ef1b3563a55dc80a03927d 100644
--- a/include/net/inet6_hashtables.h
+++ b/include/net/inet6_hashtables.h
@@ -105,13 +105,22 @@  struct sock *inet6_lookup(struct net *net, struct inet_hashinfo *hashinfo,
 int inet6_hash(struct sock *sk);
 #endif /* IS_ENABLED(CONFIG_IPV6) */
 
-#define INET6_MATCH(__sk, __net, __saddr, __daddr, __ports, __dif, __sdif) \
-	(((__sk)->sk_portpair == (__ports))			&&	\
-	 ((__sk)->sk_family == AF_INET6)			&&	\
-	 ipv6_addr_equal(&(__sk)->sk_v6_daddr, (__saddr))		&&	\
-	 ipv6_addr_equal(&(__sk)->sk_v6_rcv_saddr, (__daddr))	&&	\
-	 (((__sk)->sk_bound_dev_if == (__dif))	||			\
-	  ((__sk)->sk_bound_dev_if == (__sdif)))		&&	\
-	 net_eq(sock_net(__sk), (__net)))
+static inline bool INET6_MATCH(const struct sock *sk, struct net *net,
+			       const struct in6_addr *saddr,
+			       const struct in6_addr *daddr,
+			       const __portpair ports,
+			       const int dif, const int sdif)
+{
+	int bound_dev_if;
 
+	if (!net_eq(sock_net(sk), net) ||
+	    sk->sk_family != AF_INET6 ||
+	    sk->sk_portpair != ports ||
+	    !ipv6_addr_equal(&sk->sk_v6_daddr, saddr) ||
+	    !ipv6_addr_equal(&sk->sk_v6_rcv_saddr, daddr))
+		return false;
+
+	bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);
+	return bound_dev_if == dif || bound_dev_if == sdif;
+}
 #endif /* _INET6_HASHTABLES_H */