All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ipv6: check return value of ipv6_skip_exthdr
@ 2021-11-17 18:16 Jordy Zomer
  2021-11-17 18:44 ` Kees Cook
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Jordy Zomer @ 2021-11-17 18:16 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-hardening, Jordy Zomer, Steffen Klassert, Herbert Xu,
	David S. Miller, Hideaki YOSHIFUJI, David Ahern, Jakub Kicinski,
	netdev

The offset value is used in pointer math on skb->data.
Since ipv6_skip_exthdr may return -1 the pointer to uh and th
may not point to the actual udp and tcp headers and potentially
overwrite other stuff. This is why I think this should be checked.

Signed-off-by: Jordy Zomer <jordy@pwning.systems>
---
 net/ipv6/esp6.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index ed2f061b8768..dc4251655df9 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -808,6 +808,11 @@ int esp6_input_done2(struct sk_buff *skb, int err)
 		struct tcphdr *th;
 
 		offset = ipv6_skip_exthdr(skb, offset, &nexthdr, &frag_off);
+
+		if (offset < 0)
+			err = -EINVAL;
+			goto out;
+
 		uh = (void *)(skb->data + offset);
 		th = (void *)(skb->data + offset);
 		hdr_len += offset;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] ipv6: check return value of ipv6_skip_exthdr
  2021-11-17 18:16 [PATCH] ipv6: check return value of ipv6_skip_exthdr Jordy Zomer
@ 2021-11-17 18:44 ` Kees Cook
  2021-11-17 18:46 ` Kees Cook
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2021-11-17 18:44 UTC (permalink / raw)
  To: Jordy Zomer
  Cc: linux-kernel, linux-hardening, Steffen Klassert, Herbert Xu,
	David S. Miller, Hideaki YOSHIFUJI, David Ahern, Jakub Kicinski,
	netdev

On Wed, Nov 17, 2021 at 07:16:10PM +0100, Jordy Zomer wrote:
> The offset value is used in pointer math on skb->data.
> Since ipv6_skip_exthdr may return -1 the pointer to uh and th
> may not point to the actual udp and tcp headers and potentially
> overwrite other stuff. This is why I think this should be checked.
> 
> Signed-off-by: Jordy Zomer <jordy@pwning.systems>
> ---
>  net/ipv6/esp6.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
> index ed2f061b8768..dc4251655df9 100644
> --- a/net/ipv6/esp6.c
> +++ b/net/ipv6/esp6.c
> @@ -808,6 +808,11 @@ int esp6_input_done2(struct sk_buff *skb, int err)
>  		struct tcphdr *th;
>  
>  		offset = ipv6_skip_exthdr(skb, offset, &nexthdr, &frag_off);
> +
> +		if (offset < 0)
> +			err = -EINVAL;
> +			goto out;
> +

Ew. Yeah, it seems like ipv6_skip_exthdr() needs to be checked in a lot
of places. If this is part of protocol decoding, I'm surprised fuzzers
haven't found this? Is this state reachable?

I assume so, as there have been similar fixes in the past:
https://git.kernel.org/linus/92c6058024e87087cf1b99b0389d67c0a886360e

>  		uh = (void *)(skb->data + offset);
>  		th = (void *)(skb->data + offset);
>  		hdr_len += offset;
> -- 
> 2.27.0
> 

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ipv6: check return value of ipv6_skip_exthdr
  2021-11-17 18:16 [PATCH] ipv6: check return value of ipv6_skip_exthdr Jordy Zomer
  2021-11-17 18:44 ` Kees Cook
@ 2021-11-17 18:46 ` Kees Cook
  2021-11-17 19:06 ` [PATCH v2] " Jordy Zomer
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2021-11-17 18:46 UTC (permalink / raw)
  To: Jordy Zomer
  Cc: linux-kernel, linux-hardening, Steffen Klassert, Herbert Xu,
	David S. Miller, Hideaki YOSHIFUJI, David Ahern, Jakub Kicinski,
	netdev

On Wed, Nov 17, 2021 at 07:16:10PM +0100, Jordy Zomer wrote:
> The offset value is used in pointer math on skb->data.
> Since ipv6_skip_exthdr may return -1 the pointer to uh and th
> may not point to the actual udp and tcp headers and potentially
> overwrite other stuff. This is why I think this should be checked.
> 
> Signed-off-by: Jordy Zomer <jordy@pwning.systems>
> ---
>  net/ipv6/esp6.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
> index ed2f061b8768..dc4251655df9 100644
> --- a/net/ipv6/esp6.c
> +++ b/net/ipv6/esp6.c
> @@ -808,6 +808,11 @@ int esp6_input_done2(struct sk_buff *skb, int err)
>  		struct tcphdr *th;
>  
>  		offset = ipv6_skip_exthdr(skb, offset, &nexthdr, &frag_off);
> +
> +		if (offset < 0)
> +			err = -EINVAL;
> +			goto out;
> +

Oh, and, this needs { }s. The CI noticed too:
https://patchwork.kernel.org/project/netdevbpf/patch/20211117181610.2731938-1-jordy@pwning.systems/

>  		uh = (void *)(skb->data + offset);
>  		th = (void *)(skb->data + offset);
>  		hdr_len += offset;
> -- 
> 2.27.0
> 

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] ipv6: check return value of ipv6_skip_exthdr
  2021-11-17 18:16 [PATCH] ipv6: check return value of ipv6_skip_exthdr Jordy Zomer
  2021-11-17 18:44 ` Kees Cook
  2021-11-17 18:46 ` Kees Cook
@ 2021-11-17 19:06 ` Jordy Zomer
  2021-11-18 11:50   ` patchwork-bot+netdevbpf
  2021-11-19 22:36 ` [PATCH] " kernel test robot
  2021-11-20 18:00   ` kernel test robot
  4 siblings, 1 reply; 8+ messages in thread
From: Jordy Zomer @ 2021-11-17 19:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kees Cook, Jordy Zomer, Steffen Klassert, Herbert Xu,
	David S. Miller, Hideaki YOSHIFUJI, David Ahern, Jakub Kicinski,
	netdev

The offset value is used in pointer math on skb->data.
Since ipv6_skip_exthdr may return -1 the pointer to uh and th
may not point to the actual udp and tcp headers and potentially
overwrite other stuff. This is why I think this should be checked.

EDIT:  added {}'s, thanks Kees

Signed-off-by: Jordy Zomer <jordy@pwning.systems>
---
 net/ipv6/esp6.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index ed2f061b8768..f0bac6f7ab6b 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -808,6 +808,12 @@ int esp6_input_done2(struct sk_buff *skb, int err)
 		struct tcphdr *th;
 
 		offset = ipv6_skip_exthdr(skb, offset, &nexthdr, &frag_off);
+
+		if (offset < 0) {
+			err = -EINVAL;
+			goto out;
+		}
+
 		uh = (void *)(skb->data + offset);
 		th = (void *)(skb->data + offset);
 		hdr_len += offset;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] ipv6: check return value of ipv6_skip_exthdr
  2021-11-17 19:06 ` [PATCH v2] " Jordy Zomer
@ 2021-11-18 11:50   ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-11-18 11:50 UTC (permalink / raw)
  To: Jordy Zomer
  Cc: linux-kernel, keescook, steffen.klassert, herbert, davem,
	yoshfuji, dsahern, kuba, netdev

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Wed, 17 Nov 2021 20:06:48 +0100 you wrote:
> The offset value is used in pointer math on skb->data.
> Since ipv6_skip_exthdr may return -1 the pointer to uh and th
> may not point to the actual udp and tcp headers and potentially
> overwrite other stuff. This is why I think this should be checked.
> 
> EDIT:  added {}'s, thanks Kees
> 
> [...]

Here is the summary with links:
  - [v2] ipv6: check return value of ipv6_skip_exthdr
    https://git.kernel.org/netdev/net/c/5f9c55c8066b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ipv6: check return value of ipv6_skip_exthdr
  2021-11-17 18:16 [PATCH] ipv6: check return value of ipv6_skip_exthdr Jordy Zomer
                   ` (2 preceding siblings ...)
  2021-11-17 19:06 ` [PATCH v2] " Jordy Zomer
@ 2021-11-19 22:36 ` kernel test robot
  2021-11-20 18:00   ` kernel test robot
  4 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2021-11-19 22:36 UTC (permalink / raw)
  To: kbuild-all

[-- 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 --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ipv6: check return value of ipv6_skip_exthdr
  2021-11-17 18:16 [PATCH] ipv6: check return value of ipv6_skip_exthdr Jordy Zomer
@ 2021-11-20 18:00   ` kernel test robot
  2021-11-17 18:46 ` Kees Cook
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2021-11-20 18:00 UTC (permalink / raw)
  To: Jordy Zomer; +Cc: llvm, kbuild-all

[-- Attachment #1: Type: text/plain, Size: 5396 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: arm64-buildonly-randconfig-r004-20211118 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c46becf500df2a7fb4b4fce16178a036c344315a)
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 arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # 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
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm64 

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:814:4: warning: misleading indentation; statement is not part of the previous 'if' [-Wmisleading-indentation]
                           goto out;
                           ^
   net/ipv6/esp6.c:812:3: note: previous statement is here
                   if (offset < 0)
                   ^
   1 warning generated.


vim +/if +814 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@lists.01.org

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ipv6: check return value of ipv6_skip_exthdr
@ 2021-11-20 18:00   ` kernel test robot
  0 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2021-11-20 18:00 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 5547 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: arm64-buildonly-randconfig-r004-20211118 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c46becf500df2a7fb4b4fce16178a036c344315a)
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 arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # 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
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm64 

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:814:4: warning: misleading indentation; statement is not part of the previous 'if' [-Wmisleading-indentation]
                           goto out;
                           ^
   net/ipv6/esp6.c:812:3: note: previous statement is here
                   if (offset < 0)
                   ^
   1 warning generated.


vim +/if +814 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: 44884 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-11-20 18:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-17 18:16 [PATCH] ipv6: check return value of ipv6_skip_exthdr Jordy Zomer
2021-11-17 18:44 ` Kees Cook
2021-11-17 18:46 ` Kees Cook
2021-11-17 19:06 ` [PATCH v2] " Jordy Zomer
2021-11-18 11:50   ` patchwork-bot+netdevbpf
2021-11-19 22:36 ` [PATCH] " kernel test robot
2021-11-20 18:00 ` kernel test robot
2021-11-20 18:00   ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.