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:   Fri, 28 Oct 2022 13:15:16 +0800
From:   kernel test robot <lkp@...el.com>
To:     Dmitry Safonov <dima@...sta.com>, linux-kernel@...r.kernel.org,
        David Ahern <dsahern@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>
Cc:     llvm@...ts.linux.dev, oe-kbuild-all@...ts.linux.dev,
        Dmitry Safonov <dima@...sta.com>,
        Andy Lutomirski <luto@...capital.net>,
        Ard Biesheuvel <ardb@...nel.org>,
        Bob Gilligan <gilligan@...sta.com>,
        Dan Carpenter <error27@...il.com>,
        Eric Biggers <ebiggers@...nel.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Francesco Ruggeri <fruggeri@...sta.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        Ivan Delalande <colona@...sta.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Leonard Crestez <cdleonard@...il.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Salam Noureddine <noureddine@...sta.com>,
        Shuah Khan <skhan@...uxfoundation.org>, netdev@...r.kernel.org,
        linux-crypto@...r.kernel.org
Subject: Re: [PATCH v3 17/36] net/tcp: Verify inbound TCP-AO signed segments

Hi Dmitry,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 4dc12f37a8e98e1dca5521c14625c869537b50b6]

url:    https://github.com/intel-lab-lkp/linux/commits/Dmitry-Safonov/net-tcp-Add-TCP-AO-support/20221028-045452
base:   4dc12f37a8e98e1dca5521c14625c869537b50b6
patch link:    https://lore.kernel.org/r/20221027204347.529913-18-dima%40arista.com
patch subject: [PATCH v3 17/36] net/tcp: Verify inbound TCP-AO signed segments
config: x86_64-randconfig-a005
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
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
        # https://github.com/intel-lab-lkp/linux/commit/f735fab5365661e9531aa2d77f5bf959d347dd21
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Dmitry-Safonov/net-tcp-Add-TCP-AO-support/20221028-045452
        git checkout f735fab5365661e9531aa2d77f5bf959d347dd21
        # 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=x86_64 SHELL=/bin/bash net/ipv4/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@...el.com>

All warnings (new ones prefixed by >>):

>> net/ipv4/tcp_ao.c:800:14: warning: variable 'sisn' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
                   } else if (unlikely(th->ack && !th->syn)) {
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:48:23: note: expanded from macro 'unlikely'
   #  define unlikely(x)   (__branch_check__(x, 0, __builtin_constant_p(x)))
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/ipv4/tcp_ao.c:819:46: note: uninitialized use occurs here
           ops->ao_calc_key_skb(key, traffic_key, skb, sisn, disn);
                                                       ^~~~
   net/ipv4/tcp_ao.c:800:10: note: remove the 'if' if its condition is always true
                   } else if (unlikely(th->ack && !th->syn)) {
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/ipv4/tcp_ao.c:727:13: note: initialize the variable 'sisn' to silence this warning
           __be32 sisn, disn;
                      ^
                       = 0
>> net/ipv4/tcp_ao.c:800:14: warning: variable 'disn' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
                   } else if (unlikely(th->ack && !th->syn)) {
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:48:23: note: expanded from macro 'unlikely'
   #  define unlikely(x)   (__branch_check__(x, 0, __builtin_constant_p(x)))
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/ipv4/tcp_ao.c:819:52: note: uninitialized use occurs here
           ops->ao_calc_key_skb(key, traffic_key, skb, sisn, disn);
                                                             ^~~~
   net/ipv4/tcp_ao.c:800:10: note: remove the 'if' if its condition is always true
                   } else if (unlikely(th->ack && !th->syn)) {
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/ipv4/tcp_ao.c:727:19: note: initialize the variable 'disn' to silence this warning
           __be32 sisn, disn;
                            ^
                             = 0
   2 warnings generated.


vim +800 net/ipv4/tcp_ao.c

   715	
   716	enum skb_drop_reason
   717	tcp_inbound_ao_hash(struct sock *sk, const struct sk_buff *skb,
   718			    unsigned short int family, const struct request_sock *req,
   719			    const struct tcp_ao_hdr *aoh)
   720	{
   721		const struct tcp_sock_af_ops *ops = tcp_sk(sk)->af_specific;
   722		u8 key_buf[TCP_AO_MAX_HASH_SIZE] __tcp_ao_key_align;
   723		const struct tcphdr *th = tcp_hdr(skb);
   724		u8 *phash = (u8 *)(aoh + 1); /* hash goes just after the header */
   725		struct tcp_ao_info *info;
   726		struct tcp_ao_key *key;
   727		__be32 sisn, disn;
   728		u8 *traffic_key;
   729		u32 sne = 0;
   730	
   731		info = rcu_dereference(tcp_sk(sk)->ao_info);
   732		if (!info)
   733			return SKB_DROP_REASON_TCP_AOUNEXPECTED;
   734	
   735		/* Fast-path */
   736		/* TODO: fix fastopen and simultaneous open (TCPF_SYN_RECV) */
   737		if (likely((1 << sk->sk_state) & (TCP_AO_ESTABLISHED | TCPF_SYN_RECV))) {
   738			enum skb_drop_reason err;
   739	
   740			/* Check if this socket's rnext_key matches the keyid in the
   741			 * packet. If not we lookup the key based on the keyid
   742			 * matching the rcvid in the mkt.
   743			 */
   744			key = info->rnext_key;
   745			if (key->rcvid != aoh->keyid) {
   746				key = tcp_ao_do_lookup_rcvid(sk, aoh->keyid);
   747				if (!key)
   748					goto key_not_found;
   749			}
   750	
   751			if (unlikely(th->syn && !th->ack)) {
   752				/* Delayed retransmitted syn */
   753				sisn = th->seq;
   754				disn = 0;
   755				goto verify_hash;
   756			}
   757	
   758			sne = tcp_ao_compute_sne(info->rcv_sne, info->rcv_sne_seq,
   759						 ntohl(th->seq));
   760			/* Established socket, traffic key are cached */
   761			traffic_key = rcv_other_key(key);
   762			err = tcp_ao_verify_hash(sk, skb, family, info, aoh, key,
   763						 traffic_key, phash, sne);
   764			if (err)
   765				return err;
   766			/* Key rotation: the peer asks us to use new key (RNext) */
   767			if (unlikely(aoh->rnext_keyid != info->current_key->sndid)) {
   768				/* If the key is not found we do nothing. */
   769				key = tcp_ao_do_lookup_sndid(sk, aoh->rnext_keyid);
   770				if (key)
   771					/* pairs with tcp_ao_del_cmd */
   772					WRITE_ONCE(info->current_key, key);
   773			}
   774			return SKB_NOT_DROPPED_YET;
   775		}
   776	
   777		/* Lookup key based on peer address and keyid.
   778		 * current_key and rnext_key must not be used on tcp listen
   779		 * sockets as otherwise:
   780		 * - request sockets would race on those key pointers
   781		 * - tcp_ao_del_cmd() allows async key removal
   782		 */
   783		key = tcp_ao_inbound_lookup(family, sk, skb, -1, aoh->keyid);
   784		if (!key)
   785			goto key_not_found;
   786	
   787		if (th->syn && !th->ack) {
   788			sisn = th->seq;
   789			disn = 0;
   790			goto verify_hash;
   791		}
   792	
   793		if (sk->sk_state == TCP_LISTEN) {
   794			/* Make the initial syn the likely case here */
   795			if (unlikely(req)) {
   796				sne = tcp_ao_compute_sne(0, tcp_rsk(req)->rcv_isn,
   797							 ntohl(th->seq));
   798				sisn = htonl(tcp_rsk(req)->rcv_isn);
   799				disn = htonl(tcp_rsk(req)->snt_isn);
 > 800			} else if (unlikely(th->ack && !th->syn)) {
   801				/* Possible syncookie packet */
   802				sisn = htonl(ntohl(th->seq) - 1);
   803				disn = htonl(ntohl(th->ack_seq) - 1);
   804				sne = tcp_ao_compute_sne(0, ntohl(sisn),
   805							 ntohl(th->seq));
   806			}
   807		} else if (sk->sk_state == TCP_SYN_SENT) {
   808			disn = info->lisn;
   809			if (th->syn)
   810				sisn = th->seq;
   811			else
   812				sisn = info->risn;
   813		} else {
   814			WARN_ONCE(1, "TCP-AO: Unknown sk_state %d", sk->sk_state);
   815			return SKB_DROP_REASON_TCP_AOFAILURE;
   816		}
   817	verify_hash:
   818		traffic_key = key_buf;
   819		ops->ao_calc_key_skb(key, traffic_key, skb, sisn, disn);
   820		return tcp_ao_verify_hash(sk, skb, family, info, aoh, key,
   821					  traffic_key, phash, sne);
   822	
   823	key_not_found:
   824		return SKB_DROP_REASON_TCP_AOKEYNOTFOUND;
   825	}
   826	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

View attachment "config" of type "text/plain" (141264 bytes)

Powered by blists - more mailing lists