pull request (net): ipsec 2022-03-16

pull request (net): ipsec 2022-03-16

[Steffen Klassert]

Two last fixes for this release cycle:

1) Fix a kernel-info-leak in pfkey.
   From Haimin Zhang.

2) Fix an incorrect check of the return value of ipv6_skip_exthdr.
   From Sabrina Dubroca.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit 5f147476057832b8f87461ff6da35b5d2e1c2c29:

  Merge branch 'selftests-pmtu-sh-fix-cleanup-of-processes-launched-in-subshell' (2022-03-09 20:23:38 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master

for you to fetch changes up to 4db4075f92af2b28f415fc979ab626e6b37d67b6:

  esp6: fix check on ipv6_skip_exthdr's return value (2022-03-14 11:42:27 +0100)

----------------------------------------------------------------
Haimin Zhang (1):
      af_key: add __GFP_ZERO flag for compose_sadb_supported in function pfkey_register

Sabrina Dubroca (1):
      esp6: fix check on ipv6_skip_exthdr's return value

 net/ipv6/esp6.c  | 3 +--
 net/key/af_key.c | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

[PATCH 1/2] af_key: add __GFP_ZERO flag for compose_sadb_supported in function pfkey_register

[Steffen Klassert]

From: Haimin Zhang <tcs_kernel@tencent.com>

Add __GFP_ZERO flag for compose_sadb_supported in function pfkey_register
to initialize the buffer of supp_skb to fix a kernel-info-leak issue.
1) Function pfkey_register calls compose_sadb_supported to request
a sk_buff. 2) compose_sadb_supported calls alloc_sbk to allocate
a sk_buff, but it doesn't zero it. 3) If auth_len is greater 0, then
compose_sadb_supported treats the memory as a struct sadb_supported and
begins to initialize. But it just initializes the field sadb_supported_len
and field sadb_supported_exttype without field sadb_supported_reserved.

Reported-by: TCS Robot <tcs_robot@tencent.com>
Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/key/af_key.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 9bf52a09b5ff..fd51db3be91c 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -1699,7 +1699,7 @@ static int pfkey_register(struct sock *sk, struct sk_buff *skb, const struct sad
 
 	xfrm_probe_algs();
 
-	supp_skb = compose_sadb_supported(hdr, GFP_KERNEL);
+	supp_skb = compose_sadb_supported(hdr, GFP_KERNEL | __GFP_ZERO);
 	if (!supp_skb) {
 		if (hdr->sadb_msg_satype != SADB_SATYPE_UNSPEC)
 			pfk->registered &= ~(1<<hdr->sadb_msg_satype);
-- 
2.25.1

[PATCH 2/2] esp6: fix check on ipv6_skip_exthdr's return value

[Steffen Klassert]

From: Sabrina Dubroca <sd@queasysnail.net>

Commit 5f9c55c8066b ("ipv6: check return value of ipv6_skip_exthdr")
introduced an incorrect check, which leads to all ESP packets over
either TCPv6 or UDPv6 encapsulation being dropped. In this particular
case, offset is negative, since skb->data points to the ESP header in
the following chain of headers, while skb->network_header points to
the IPv6 header:

    IPv6 | ext | ... | ext | UDP | ESP | ...

That doesn't seem to be a problem, especially considering that if we
reach esp6_input_done2, we're guaranteed to have a full set of headers
available (otherwise the packet would have been dropped earlier in the
stack). However, it means that the return value will (intentionally)
be negative. We can make the test more specific, as the expected
return value of ipv6_skip_exthdr will be the (negated) size of either
a UDP header, or a TCP header with possible options.

In the future, we should probably either make ipv6_skip_exthdr
explicitly accept negative offsets (and adjust its return value for
error cases), or make ipv6_skip_exthdr only take non-negative
offsets (and audit all callers).

Fixes: 5f9c55c8066b ("ipv6: check return value of ipv6_skip_exthdr")
Reported-by: Xiumei Mu <xmu@redhat.com>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv6/esp6.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index b0ffbcd5432d..55d604c9b3b3 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -812,8 +812,7 @@ 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) {
+		if (offset == -1) {
 			err = -EINVAL;
 			goto out;
 		}
-- 
2.25.1

Re: pull request (net): ipsec 2022-03-16

[Jakub Kicinski]

On Wed, 16 Mar 2022 13:11:40 +0100 Steffen Klassert wrote:
> Two last fixes for this release cycle:
> 
> 1) Fix a kernel-info-leak in pfkey.
>    From Haimin Zhang.
> 
> 2) Fix an incorrect check of the return value of ipv6_skip_exthdr.
>    From Sabrina Dubroca.

Excellent, thank you!

> Please pull or let me know if there are problems.

One minor improvement to appease patchwork would be to add / keep the
[PATCH 0/n] prefix on the PR / cover letter when posting the patches
under it. It seems that patchwork is hopeless in delineating the
patches and the PR if that's not there. For whatever reason it grouped
the PR and patch 2 as a series and patch 1 was left separate :S

Re: pull request (net): ipsec 2022-03-16

[patchwork-bot+netdevbpf]

Hello:

This pull request was applied to netdev/net.git (master)
by Steffen Klassert <steffen.klassert@secunet.com>:

On Wed, 16 Mar 2022 13:11:40 +0100 you wrote:
> Two last fixes for this release cycle:
> 
> 1) Fix a kernel-info-leak in pfkey.
>    From Haimin Zhang.
> 
> 2) Fix an incorrect check of the return value of ipv6_skip_exthdr.
>    From Sabrina Dubroca.
> 
> [...]

Here is the summary with links:
  - pull request (net): ipsec 2022-03-16
    https://git.kernel.org/netdev/net/c/186abea8a80b
  - [2/2] esp6: fix check on ipv6_skip_exthdr's return value
    https://git.kernel.org/netdev/net/c/4db4075f92af

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

Re: [PATCH 1/2] af_key: add __GFP_ZERO flag for compose_sadb_supported in function pfkey_register

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (master)
by Steffen Klassert <steffen.klassert@secunet.com>:

On Wed, 16 Mar 2022 13:11:41 +0100 you wrote:
> From: Haimin Zhang <tcs_kernel@tencent.com>
> 
> Add __GFP_ZERO flag for compose_sadb_supported in function pfkey_register
> to initialize the buffer of supp_skb to fix a kernel-info-leak issue.
> 1) Function pfkey_register calls compose_sadb_supported to request
> a sk_buff. 2) compose_sadb_supported calls alloc_sbk to allocate
> a sk_buff, but it doesn't zero it. 3) If auth_len is greater 0, then
> compose_sadb_supported treats the memory as a struct sadb_supported and
> begins to initialize. But it just initializes the field sadb_supported_len
> and field sadb_supported_exttype without field sadb_supported_reserved.
> 
> [...]

Here is the summary with links:
  - [1/2] af_key: add __GFP_ZERO flag for compose_sadb_supported in function pfkey_register
    https://git.kernel.org/netdev/net/c/9a564bccb78a

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

Re: pull request (net): ipsec 2022-03-16

[Steffen Klassert]

On Wed, Mar 16, 2022 at 11:44:38AM -0700, Jakub Kicinski wrote:
> On Wed, 16 Mar 2022 13:11:40 +0100 Steffen Klassert wrote:
> > Two last fixes for this release cycle:
> > 
> > 1) Fix a kernel-info-leak in pfkey.
> >    From Haimin Zhang.
> > 
> > 2) Fix an incorrect check of the return value of ipv6_skip_exthdr.
> >    From Sabrina Dubroca.
> 
> Excellent, thank you!
> 
> > Please pull or let me know if there are problems.
> 
> One minor improvement to appease patchwork would be to add / keep the
> [PATCH 0/n] prefix on the PR / cover letter when posting the patches
> under it.

I did that in the ipsec-next pull request, let me know if this is
OK as I did it.

> It seems that patchwork is hopeless in delineating the
> patches and the PR if that's not there. For whatever reason it grouped
> the PR and patch 2 as a series and patch 1 was left separate :S

I guess this is why I get always two mails from patchwork-bot for
each pull request. I already wondered why that happens :)

Re: pull request (net): ipsec 2022-03-16

[Jakub Kicinski]

On Sat, 19 Mar 2022 08:49:11 +0100 Steffen Klassert wrote:
> On Wed, Mar 16, 2022 at 11:44:38AM -0700, Jakub Kicinski wrote:
> > One minor improvement to appease patchwork would be to add / keep the
> > [PATCH 0/n] prefix on the PR / cover letter when posting the patches
> > under it.  
> 
> I did that in the ipsec-next pull request, let me know if this is
> OK as I did it.

Yes, that one worked out perfectly. Thanks!

> > It seems that patchwork is hopeless in delineating the
> > patches and the PR if that's not there. For whatever reason it grouped
> > the PR and patch 2 as a series and patch 1 was left separate :S  
> 
> I guess this is why I get always two mails from patchwork-bot for
> each pull request. I already wondered why that happens :)

To be honest the pr handling in the patchwork-bot is not 100% accurate,
I wish it was responding to the pr / cover letter.  We'll get there :)