All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] pull request (net): ipsec 2022-11-23
@ 2022-11-23  9:31 Steffen Klassert
  2022-11-23  9:31 ` [PATCH 1/6] xfrm: fix "disable_policy" on ipv4 early demux Steffen Klassert
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Steffen Klassert @ 2022-11-23  9:31 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev

1) Fix "disable_policy" on ipv4 early demuxP Packets after
   the initial packet in a flow might be incorectly dropped
   on early demux if there are no matching policies.
   From Eyal Birger.

2) Fix a kernel warning in case XFRM encap type is not
   available. From Eyal Birger.

3) Fix ESN wrap around for GSO to avoid a double usage of a
    sequence number. From Christian Langrock.

4) Fix a send_acquire race with pfkey_register.
   From Herbert Xu.

5) Fix a list corruption panic in __xfrm_state_delete().
   Thomas Jarosch.

6) Fix an unchecked return value in xfrm6_init().
   Chen Zhongjin.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit 1d22f78d05737ce21bff7b88b6e58873f35e65ba:

  Merge tag 'ieee802154-for-net-2022-10-05' of git://git.kernel.org/pub/scm/linux/kernel/git/sschmidt/wpan (2022-10-05 20:38:46 -0700)

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 40781bfb836eda57d19c0baa37c7e72590e05fdc:

  xfrm: Fix ignored return value in xfrm6_init() (2022-11-22 07:16:34 +0100)

----------------------------------------------------------------
Chen Zhongjin (1):
      xfrm: Fix ignored return value in xfrm6_init()

Christian Langrock (1):
      xfrm: replay: Fix ESN wrap around for GSO

Eyal Birger (2):
      xfrm: fix "disable_policy" on ipv4 early demux
      xfrm: lwtunnel: squelch kernel warning in case XFRM encap type is not available

Herbert Xu (1):
      af_key: Fix send_acquire race with pfkey_register

Thomas Jarosch (1):
      xfrm: Fix oops in __xfrm_state_delete()

 net/core/lwtunnel.c     |  4 +++-
 net/ipv4/esp4_offload.c |  3 +++
 net/ipv4/ip_input.c     |  5 +++++
 net/ipv6/esp6_offload.c |  3 +++
 net/ipv6/xfrm6_policy.c |  6 +++++-
 net/key/af_key.c        | 34 +++++++++++++++++++++++-----------
 net/xfrm/xfrm_device.c  | 15 ++++++++++++++-
 net/xfrm/xfrm_replay.c  |  2 +-
 8 files changed, 57 insertions(+), 15 deletions(-)

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

* [PATCH 1/6] xfrm: fix "disable_policy" on ipv4 early demux
  2022-11-23  9:31 [PATCH 0/6] pull request (net): ipsec 2022-11-23 Steffen Klassert
@ 2022-11-23  9:31 ` Steffen Klassert
  2022-11-24  3:30   ` patchwork-bot+netdevbpf
  2022-11-23  9:31 ` [PATCH 2/6] xfrm: lwtunnel: squelch kernel warning in case XFRM encap type is not available Steffen Klassert
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 9+ messages in thread
From: Steffen Klassert @ 2022-11-23  9:31 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Eyal Birger <eyal.birger@gmail.com>

The commit in the "Fixes" tag tried to avoid a case where policy check
is ignored due to dst caching in next hops.

However, when the traffic is locally consumed, the dst may be cached
in a local TCP or UDP socket as part of early demux. In this case the
"disable_policy" flag is not checked as ip_route_input_noref() was only
called before caching, and thus, packets after the initial packet in a
flow will be dropped if not matching policies.

Fix by checking the "disable_policy" flag also when a valid dst is
already available.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=216557
Reported-by: Monil Patel <monil191989@gmail.com>
Fixes: e6175a2ed1f1 ("xfrm: fix "disable_policy" flag use when arriving from different devices")
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>

----

v2: use dev instead of skb->dev
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/ip_input.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 1b512390b3cf..e880ce77322a 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -366,6 +366,11 @@ static int ip_rcv_finish_core(struct net *net, struct sock *sk,
 					   iph->tos, dev);
 		if (unlikely(err))
 			goto drop_error;
+	} else {
+		struct in_device *in_dev = __in_dev_get_rcu(dev);
+
+		if (in_dev && IN_DEV_ORCONF(in_dev, NOPOLICY))
+			IPCB(skb)->flags |= IPSKB_NOPOLICY;
 	}
 
 #ifdef CONFIG_IP_ROUTE_CLASSID
-- 
2.25.1


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

* [PATCH 2/6] xfrm: lwtunnel: squelch kernel warning in case XFRM encap type is not available
  2022-11-23  9:31 [PATCH 0/6] pull request (net): ipsec 2022-11-23 Steffen Klassert
  2022-11-23  9:31 ` [PATCH 1/6] xfrm: fix "disable_policy" on ipv4 early demux Steffen Klassert
@ 2022-11-23  9:31 ` Steffen Klassert
  2022-11-23  9:31 ` [PATCH 3/6] xfrm: replay: Fix ESN wrap around for GSO Steffen Klassert
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2022-11-23  9:31 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Eyal Birger <eyal.birger@gmail.com>

Ido reported that a kernel warning [1] can be triggered from
user space when the kernel is compiled with CONFIG_MODULES=y and
CONFIG_XFRM=n when adding an xfrm encap type route, e.g:

$ ip route add 198.51.100.0/24 dev dummy1 encap xfrm if_id 1
Error: lwt encapsulation type not supported.

The reason for the warning is that the LWT infrastructure has an
autoloading feature which is meant only for encap types that don't
use a net device,  which is not the case in xfrm encap.

Mute this warning for xfrm encap as there's no encap module to autoload
in this case.

[1]
 WARNING: CPU: 3 PID: 2746262 at net/core/lwtunnel.c:57 lwtunnel_valid_encap_type+0x4f/0x120
[...]
 Call Trace:
  <TASK>
  rtm_to_fib_config+0x211/0x350
  inet_rtm_newroute+0x3a/0xa0
  rtnetlink_rcv_msg+0x154/0x3c0
  netlink_rcv_skb+0x49/0xf0
  netlink_unicast+0x22f/0x350
  netlink_sendmsg+0x208/0x440
  ____sys_sendmsg+0x21f/0x250
  ___sys_sendmsg+0x83/0xd0
  __sys_sendmsg+0x54/0xa0
  do_syscall_64+0x35/0x80
  entry_SYSCALL_64_after_hwframe+0x63/0xcd

Reported-by: Ido Schimmel <idosch@idosch.org>
Fixes: 2c2493b9da91 ("xfrm: lwtunnel: add lwtunnel support for xfrm interfaces in collect_md mode")
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
Tested-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/core/lwtunnel.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
index 6fac2f0ef074..711cd3b4347a 100644
--- a/net/core/lwtunnel.c
+++ b/net/core/lwtunnel.c
@@ -48,9 +48,11 @@ static const char *lwtunnel_encap_str(enum lwtunnel_encap_types encap_type)
 		return "RPL";
 	case LWTUNNEL_ENCAP_IOAM6:
 		return "IOAM6";
+	case LWTUNNEL_ENCAP_XFRM:
+		/* module autoload not supported for encap type */
+		return NULL;
 	case LWTUNNEL_ENCAP_IP6:
 	case LWTUNNEL_ENCAP_IP:
-	case LWTUNNEL_ENCAP_XFRM:
 	case LWTUNNEL_ENCAP_NONE:
 	case __LWTUNNEL_ENCAP_MAX:
 		/* should not have got here */
-- 
2.25.1


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

* [PATCH 3/6] xfrm: replay: Fix ESN wrap around for GSO
  2022-11-23  9:31 [PATCH 0/6] pull request (net): ipsec 2022-11-23 Steffen Klassert
  2022-11-23  9:31 ` [PATCH 1/6] xfrm: fix "disable_policy" on ipv4 early demux Steffen Klassert
  2022-11-23  9:31 ` [PATCH 2/6] xfrm: lwtunnel: squelch kernel warning in case XFRM encap type is not available Steffen Klassert
@ 2022-11-23  9:31 ` Steffen Klassert
  2022-11-23  9:31 ` [PATCH 4/6] af_key: Fix send_acquire race with pfkey_register Steffen Klassert
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2022-11-23  9:31 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Christian Langrock <christian.langrock@secunet.com>

When using GSO it can happen that the wrong seq_hi is used for the last
packets before the wrap around. This can lead to double usage of a
sequence number. To avoid this, we should serialize this last GSO
packet.

Fixes: d7dbefc45cf5 ("xfrm: Add xfrm_replay_overflow functions for offloading")
Co-developed-by: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Christian Langrock <christian.langrock@secunet.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/esp4_offload.c |  3 +++
 net/ipv6/esp6_offload.c |  3 +++
 net/xfrm/xfrm_device.c  | 15 ++++++++++++++-
 net/xfrm/xfrm_replay.c  |  2 +-
 4 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index 170152772d33..3969fa805679 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -314,6 +314,9 @@ static int esp_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features_
 			xo->seq.low += skb_shinfo(skb)->gso_segs;
 	}
 
+	if (xo->seq.low < seq)
+		xo->seq.hi++;
+
 	esp.seqno = cpu_to_be64(seq + ((u64)xo->seq.hi << 32));
 
 	ip_hdr(skb)->tot_len = htons(skb->len);
diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index 79d43548279c..242f4295940e 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -346,6 +346,9 @@ static int esp6_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features
 			xo->seq.low += skb_shinfo(skb)->gso_segs;
 	}
 
+	if (xo->seq.low < seq)
+		xo->seq.hi++;
+
 	esp.seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));
 
 	len = skb->len - sizeof(struct ipv6hdr);
diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index 5f5aafd418af..21269e8f2db4 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -97,6 +97,18 @@ static void xfrm_outer_mode_prep(struct xfrm_state *x, struct sk_buff *skb)
 	}
 }
 
+static inline bool xmit_xfrm_check_overflow(struct sk_buff *skb)
+{
+	struct xfrm_offload *xo = xfrm_offload(skb);
+	__u32 seq = xo->seq.low;
+
+	seq += skb_shinfo(skb)->gso_segs;
+	if (unlikely(seq < xo->seq.low))
+		return true;
+
+	return false;
+}
+
 struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t features, bool *again)
 {
 	int err;
@@ -134,7 +146,8 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur
 		return skb;
 	}
 
-	if (skb_is_gso(skb) && unlikely(x->xso.dev != dev)) {
+	if (skb_is_gso(skb) && (unlikely(x->xso.dev != dev) ||
+				unlikely(xmit_xfrm_check_overflow(skb)))) {
 		struct sk_buff *segs;
 
 		/* Packet got rerouted, fixup features and segment it. */
diff --git a/net/xfrm/xfrm_replay.c b/net/xfrm/xfrm_replay.c
index 9f4d42eb090f..ce56d659c55a 100644
--- a/net/xfrm/xfrm_replay.c
+++ b/net/xfrm/xfrm_replay.c
@@ -714,7 +714,7 @@ static int xfrm_replay_overflow_offload_esn(struct xfrm_state *x, struct sk_buff
 			oseq += skb_shinfo(skb)->gso_segs;
 		}
 
-		if (unlikely(oseq < replay_esn->oseq)) {
+		if (unlikely(xo->seq.low < replay_esn->oseq)) {
 			XFRM_SKB_CB(skb)->seq.output.hi = ++oseq_hi;
 			xo->seq.hi = oseq_hi;
 			replay_esn->oseq_hi = oseq_hi;
-- 
2.25.1


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

* [PATCH 4/6] af_key: Fix send_acquire race with pfkey_register
  2022-11-23  9:31 [PATCH 0/6] pull request (net): ipsec 2022-11-23 Steffen Klassert
                   ` (2 preceding siblings ...)
  2022-11-23  9:31 ` [PATCH 3/6] xfrm: replay: Fix ESN wrap around for GSO Steffen Klassert
@ 2022-11-23  9:31 ` Steffen Klassert
  2022-11-23  9:31 ` [PATCH 5/6] xfrm: Fix oops in __xfrm_state_delete() Steffen Klassert
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2022-11-23  9:31 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Herbert Xu <herbert@gondor.apana.org.au>

The function pfkey_send_acquire may race with pfkey_register
(which could even be in a different name space).  This may result
in a buffer overrun.

Allocating the maximum amount of memory that could be used prevents
this.

Reported-by: syzbot+1e9af9185d8850e2c2fa@syzkaller.appspotmail.com
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/key/af_key.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index c85df5b958d2..213287814328 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2905,7 +2905,7 @@ static int count_ah_combs(const struct xfrm_tmpl *t)
 			break;
 		if (!aalg->pfkey_supported)
 			continue;
-		if (aalg_tmpl_set(t, aalg) && aalg->available)
+		if (aalg_tmpl_set(t, aalg))
 			sz += sizeof(struct sadb_comb);
 	}
 	return sz + sizeof(struct sadb_prop);
@@ -2923,7 +2923,7 @@ static int count_esp_combs(const struct xfrm_tmpl *t)
 		if (!ealg->pfkey_supported)
 			continue;
 
-		if (!(ealg_tmpl_set(t, ealg) && ealg->available))
+		if (!(ealg_tmpl_set(t, ealg)))
 			continue;
 
 		for (k = 1; ; k++) {
@@ -2934,16 +2934,17 @@ static int count_esp_combs(const struct xfrm_tmpl *t)
 			if (!aalg->pfkey_supported)
 				continue;
 
-			if (aalg_tmpl_set(t, aalg) && aalg->available)
+			if (aalg_tmpl_set(t, aalg))
 				sz += sizeof(struct sadb_comb);
 		}
 	}
 	return sz + sizeof(struct sadb_prop);
 }
 
-static void dump_ah_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
+static int dump_ah_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
 {
 	struct sadb_prop *p;
+	int sz = 0;
 	int i;
 
 	p = skb_put(skb, sizeof(struct sadb_prop));
@@ -2971,13 +2972,17 @@ static void dump_ah_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
 			c->sadb_comb_soft_addtime = 20*60*60;
 			c->sadb_comb_hard_usetime = 8*60*60;
 			c->sadb_comb_soft_usetime = 7*60*60;
+			sz += sizeof(*c);
 		}
 	}
+
+	return sz + sizeof(*p);
 }
 
-static void dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
+static int dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
 {
 	struct sadb_prop *p;
+	int sz = 0;
 	int i, k;
 
 	p = skb_put(skb, sizeof(struct sadb_prop));
@@ -3019,8 +3024,11 @@ static void dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
 			c->sadb_comb_soft_addtime = 20*60*60;
 			c->sadb_comb_hard_usetime = 8*60*60;
 			c->sadb_comb_soft_usetime = 7*60*60;
+			sz += sizeof(*c);
 		}
 	}
+
+	return sz + sizeof(*p);
 }
 
 static int key_notify_policy_expire(struct xfrm_policy *xp, const struct km_event *c)
@@ -3150,6 +3158,7 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
 	struct sadb_x_sec_ctx *sec_ctx;
 	struct xfrm_sec_ctx *xfrm_ctx;
 	int ctx_size = 0;
+	int alg_size = 0;
 
 	sockaddr_size = pfkey_sockaddr_size(x->props.family);
 	if (!sockaddr_size)
@@ -3161,16 +3170,16 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
 		sizeof(struct sadb_x_policy);
 
 	if (x->id.proto == IPPROTO_AH)
-		size += count_ah_combs(t);
+		alg_size = count_ah_combs(t);
 	else if (x->id.proto == IPPROTO_ESP)
-		size += count_esp_combs(t);
+		alg_size = count_esp_combs(t);
 
 	if ((xfrm_ctx = x->security)) {
 		ctx_size = PFKEY_ALIGN8(xfrm_ctx->ctx_len);
 		size +=  sizeof(struct sadb_x_sec_ctx) + ctx_size;
 	}
 
-	skb =  alloc_skb(size + 16, GFP_ATOMIC);
+	skb =  alloc_skb(size + alg_size + 16, GFP_ATOMIC);
 	if (skb == NULL)
 		return -ENOMEM;
 
@@ -3224,10 +3233,13 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
 	pol->sadb_x_policy_priority = xp->priority;
 
 	/* Set sadb_comb's. */
+	alg_size = 0;
 	if (x->id.proto == IPPROTO_AH)
-		dump_ah_combs(skb, t);
+		alg_size = dump_ah_combs(skb, t);
 	else if (x->id.proto == IPPROTO_ESP)
-		dump_esp_combs(skb, t);
+		alg_size = dump_esp_combs(skb, t);
+
+	hdr->sadb_msg_len += alg_size / 8;
 
 	/* security context */
 	if (xfrm_ctx) {
-- 
2.25.1


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

* [PATCH 5/6] xfrm: Fix oops in __xfrm_state_delete()
  2022-11-23  9:31 [PATCH 0/6] pull request (net): ipsec 2022-11-23 Steffen Klassert
                   ` (3 preceding siblings ...)
  2022-11-23  9:31 ` [PATCH 4/6] af_key: Fix send_acquire race with pfkey_register Steffen Klassert
@ 2022-11-23  9:31 ` Steffen Klassert
  2022-11-23  9:31 ` [PATCH 6/6] xfrm: Fix ignored return value in xfrm6_init() Steffen Klassert
  2022-11-24 17:38 ` [PATCH 0/6] pull request (net): ipsec 2022-11-23 Alexander Lobakin
  6 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2022-11-23  9:31 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Thomas Jarosch <thomas.jarosch@intra2net.com>

Kernel 5.14 added a new "byseq" index to speed
up xfrm_state lookups by sequence number in commit
fe9f1d8779cb ("xfrm: add state hashtable keyed by seq")

While the patch was thorough, the function pfkey_send_new_mapping()
in net/af_key.c also modifies x->km.seq and never added
the current xfrm_state to the "byseq" index.

This leads to the following kernel Ooops:
    BUG: kernel NULL pointer dereference, address: 0000000000000000
    ..
    RIP: 0010:__xfrm_state_delete+0xc9/0x1c0
    ..
    Call Trace:
    <TASK>
    xfrm_state_delete+0x1e/0x40
    xfrm_del_sa+0xb0/0x110 [xfrm_user]
    xfrm_user_rcv_msg+0x12d/0x270 [xfrm_user]
    ? remove_entity_load_avg+0x8a/0xa0
    ? copy_to_user_state_extra+0x580/0x580 [xfrm_user]
    netlink_rcv_skb+0x51/0x100
    xfrm_netlink_rcv+0x30/0x50 [xfrm_user]
    netlink_unicast+0x1a6/0x270
    netlink_sendmsg+0x22a/0x480
    __sys_sendto+0x1a6/0x1c0
    ? __audit_syscall_entry+0xd8/0x130
    ? __audit_syscall_exit+0x249/0x2b0
    __x64_sys_sendto+0x23/0x30
    do_syscall_64+0x3a/0x90
    entry_SYSCALL_64_after_hwframe+0x61/0xcb

Exact location of the crash in __xfrm_state_delete():
    if (x->km.seq)
        hlist_del_rcu(&x->byseq);

The hlist_node "byseq" was never populated.

The bug only triggers if a new NAT traversal mapping (changed IP or port)
is detected in esp_input_done2() / esp6_input_done2(), which in turn
indirectly calls pfkey_send_new_mapping() *if* the kernel is compiled
with CONFIG_NET_KEY and "af_key" is active.

The PF_KEYv2 message SADB_X_NAT_T_NEW_MAPPING is not part of RFC 2367.
Various implementations have been examined how they handle
the "sadb_msg_seq" header field:

- racoon (Android): does not process SADB_X_NAT_T_NEW_MAPPING
- strongswan: does not care about sadb_msg_seq
- openswan: does not care about sadb_msg_seq

There is no standard how PF_KEYv2 sadb_msg_seq should be populated
for SADB_X_NAT_T_NEW_MAPPING and it's not used in popular
implementations either. Herbert Xu suggested we should just
use the current km.seq value as is. This fixes the root cause
of the oops since we no longer modify km.seq itself.

The update of "km.seq" looks like a copy'n'paste error
from pfkey_send_acquire(). SADB_ACQUIRE must indeed assign a unique km.seq
number according to RFC 2367. It has been verified that code paths
involving pfkey_send_acquire() don't cause the same Oops.

PF_KEYv2 SADB_X_NAT_T_NEW_MAPPING support was originally added here:
    https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git

    commit cbc3488685b20e7b2a98ad387a1a816aada569d8
    Author:     Derek Atkins <derek@ihtfp.com>
    AuthorDate: Wed Apr 2 13:21:02 2003 -0800

        [IPSEC]: Implement UDP Encapsulation framework.

        In particular, implement ESPinUDP encapsulation for IPsec
        Nat Traversal.

A note on triggering the bug: I was not able to trigger it using VMs.
There is one VPN using a high latency link on our production VPN server
that triggered it like once a day though.

Link: https://github.com/strongswan/strongswan/issues/992
Link: https://lore.kernel.org/netdev/00959f33ee52c4b3b0084d42c430418e502db554.1652340703.git.antony.antony@secunet.com/T/
Link: https://lore.kernel.org/netdev/20221027142455.3975224-1-chenzhihao@meizu.com/T/

Fixes: fe9f1d8779cb ("xfrm: add state hashtable keyed by seq")
Reported-by: Roth Mark <rothm@mail.com>
Reported-by: Zhihao Chen <chenzhihao@meizu.com>
Tested-by: Roth Mark <rothm@mail.com>
Signed-off-by: Thomas Jarosch <thomas.jarosch@intra2net.com>
Acked-by: Antony Antony <antony.antony@secunet.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
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 213287814328..95edcbedf6ef 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -3394,7 +3394,7 @@ static int pfkey_send_new_mapping(struct xfrm_state *x, xfrm_address_t *ipaddr,
 	hdr->sadb_msg_len = size / sizeof(uint64_t);
 	hdr->sadb_msg_errno = 0;
 	hdr->sadb_msg_reserved = 0;
-	hdr->sadb_msg_seq = x->km.seq = get_acqseq();
+	hdr->sadb_msg_seq = x->km.seq;
 	hdr->sadb_msg_pid = 0;
 
 	/* SA */
-- 
2.25.1


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

* [PATCH 6/6] xfrm: Fix ignored return value in xfrm6_init()
  2022-11-23  9:31 [PATCH 0/6] pull request (net): ipsec 2022-11-23 Steffen Klassert
                   ` (4 preceding siblings ...)
  2022-11-23  9:31 ` [PATCH 5/6] xfrm: Fix oops in __xfrm_state_delete() Steffen Klassert
@ 2022-11-23  9:31 ` Steffen Klassert
  2022-11-24 17:38 ` [PATCH 0/6] pull request (net): ipsec 2022-11-23 Alexander Lobakin
  6 siblings, 0 replies; 9+ messages in thread
From: Steffen Klassert @ 2022-11-23  9:31 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev

From: Chen Zhongjin <chenzhongjin@huawei.com>

When IPv6 module initializing in xfrm6_init(), register_pernet_subsys()
is possible to fail but its return value is ignored.

If IPv6 initialization fails later and xfrm6_fini() is called,
removing uninitialized list in xfrm6_net_ops will cause null-ptr-deref:

KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
CPU: 1 PID: 330 Comm: insmod
RIP: 0010:unregister_pernet_operations+0xc9/0x450
Call Trace:
 <TASK>
 unregister_pernet_subsys+0x31/0x3e
 xfrm6_fini+0x16/0x30 [ipv6]
 ip6_route_init+0xcd/0x128 [ipv6]
 inet6_init+0x29c/0x602 [ipv6]
 ...

Fix it by catching the error return value of register_pernet_subsys().

Fixes: 8d068875caca ("xfrm: make gc_thresh configurable in all namespaces")
Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv6/xfrm6_policy.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 4a4b0e49ec92..ea435eba3053 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -287,9 +287,13 @@ int __init xfrm6_init(void)
 	if (ret)
 		goto out_state;
 
-	register_pernet_subsys(&xfrm6_net_ops);
+	ret = register_pernet_subsys(&xfrm6_net_ops);
+	if (ret)
+		goto out_protocol;
 out:
 	return ret;
+out_protocol:
+	xfrm6_protocol_fini();
 out_state:
 	xfrm6_state_fini();
 out_policy:
-- 
2.25.1


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

* Re: [PATCH 1/6] xfrm: fix "disable_policy" on ipv4 early demux
  2022-11-23  9:31 ` [PATCH 1/6] xfrm: fix "disable_policy" on ipv4 early demux Steffen Klassert
@ 2022-11-24  3:30   ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-11-24  3:30 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: davem, kuba, herbert, netdev

Hello:

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

On Wed, 23 Nov 2022 10:31:11 +0100 you wrote:
> From: Eyal Birger <eyal.birger@gmail.com>
> 
> The commit in the "Fixes" tag tried to avoid a case where policy check
> is ignored due to dst caching in next hops.
> 
> However, when the traffic is locally consumed, the dst may be cached
> in a local TCP or UDP socket as part of early demux. In this case the
> "disable_policy" flag is not checked as ip_route_input_noref() was only
> called before caching, and thus, packets after the initial packet in a
> flow will be dropped if not matching policies.
> 
> [...]

Here is the summary with links:
  - [1/6] xfrm: fix "disable_policy" on ipv4 early demux
    https://git.kernel.org/netdev/net/c/3a5913183aa1
  - [2/6] xfrm: lwtunnel: squelch kernel warning in case XFRM encap type is not available
    https://git.kernel.org/netdev/net/c/d83f7040e184
  - [3/6] xfrm: replay: Fix ESN wrap around for GSO
    https://git.kernel.org/netdev/net/c/4b549ccce941
  - [4/6] af_key: Fix send_acquire race with pfkey_register
    https://git.kernel.org/netdev/net/c/7f57f8165cb6
  - [5/6] xfrm: Fix oops in __xfrm_state_delete()
    https://git.kernel.org/netdev/net/c/b97df039a68b
  - [6/6] xfrm: Fix ignored return value in xfrm6_init()
    https://git.kernel.org/netdev/net/c/40781bfb836e

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] 9+ messages in thread

* Re: [PATCH 0/6] pull request (net): ipsec 2022-11-23
  2022-11-23  9:31 [PATCH 0/6] pull request (net): ipsec 2022-11-23 Steffen Klassert
                   ` (5 preceding siblings ...)
  2022-11-23  9:31 ` [PATCH 6/6] xfrm: Fix ignored return value in xfrm6_init() Steffen Klassert
@ 2022-11-24 17:38 ` Alexander Lobakin
  6 siblings, 0 replies; 9+ messages in thread
From: Alexander Lobakin @ 2022-11-24 17:38 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: Alexander Lobakin, David Miller, Jakub Kicinski, Herbert Xu, netdev

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Wed, 23 Nov 2022 10:31:10 +0100

> 1) Fix "disable_policy" on ipv4 early demuxP Packets after
>    the initial packet in a flow might be incorectly dropped
>    on early demux if there are no matching policies.
>    From Eyal Birger.
> 
> 2) Fix a kernel warning in case XFRM encap type is not
>    available. From Eyal Birger.
> 
> 3) Fix ESN wrap around for GSO to avoid a double usage of a
>     sequence number. From Christian Langrock.
> 
> 4) Fix a send_acquire race with pfkey_register.
>    From Herbert Xu.
> 
> 5) Fix a list corruption panic in __xfrm_state_delete().
>    Thomas Jarosch.
> 
> 6) Fix an unchecked return value in xfrm6_init().
>    Chen Zhongjin.
> 
> Please pull or let me know if there are problems.
> 
> Thanks!
> 
> The following changes since commit 1d22f78d05737ce21bff7b88b6e58873f35e65ba:
> 
>   Merge tag 'ieee802154-for-net-2022-10-05' of git://git.kernel.org/pub/scm/linux/kernel/git/sschmidt/wpan (2022-10-05 20:38:46 -0700)
> 
> 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 40781bfb836eda57d19c0baa37c7e72590e05fdc:
> 
>   xfrm: Fix ignored return value in xfrm6_init() (2022-11-22 07:16:34 +0100)
> 
> ----------------------------------------------------------------
> Chen Zhongjin (1):
>       xfrm: Fix ignored return value in xfrm6_init()
> 
> Christian Langrock (1):
>       xfrm: replay: Fix ESN wrap around for GSO
> 
> Eyal Birger (2):
>       xfrm: fix "disable_policy" on ipv4 early demux
>       xfrm: lwtunnel: squelch kernel warning in case XFRM encap type is not available
> 
> Herbert Xu (1):
>       af_key: Fix send_acquire race with pfkey_register
> 
> Thomas Jarosch (1):
>       xfrm: Fix oops in __xfrm_state_delete()
> 
>  net/core/lwtunnel.c     |  4 +++-
>  net/ipv4/esp4_offload.c |  3 +++
>  net/ipv4/ip_input.c     |  5 +++++
>  net/ipv6/esp6_offload.c |  3 +++
>  net/ipv6/xfrm6_policy.c |  6 +++++-
>  net/key/af_key.c        | 34 +++++++++++++++++++++++-----------
>  net/xfrm/xfrm_device.c  | 15 ++++++++++++++-
>  net/xfrm/xfrm_replay.c  |  2 +-
>  8 files changed, 57 insertions(+), 15 deletions(-)

(for the whole PR)

Reviewed-by: Alexander Lobakin <alexandr.lobakin@intel.com>

Thanks,
Olek

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

end of thread, other threads:[~2022-11-24 17:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-23  9:31 [PATCH 0/6] pull request (net): ipsec 2022-11-23 Steffen Klassert
2022-11-23  9:31 ` [PATCH 1/6] xfrm: fix "disable_policy" on ipv4 early demux Steffen Klassert
2022-11-24  3:30   ` patchwork-bot+netdevbpf
2022-11-23  9:31 ` [PATCH 2/6] xfrm: lwtunnel: squelch kernel warning in case XFRM encap type is not available Steffen Klassert
2022-11-23  9:31 ` [PATCH 3/6] xfrm: replay: Fix ESN wrap around for GSO Steffen Klassert
2022-11-23  9:31 ` [PATCH 4/6] af_key: Fix send_acquire race with pfkey_register Steffen Klassert
2022-11-23  9:31 ` [PATCH 5/6] xfrm: Fix oops in __xfrm_state_delete() Steffen Klassert
2022-11-23  9:31 ` [PATCH 6/6] xfrm: Fix ignored return value in xfrm6_init() Steffen Klassert
2022-11-24 17:38 ` [PATCH 0/6] pull request (net): ipsec 2022-11-23 Alexander Lobakin

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.