Re: [PATCH] ipv6: check return value of ipv6_skip_exthdr

[kernel test robot <lkp@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 5463 bytes --]

Hi Jordy,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on klassert-ipsec/master]
[also build test WARNING on klassert-ipsec-next/master linux/master v5.16-rc1 next-20211118]
[cannot apply to linus/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jordy-Zomer/ipv6-check-return-value-of-ipv6_skip_exthdr/20211118-021714
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/afe093a81395e12df66d6b2145bcb98d4fc67b55
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jordy-Zomer/ipv6-check-return-value-of-ipv6_skip_exthdr/20211118-021714
        git checkout afe093a81395e12df66d6b2145bcb98d4fc67b55
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

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

All warnings (new ones prefixed by >>):

   net/ipv6/esp6.c: In function 'esp6_input_done2':
>> net/ipv6/esp6.c:812:3: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
     812 |   if (offset < 0)
         |   ^~
   net/ipv6/esp6.c:814:4: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
     814 |    goto out;
         |    ^~~~

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for SND_SOC_MT6359
   Depends on SOUND && !UML && SND && SND_SOC && MTK_PMIC_WRAP
   Selected by
   - SND_SOC_MT8195_MT6359_RT1019_RT5682 && SOUND && !UML && SND && SND_SOC && I2C && SND_SOC_MT8195


vim +/if +812 net/ipv6/esp6.c

   782	
   783	int esp6_input_done2(struct sk_buff *skb, int err)
   784	{
   785		struct xfrm_state *x = xfrm_input_state(skb);
   786		struct xfrm_offload *xo = xfrm_offload(skb);
   787		struct crypto_aead *aead = x->data;
   788		int hlen = sizeof(struct ip_esp_hdr) + crypto_aead_ivsize(aead);
   789		int hdr_len = skb_network_header_len(skb);
   790	
   791		if (!xo || !(xo->flags & CRYPTO_DONE))
   792			kfree(ESP_SKB_CB(skb)->tmp);
   793	
   794		if (unlikely(err))
   795			goto out;
   796	
   797		err = esp_remove_trailer(skb);
   798		if (unlikely(err < 0))
   799			goto out;
   800	
   801		if (x->encap) {
   802			const struct ipv6hdr *ip6h = ipv6_hdr(skb);
   803			int offset = skb_network_offset(skb) + sizeof(*ip6h);
   804			struct xfrm_encap_tmpl *encap = x->encap;
   805			u8 nexthdr = ip6h->nexthdr;
   806			__be16 frag_off, source;
   807			struct udphdr *uh;
   808			struct tcphdr *th;
   809	
   810			offset = ipv6_skip_exthdr(skb, offset, &nexthdr, &frag_off);
   811	
 > 812			if (offset < 0)
   813				err = -EINVAL;
   814				goto out;
   815	
   816			uh = (void *)(skb->data + offset);
   817			th = (void *)(skb->data + offset);
   818			hdr_len += offset;
   819	
   820			switch (x->encap->encap_type) {
   821			case TCP_ENCAP_ESPINTCP:
   822				source = th->source;
   823				break;
   824			case UDP_ENCAP_ESPINUDP:
   825			case UDP_ENCAP_ESPINUDP_NON_IKE:
   826				source = uh->source;
   827				break;
   828			default:
   829				WARN_ON_ONCE(1);
   830				err = -EINVAL;
   831				goto out;
   832			}
   833	
   834			/*
   835			 * 1) if the NAT-T peer's IP or port changed then
   836			 *    advertize the change to the keying daemon.
   837			 *    This is an inbound SA, so just compare
   838			 *    SRC ports.
   839			 */
   840			if (!ipv6_addr_equal(&ip6h->saddr, &x->props.saddr.in6) ||
   841			    source != encap->encap_sport) {
   842				xfrm_address_t ipaddr;
   843	
   844				memcpy(&ipaddr.a6, &ip6h->saddr.s6_addr, sizeof(ipaddr.a6));
   845				km_new_mapping(x, &ipaddr, source);
   846	
   847				/* XXX: perhaps add an extra
   848				 * policy check here, to see
   849				 * if we should allow or
   850				 * reject a packet from a
   851				 * different source
   852				 * address/port.
   853				 */
   854			}
   855	
   856			/*
   857			 * 2) ignore UDP/TCP checksums in case
   858			 *    of NAT-T in Transport Mode, or
   859			 *    perform other post-processing fixes
   860			 *    as per draft-ietf-ipsec-udp-encaps-06,
   861			 *    section 3.1.2
   862			 */
   863			if (x->props.mode == XFRM_MODE_TRANSPORT)
   864				skb->ip_summed = CHECKSUM_UNNECESSARY;
   865		}
   866	
   867		skb_postpull_rcsum(skb, skb_network_header(skb),
   868				   skb_network_header_len(skb));
   869		skb_pull_rcsum(skb, hlen);
   870		if (x->props.mode == XFRM_MODE_TUNNEL)
   871			skb_reset_transport_header(skb);
   872		else
   873			skb_set_transport_header(skb, -hdr_len);
   874	
   875		/* RFC4303: Drop dummy packets without any error */
   876		if (err == IPPROTO_NONE)
   877			err = -EINVAL;
   878	
   879	out:
   880		return err;
   881	}
   882	EXPORT_SYMBOL_GPL(esp6_input_done2);
   883	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 66457 bytes --]