[PATCH ipsec 1/8] xfrm: state: exact mark/mask match for SPI-keyed control-plane SA lookups

[PATCH ipsec 1/8] xfrm: state: exact mark/mask match for SPI-keyed control-plane SA lookups

[Antony Antony]

Add __xfrm_state_lookup_exact(), an identity-match like
__xfrm_state_lookup().

Wire it into every SPI-keyed control-plane path: DELSA/GETSA, UPDSA,
GETAE/NEWAE, EXPIRE, MIGRATE_STATE.

xfrm_state_add()'s duplicate-detect keeps the wildcard
__xfrm_state_locate() - unrelated, unchanged.

Fixes: 3d6acfa7641f ("xfrm: SA lookups with mark")
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 include/net/xfrm.h    |  3 ++
 net/xfrm/xfrm_state.c | 80 ++++++++++++++++++++++++++++++++++++++++++++-------
 net/xfrm/xfrm_user.c  | 29 ++++++++++---------
 3 files changed, 88 insertions(+), 24 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 519a0156a05c..f6ed590cb2ff 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1746,6 +1746,9 @@ struct xfrm_state *xfrm_state_lookup_byaddr(struct net *net, u32 mark,
 					    const xfrm_address_t *saddr,
 					    u8 proto,
 					    unsigned short family);
+struct xfrm_state *xfrm_state_lookup_exact(struct net *net, const struct xfrm_mark *mark,
+					   const xfrm_address_t *daddr, __be32 spi,
+					   u8 proto, unsigned short family);
 #ifdef CONFIG_XFRM_SUB_POLICY
 void xfrm_tmpl_sort(struct xfrm_tmpl **dst, struct xfrm_tmpl **src, int n,
 		    unsigned short family);
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index c58cd024e3c6..df761ce1c290 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1172,11 +1172,22 @@ static struct xfrm_state *__xfrm_state_lookup_all(const struct xfrm_hash_state_p
 	return NULL;
 }
 
-static struct xfrm_state *__xfrm_state_lookup(const struct xfrm_hash_state_ptrs *state_ptrs,
-					      u32 mark,
-					      const xfrm_address_t *daddr,
-					      __be32 spi, u8 proto,
-					      unsigned short family)
+/* exact=false: data-plane wildcard match against x's mask. exact=true:
+ * control-plane identity match, mark and mask must both match exactly.
+ */
+static bool xfrm_state_mark_matches(const struct xfrm_state *x, u32 mark, u32 mask, bool exact)
+{
+	if (exact)
+		return x->mark.v == mark && x->mark.m == mask;
+	return (mark & x->mark.m) == x->mark.v;
+}
+
+static struct xfrm_state *
+__xfrm_state_lookup(const struct xfrm_hash_state_ptrs *state_ptrs,
+		    u32 mark, u32 mask, bool exact,
+		    const xfrm_address_t *daddr,
+		    __be32 spi, u8 proto,
+		    unsigned short family)
 {
 	unsigned int h = __xfrm_spi_hash(daddr, spi, proto, family, state_ptrs->hmask);
 	struct xfrm_state *x;
@@ -1188,7 +1199,7 @@ static struct xfrm_state *__xfrm_state_lookup(const struct xfrm_hash_state_ptrs
 		    !xfrm_addr_equal(&x->id.daddr, daddr, family))
 			continue;
 
-		if ((mark & x->mark.m) != x->mark.v)
+		if (!xfrm_state_mark_matches(x, mark, mask, exact))
 			continue;
 		if (!xfrm_state_hold_rcu(x))
 			continue;
@@ -1198,6 +1209,17 @@ static struct xfrm_state *__xfrm_state_lookup(const struct xfrm_hash_state_ptrs
 	return NULL;
 }
 
+static struct xfrm_state *
+__xfrm_state_lookup_exact(const struct xfrm_hash_state_ptrs *state_ptrs,
+			  const struct xfrm_mark *mark,
+			  const xfrm_address_t *daddr,
+			  __be32 spi, u8 proto,
+			  unsigned short family)
+{
+	return __xfrm_state_lookup(state_ptrs, mark->v, mark->m, true,
+				   daddr, spi, proto, family);
+}
+
 struct xfrm_state *xfrm_input_state_lookup(struct net *net, u32 mark,
 					   const xfrm_address_t *daddr,
 					   __be32 spi, u8 proto,
@@ -1228,7 +1250,7 @@ struct xfrm_state *xfrm_input_state_lookup(struct net *net, u32 mark,
 
 	xfrm_hash_ptrs_get(net, &state_ptrs);
 
-	x = __xfrm_state_lookup(&state_ptrs, mark, daddr, spi, proto, family);
+	x = __xfrm_state_lookup(&state_ptrs, mark, 0, false, daddr, spi, proto, family);
 	if (x) {
 		spin_lock(&net->xfrm.xfrm_state_lock);
 		if (x->km.state != XFRM_STATE_VALID) {
@@ -1288,7 +1310,7 @@ __xfrm_state_locate(struct xfrm_state *x, int use_spi, int family)
 	xfrm_hash_ptrs_get(net, &state_ptrs);
 
 	if (use_spi)
-		return __xfrm_state_lookup(&state_ptrs, mark, &x->id.daddr,
+		return __xfrm_state_lookup(&state_ptrs, mark, 0, false, &x->id.daddr,
 					   x->id.spi, x->id.proto, family);
 	else
 		return __xfrm_state_lookup_byaddr(&state_ptrs, mark,
@@ -1297,6 +1319,27 @@ __xfrm_state_locate(struct xfrm_state *x, int use_spi, int family)
 						  x->id.proto, family);
 }
 
+/* Used by xfrm_state_update() only; xfrm_state_add()'s dup check keeps
+ * using the wildcard __xfrm_state_locate() above.
+ */
+static inline struct xfrm_state *
+__xfrm_state_locate_exact(struct xfrm_state *x, int use_spi, int family)
+{
+	struct xfrm_hash_state_ptrs state_ptrs;
+	struct net *net = xs_net(x);
+
+	xfrm_hash_ptrs_get(net, &state_ptrs);
+
+	if (use_spi)
+		return __xfrm_state_lookup_exact(&state_ptrs, &x->mark, &x->id.daddr,
+						 x->id.spi, x->id.proto, family);
+	else
+		return __xfrm_state_lookup_byaddr(&state_ptrs, x->mark.v & x->mark.m,
+						  &x->id.daddr,
+						  &x->props.saddr,
+						  x->id.proto, family);
+}
+
 static void xfrm_hash_grow_check(struct net *net, int have_hash_collision)
 {
 	if (have_hash_collision &&
@@ -2229,7 +2272,7 @@ int xfrm_state_update(struct xfrm_state *x)
 	to_put = NULL;
 
 	spin_lock_bh(&net->xfrm.xfrm_state_lock);
-	x1 = __xfrm_state_locate(x, use_spi, x->props.family);
+	x1 = __xfrm_state_locate_exact(x, use_spi, x->props.family);
 
 	err = -ESRCH;
 	if (!x1)
@@ -2374,7 +2417,7 @@ xfrm_state_lookup(struct net *net, u32 mark, const xfrm_address_t *daddr, __be32
 	rcu_read_lock();
 	xfrm_hash_ptrs_get(net, &state_ptrs);
 
-	x = __xfrm_state_lookup(&state_ptrs, mark, daddr, spi, proto, family);
+	x = __xfrm_state_lookup(&state_ptrs, mark, 0, false, daddr, spi, proto, family);
 	rcu_read_unlock();
 	return x;
 }
@@ -2398,6 +2441,23 @@ xfrm_state_lookup_byaddr(struct net *net, u32 mark,
 }
 EXPORT_SYMBOL(xfrm_state_lookup_byaddr);
 
+struct xfrm_state *
+xfrm_state_lookup_exact(struct net *net, const struct xfrm_mark *mark,
+			const xfrm_address_t *daddr, __be32 spi,
+			u8 proto, unsigned short family)
+{
+	struct xfrm_hash_state_ptrs state_ptrs;
+	struct xfrm_state *x;
+
+	rcu_read_lock();
+	xfrm_hash_ptrs_get(net, &state_ptrs);
+
+	x = __xfrm_state_lookup_exact(&state_ptrs, mark, daddr, spi, proto, family);
+	rcu_read_unlock();
+	return x;
+}
+EXPORT_SYMBOL(xfrm_state_lookup_exact);
+
 struct xfrm_state *
 xfrm_find_acq(struct net *net, const struct xfrm_mark *mark, u8 mode, u32 reqid,
 	      u32 if_id, u32 pcpu_num, u8 proto, const xfrm_address_t *daddr,
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 6384795ee6b2..b56fca666b89 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1089,11 +1089,12 @@ static struct xfrm_state *xfrm_user_state_lookup(struct net *net,
 	struct xfrm_state *x = NULL;
 	struct xfrm_mark m;
 	int err;
-	u32 mark = xfrm_mark_get(attrs, &m);
+
+	xfrm_mark_get(attrs, &m);
 
 	if (xfrm_id_proto_match(p->proto, IPSEC_PROTO_ANY)) {
 		err = -ESRCH;
-		x = xfrm_state_lookup(net, mark, &p->daddr, p->spi, p->proto, p->family);
+		x = xfrm_state_lookup_exact(net, &m, &p->daddr, p->spi, p->proto, p->family);
 	} else {
 		xfrm_address_t *saddr = NULL;
 
@@ -1104,7 +1105,7 @@ static struct xfrm_state *xfrm_user_state_lookup(struct net *net,
 		}
 
 		err = -ESRCH;
-		x = xfrm_state_lookup_byaddr(net, mark,
+		x = xfrm_state_lookup_byaddr(net, m.v & m.m,
 					     &p->daddr, saddr,
 					     p->proto, p->family);
 	}
@@ -2788,14 +2789,13 @@ static int xfrm_get_ae(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct sk_buff *r_skb;
 	int err;
 	struct km_event c;
-	u32 mark;
 	struct xfrm_mark m;
 	struct xfrm_aevent_id *p = nlmsg_data(nlh);
 	struct xfrm_usersa_id *id = &p->sa_id;
 
-	mark = xfrm_mark_get(attrs, &m);
+	xfrm_mark_get(attrs, &m);
 
-	x = xfrm_state_lookup(net, mark, &id->daddr, id->spi, id->proto, id->family);
+	x = xfrm_state_lookup_exact(net, &m, &id->daddr, id->spi, id->proto, id->family);
 	if (x == NULL)
 		return -ESRCH;
 
@@ -2836,7 +2836,6 @@ static int xfrm_new_ae(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct xfrm_state *x;
 	struct km_event c;
 	int err = -EINVAL;
-	u32 mark = 0;
 	struct xfrm_mark m;
 	struct xfrm_aevent_id *p = nlmsg_data(nlh);
 	struct nlattr *rp = attrs[XFRMA_REPLAY_VAL];
@@ -2856,9 +2855,10 @@ static int xfrm_new_ae(struct sk_buff *skb, struct nlmsghdr *nlh,
 		return err;
 	}
 
-	mark = xfrm_mark_get(attrs, &m);
+	xfrm_mark_get(attrs, &m);
 
-	x = xfrm_state_lookup(net, mark, &p->sa_id.daddr, p->sa_id.spi, p->sa_id.proto, p->sa_id.family);
+	x = xfrm_state_lookup_exact(net, &m, &p->sa_id.daddr, p->sa_id.spi,
+				    p->sa_id.proto, p->sa_id.family);
 	if (x == NULL)
 		return -ESRCH;
 
@@ -2992,9 +2992,10 @@ static int xfrm_add_sa_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct xfrm_user_expire *ue = nlmsg_data(nlh);
 	struct xfrm_usersa_info *p = &ue->state;
 	struct xfrm_mark m;
-	u32 mark = xfrm_mark_get(attrs, &m);
 
-	x = xfrm_state_lookup(net, mark, &p->id.daddr, p->id.spi, p->id.proto, p->family);
+	xfrm_mark_get(attrs, &m);
+
+	x = xfrm_state_lookup_exact(net, &m, &p->id.daddr, p->id.spi, p->id.proto, p->family);
 
 	err = -ENOENT;
 	if (x == NULL)
@@ -3361,9 +3362,9 @@ static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	copy_from_user_migrate_state(&m, um);
 
-	x = xfrm_state_lookup(net, m.old_mark.v & m.old_mark.m,
-			      &um->id.daddr, um->id.spi,
-			      um->id.proto, um->id.family);
+	x = xfrm_state_lookup_exact(net, &m.old_mark,
+				    &um->id.daddr, um->id.spi,
+				    um->id.proto, um->id.family);
 	if (!x) {
 		NL_SET_ERR_MSG(extack, "Can not find state");
 		return -ESRCH;

-- 
2.47.3

[PATCH ipsec 2/8] xfrm: state: exact mark/mask match for by-address control-plane SA lookups

[Antony Antony]

Add mark/mask/exact parameters directly to __xfrm_state_lookup_byaddr(),
reusing the SPI-keyed lookup's comparison.

Use __xfrm_state_locate_exact()'s by-address branch UPDSA
and xfrm_user_state_lookup()'s by-address branch
(DELSA/GETSA for non-SPI-keyed lookups).

Fixes: 3d6acfa7641f ("xfrm: SA lookups with mark")
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 include/net/xfrm.h     |  2 +-
 net/ipv6/xfrm6_input.c |  2 +-
 net/xfrm/xfrm_state.c  | 21 +++++++++++----------
 net/xfrm/xfrm_user.c   |  2 +-
 4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index f6ed590cb2ff..ebe514376254 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1741,7 +1741,7 @@ struct xfrm_state *xfrm_input_state_lookup(struct net *net, u32 mark,
 					   const xfrm_address_t *daddr,
 					   __be32 spi, u8 proto,
 					   unsigned short family);
-struct xfrm_state *xfrm_state_lookup_byaddr(struct net *net, u32 mark,
+struct xfrm_state *xfrm_state_lookup_byaddr(struct net *net, u32 mark, u32 mask, bool exact,
 					    const xfrm_address_t *daddr,
 					    const xfrm_address_t *saddr,
 					    u8 proto,
diff --git a/net/ipv6/xfrm6_input.c b/net/ipv6/xfrm6_input.c
index 89d0443b5307..2dd347fece52 100644
--- a/net/ipv6/xfrm6_input.c
+++ b/net/ipv6/xfrm6_input.c
@@ -272,7 +272,7 @@ int xfrm6_input_addr(struct sk_buff *skb, xfrm_address_t *daddr,
 			break;
 		}
 
-		x = xfrm_state_lookup_byaddr(net, skb->mark, dst, src, proto, AF_INET6);
+		x = xfrm_state_lookup_byaddr(net, skb->mark, 0, false, dst, src, proto, AF_INET6);
 		if (!x)
 			continue;
 
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index df761ce1c290..d78cfe481f75 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1274,11 +1274,12 @@ struct xfrm_state *xfrm_input_state_lookup(struct net *net, u32 mark,
 }
 EXPORT_SYMBOL(xfrm_input_state_lookup);
 
-static struct xfrm_state *__xfrm_state_lookup_byaddr(const struct xfrm_hash_state_ptrs *state_ptrs,
-						     u32 mark,
-						     const xfrm_address_t *daddr,
-						     const xfrm_address_t *saddr,
-						     u8 proto, unsigned short family)
+static struct xfrm_state *
+__xfrm_state_lookup_byaddr(const struct xfrm_hash_state_ptrs *state_ptrs,
+			   u32 mark, u32 mask, bool exact,
+			   const xfrm_address_t *daddr,
+			   const xfrm_address_t *saddr,
+			   u8 proto, unsigned short family)
 {
 	unsigned int h = __xfrm_src_hash(daddr, saddr, family, state_ptrs->hmask);
 	struct xfrm_state *x;
@@ -1290,7 +1291,7 @@ static struct xfrm_state *__xfrm_state_lookup_byaddr(const struct xfrm_hash_stat
 		    !xfrm_addr_equal(&x->props.saddr, saddr, family))
 			continue;
 
-		if ((mark & x->mark.m) != x->mark.v)
+		if (!xfrm_state_mark_matches(x, mark, mask, exact))
 			continue;
 		if (!xfrm_state_hold_rcu(x))
 			continue;
@@ -1313,7 +1314,7 @@ __xfrm_state_locate(struct xfrm_state *x, int use_spi, int family)
 		return __xfrm_state_lookup(&state_ptrs, mark, 0, false, &x->id.daddr,
 					   x->id.spi, x->id.proto, family);
 	else
-		return __xfrm_state_lookup_byaddr(&state_ptrs, mark,
+		return __xfrm_state_lookup_byaddr(&state_ptrs, mark, 0, false,
 						  &x->id.daddr,
 						  &x->props.saddr,
 						  x->id.proto, family);
@@ -1334,7 +1335,7 @@ __xfrm_state_locate_exact(struct xfrm_state *x, int use_spi, int family)
 		return __xfrm_state_lookup_exact(&state_ptrs, &x->mark, &x->id.daddr,
 						 x->id.spi, x->id.proto, family);
 	else
-		return __xfrm_state_lookup_byaddr(&state_ptrs, x->mark.v & x->mark.m,
+		return __xfrm_state_lookup_byaddr(&state_ptrs, x->mark.v, x->mark.m, true,
 						  &x->id.daddr,
 						  &x->props.saddr,
 						  x->id.proto, family);
@@ -2424,7 +2425,7 @@ xfrm_state_lookup(struct net *net, u32 mark, const xfrm_address_t *daddr, __be32
 EXPORT_SYMBOL(xfrm_state_lookup);
 
 struct xfrm_state *
-xfrm_state_lookup_byaddr(struct net *net, u32 mark,
+xfrm_state_lookup_byaddr(struct net *net, u32 mark, u32 mask, bool exact,
 			 const xfrm_address_t *daddr, const xfrm_address_t *saddr,
 			 u8 proto, unsigned short family)
 {
@@ -2435,7 +2436,7 @@ xfrm_state_lookup_byaddr(struct net *net, u32 mark,
 
 	xfrm_hash_ptrs_get(net, &state_ptrs);
 
-	x = __xfrm_state_lookup_byaddr(&state_ptrs, mark, daddr, saddr, proto, family);
+	x = __xfrm_state_lookup_byaddr(&state_ptrs, mark, mask, exact, daddr, saddr, proto, family);
 	rcu_read_unlock();
 	return x;
 }
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index b56fca666b89..11ec3b14a42f 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1105,7 +1105,7 @@ static struct xfrm_state *xfrm_user_state_lookup(struct net *net,
 		}
 
 		err = -ESRCH;
-		x = xfrm_state_lookup_byaddr(net, m.v & m.m,
+		x = xfrm_state_lookup_byaddr(net, m.v, m.m, true,
 					     &p->daddr, saddr,
 					     p->proto, p->family);
 	}

-- 
2.47.3

[PATCH ipsec 3/8] selftests: net: xfrm_state: add mark shadowing tests for state lookups

[Antony Antony]

Add SPI and by-address wildcard-mark shadowing tests.

Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 tools/testing/selftests/net/xfrm_state.sh | 130 +++++++++++++++++++++++++++++-
 1 file changed, 129 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/xfrm_state.sh b/tools/testing/selftests/net/xfrm_state.sh
index f6c54a6496d7..f202073726a9 100755
--- a/tools/testing/selftests/net/xfrm_state.sh
+++ b/tools/testing/selftests/net/xfrm_state.sh
@@ -42,7 +42,11 @@ tests="
 	mtu_ipv4_r2			IPv4 MTU exceeded from ESP router r2
 	mtu_ipv6_r2			IPv6 MTU exceeded from ESP router r2
 	mtu_ipv4_r3			IPv4 MTU exceeded from router r3
-	mtu_ipv6_r3			IPv6 MTU exceeded from router r3"
+	mtu_ipv6_r3			IPv6 MTU exceeded from router r3
+	mark_wildcard_shadow		mark: wildcard SA in by-spi state get lookup
+	mark_wildcard_delete		mark: wildcard SA in by-spi state delete
+	mark_wildcard_get_addr		mark: wildcard SA in by-address get lookup
+	mark_wildcard_delete_addr	mark: wildcard SA in by-address delete"
 
 prefix4="10.1"
 prefix6="fc00"
@@ -101,6 +105,10 @@ run_test() {
 		mtu_ipv6_r2)         test_mtu_ipv6_r2 ;;
 		mtu_ipv4_r3)         test_mtu_ipv4_r3 ;;
 		mtu_ipv6_r3)         test_mtu_ipv6_r3 ;;
+		mark_wildcard_shadow)      test_mark_wildcard_shadow ;;
+		mark_wildcard_delete)      test_mark_wildcard_delete ;;
+		mark_wildcard_get_addr)    test_mark_wildcard_get_addr ;;
+		mark_wildcard_delete_addr) test_mark_wildcard_delete_addr ;;
 		esac
 		ret=$?
 
@@ -167,6 +175,8 @@ setup_namespaces() {
 	[ -n "${NS_S2}" ] && ns_s2=(ip netns exec "${NS_S2}") && ns_active="${ns_active} $NS_S2"
 	[ -n "${NS_R3}" ] && ns_r3=(ip netns exec "${NS_R3}") && ns_active="${ns_active} $NS_R3"
 	[ -n "${NS_B}" ] && ns_active="${ns_active} $NS_B"
+
+	return 0
 }
 
 addr_add() {
@@ -295,6 +305,18 @@ setup_ns_set_v6x() {
 	set_xfrm_params
 }
 
+setup_ns_set_simple() {
+	# Single namespace, no veths/routes.
+	ns_set="a"
+	imax=1
+	src="10.1.1.1"
+	dst="10.1.1.2"
+	src_net="10.1.0.0/24"
+	dst_net="10.2.0.0/24"
+
+	set_xfrm_params
+}
+
 setup_network() {
 	# Create veths and add addresses
 	local -a ns_cmd
@@ -403,6 +425,7 @@ setup() {
 		ns_set_v4x)    setup_ns_set_v4x ;;
 		ns_set_v6)     setup_ns_set_v6 ;;
 		ns_set_v6x)    setup_ns_set_v6x ;;
+		ns_set_simple) setup_ns_set_simple ;;
 		namespaces)    setup_namespaces ;;
 		network)       setup_network ;;
 		xfrm)          setup_xfrm ;;
@@ -548,6 +571,111 @@ test_mtu_ipv6_r3() {
 	return "${rc}"
 }
 
+# SA_decoy (mark 0/0, added second) shadows SA_target (mark 1/1) on a
+# wildcard mark lookup. No traffic sent; these only exercise the SAD.
+
+test_mark_wildcard_shadow() {
+	setup ns_set_simple namespaces || return "$ksft_skip"
+	local result=0
+
+	run_cmd "${ns_a[@]}" ip xfrm state add \
+		src "${src}" dst "${dst}" proto esp spi 0x1000 \
+		reqid 100 mode tunnel \
+		aead 'rfc4106(gcm(aes))' 0x1111111111111111111111111111111111111111 96 \
+		mark 1 mask 1
+
+	run_cmd "${ns_a[@]}" ip xfrm state add \
+		src "${src}" dst "${dst}" proto esp spi 0x1000 \
+		reqid 100 mode tunnel \
+		aead 'rfc4106(gcm(aes))' 0x2222222222222222222222222222222222222222 96 \
+		mark 0 mask 0
+
+	run_cmd_err "${ns_a[@]}" ip xfrm state get \
+		dst "${dst}" proto esp spi 0x1000 \
+		mark 1 mask 1
+
+	# Expected: SA_target (mark 0x1/0x1). Actual (bug): SA_decoy (mark 0/0).
+	echo "$out" | grep -q "mark 0x1/0x1" || result=1
+
+	return "${result}"
+}
+
+test_mark_wildcard_delete() {
+	setup ns_set_simple namespaces || return "$ksft_skip"
+	local result=0
+
+	run_cmd "${ns_a[@]}" ip xfrm state add \
+		src "${src}" dst "${dst}" proto esp spi 0x1000 \
+		reqid 100 mode tunnel \
+		aead 'rfc4106(gcm(aes))' 0x1111111111111111111111111111111111111111 96 \
+		mark 1 mask 1
+
+	run_cmd "${ns_a[@]}" ip xfrm state add \
+		src "${src}" dst "${dst}" proto esp spi 0x1000 \
+		reqid 100 mode tunnel \
+		aead 'rfc4106(gcm(aes))' 0x2222222222222222222222222222222222222222 96 \
+		mark 0 mask 0
+
+	run_cmd "${ns_a[@]}" ip xfrm state delete \
+		dst "${dst}" proto esp spi 0x1000 \
+		mark 1 mask 1
+
+	run_cmd_err "${ns_a[@]}" ip xfrm state show
+	echo "$out" | grep -q "mark 0x1/0x1" && result=1
+
+	return "${result}"
+}
+
+# by-address counterpart: proto route2/hao (IPv6 mobility) have no SPI,
+# so xfrm_user_state_lookup() resolves them by address instead.
+
+test_mark_wildcard_get_addr() {
+	setup ns_set_simple namespaces || return "$ksft_skip"
+	local result=0
+	local src6="fc00:9::1"
+	local dst6="fc00:9::2"
+
+	run_cmd "${ns_a[@]}" ip xfrm state add \
+		src "${src6}" dst "${dst6}" proto route2 mode ro coa fc00:9::3 \
+		mark 1 mask 1
+
+	run_cmd "${ns_a[@]}" ip xfrm state add \
+		src "${src6}" dst "${dst6}" proto route2 mode ro coa fc00:9::4 \
+		mark 0 mask 0
+
+	run_cmd_err "${ns_a[@]}" ip xfrm state get \
+		src "${src6}" dst "${dst6}" proto route2 \
+		mark 1 mask 1
+
+	echo "$out" | grep -q "mark 0x1/0x1" || result=1
+
+	return "${result}"
+}
+
+test_mark_wildcard_delete_addr() {
+	setup ns_set_simple namespaces || return "$ksft_skip"
+	local result=0
+	local src6="fc00:9::1"
+	local dst6="fc00:9::2"
+
+	run_cmd "${ns_a[@]}" ip xfrm state add \
+		src "${src6}" dst "${dst6}" proto route2 mode ro coa fc00:9::3 \
+		mark 1 mask 1
+
+	run_cmd "${ns_a[@]}" ip xfrm state add \
+		src "${src6}" dst "${dst6}" proto route2 mode ro coa fc00:9::4 \
+		mark 0 mask 0
+
+	run_cmd "${ns_a[@]}" ip xfrm state delete \
+		src "${src6}" dst "${dst6}" proto route2 \
+		mark 1 mask 1
+
+	run_cmd_err "${ns_a[@]}" ip xfrm state show
+	echo "$out" | grep -q "mark 0x1/0x1" && result=1
+
+	return "${result}"
+}
+
 ################################################################################
 #
 usage() {

-- 
2.47.3

[PATCH ipsec 4/8] xfrm: fix use-after-free of migrated state in xfrm_do_migrate_state()

[Antony Antony]

A concurrent delete can free the migrated state before
xfrm_do_migrate_state() finishes using it.

Fixes: a9d155ea9b44 ("xfrm: add XFRM_MSG_MIGRATE_STATE for single SA migration")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 net/xfrm/xfrm_user.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 11ec3b14a42f..29cbdc836cfc 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -3468,6 +3468,7 @@ static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
 	__xfrm_state_delete(x);
 	spin_unlock_bh(&x->lock);
 
+	xfrm_state_hold(xc);
 	err = xfrm_state_migrate_install(x, xc, &m, extack);
 	if (err < 0) {
 		/*
@@ -3475,6 +3476,7 @@ static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
 		 * free under xfrm_cfg_mutex. Both SAs are gone if it does;
 		 * restoring x would risk SN/IV reuse.
 		 */
+		xfrm_state_put(xc);
 		goto out;
 	}
 
@@ -3493,6 +3495,7 @@ static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
 		err = 0;
 	}
 
+	xfrm_state_put(xc);
 out:
 	xfrm_state_put(x);
 	return err;

-- 
2.47.3

[PATCH ipsec 5/8] xfrm: fix hw offload state leak on xfrm_do_migrate_state() error path

[Antony Antony]

In the error path, the cloned state is dropped without removing its
hardware offload, leaking the offloaded SA entry.

Fixes: a9d155ea9b44 ("xfrm: add XFRM_MSG_MIGRATE_STATE for single SA migration")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 net/xfrm/xfrm_user.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 29cbdc836cfc..87ef198993db 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -3500,6 +3500,8 @@ static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
 	xfrm_state_put(x);
 	return err;
 out_xc:
+	if (m.xuo)
+		xfrm_dev_state_delete(xc);
 	xc->km.state = XFRM_STATE_DEAD;
 	xfrm_state_put(xc);
 	xfrm_state_put(x);

-- 
2.47.3

[PATCH ipsec 6/8] xfrm: include mark in MIGRATE_STATE SA collision check

[Antony Antony]

The SA lookup tuple is (daddr, spi, proto, family, mark), but the
EEXIST pre-check and the xfrm_state_insert() vs xfrm_state_add()
decision only considered daddr and family, ignoring mark.
A migration that only changes the mark inserts a duplicate SA tuple
into the hash tables.

Before:
root@west:~# ip xfrm state add src 10.1.1.1 dst 10.1.1.2 proto esp \
        spi 0x1000  reqid 100 mode tunnel aead "rfc4106(gcm(aes))" \
        0x1111111111111111111111111111111111111111 96 \
        mark 0x1 mask 0xff

root@west:~# ip xfrm state add src 10.1.1.1 dst 10.1.1.2 proto esp \
        spi 0x1000  reqid 100 mode tunnel aead "rfc4106(gcm(aes))" \
        0x1111111111111111111111111111111111111111 96 \
        mark 0x2 mask 0xff
root@west:~# ip xfrm state migrate dst 10.1.1.2 proto esp spi 0x1000 \
        mark 0x1 mask 0xff \
        new-dst 10.1.1.2 new-src 10.1.1.1 new-reqid 100 \
        new-mark 0x2 mask 0xff
ip x s
src 10.1.1.1 dst 10.1.1.2
    proto esp spi 0x00001000 reqid 100 mode tunnel
    replay-window 0
    mark 0x2/0xff
    aead rfc4106(gcm(aes)) 0x1111111111111111111111111111111111111111 96
    anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
src 10.1.1.1 dst 10.1.1.2
    proto esp spi 0x00001000 reqid 100 mode tunnel
    replay-window 0
    mark 0x2/0xff
    aead rfc4106(gcm(aes)) 0x1111111111111111111111111111111111111111 96
    anti-replay context: seq 0x0, oseq 0x0, bitmap 0x00000000
    sel src 0.0.0.0/0 dst 0.0.0.0/0

Notice two states with same mark 0x2/0xff.
After:

Error: New SA tuple already occupied.

Fixes: a9d155ea9b44 ("xfrm: add XFRM_MSG_MIGRATE_STATE for single SA migration")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 net/xfrm/xfrm_state.c |  8 +++++---
 net/xfrm/xfrm_user.c  | 15 ++++++++++-----
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index d78cfe481f75..bffe985e42ea 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2221,10 +2221,12 @@ int xfrm_state_migrate_install(const struct xfrm_state *x,
 			       struct netlink_ext_ack *extack)
 {
 	if (m->new_family == m->old_family &&
-	    xfrm_addr_equal(&x->id.daddr, &m->new_daddr, m->new_family)) {
+	    xfrm_addr_equal(&x->id.daddr, &m->new_daddr, m->new_family) &&
+	    xc->mark.v == x->mark.v && xc->mark.m == x->mark.m) {
 		/*
-		 * Care is needed when the destination address of the state is
-		 * to be updated as it is a part of triplet.
+		 * Care is needed when the destination address or mark of the
+		 * state is to be updated, as they are part of the lookup
+		 * triplet.
 		 */
 		xfrm_state_insert(xc);
 	} else {
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 87ef198993db..a2317e6e6802 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -3435,18 +3435,23 @@ static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
 						       x->nat_keepalive_interval);
 
 	if (m.new_family != um->id.family ||
-	    !xfrm_addr_equal(&m.new_daddr, &um->id.daddr, um->id.family)) {
+	    !xfrm_addr_equal(&m.new_daddr, &um->id.daddr, um->id.family) ||
+	    (m.new_mark && (m.new_mark->v != x->mark.v ||
+			   m.new_mark->m != x->mark.m))) {
 		u32 new_mark_key = m.new_mark ? m.new_mark->v & m.new_mark->m :
-						m.old_mark.v & m.old_mark.m;
+						x->mark.v & x->mark.m;
 		struct xfrm_state *x_new;
 
 		x_new = xfrm_state_lookup(net, new_mark_key, &m.new_daddr,
 					  um->id.spi, um->id.proto, m.new_family);
 		if (x_new) {
 			xfrm_state_put(x_new);
-			NL_SET_ERR_MSG(extack, "New SA tuple already occupied");
-			err = -EEXIST;
-			goto out;
+			if (x_new != x) {
+				NL_SET_ERR_MSG(extack, "New SA tuple already occupied");
+				err = -EEXIST;
+				goto out;
+			}
+			/* self-match via wide mark mask; not a collision */
 		}
 	}
 

-- 
2.47.3

[PATCH ipsec 7/8] xfrm: pass extack through to xfrm_init_replay() from xfrm_init_state()

[Antony Antony]

xfrm_init_state() takes an extack parameter but doesn't pass it to
xfrm_init_replay(), so replay-window errors can't be reported via
extack.

Fixes: 231a1744dc43 ("xfrm: add extack to xfrm_init_state")
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 net/xfrm/xfrm_state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index bffe985e42ea..0b3dc7789dd7 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -3369,7 +3369,7 @@ int xfrm_init_state(struct xfrm_state *x, struct netlink_ext_ack *extack)
 	if (err)
 		return err;
 
-	err = xfrm_init_replay(x, NULL);
+	err = xfrm_init_replay(x, extack);
 	if (err)
 		return err;
 

-- 
2.47.3

[PATCH ipsec 8/8] docs: xfrm: include mark in XFRM_MSG_MIGRATE_STATE EEXIST tuple

[Antony Antony]

Document mark as part of the EEXIST tuple and update the SA lookup
description to match.

Fixes: c13c0cc6f52e ("xfrm: add documentation for XFRM_MSG_MIGRATE_STATE")
Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 Documentation/networking/xfrm/xfrm_migrate_state.rst | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/Documentation/networking/xfrm/xfrm_migrate_state.rst b/Documentation/networking/xfrm/xfrm_migrate_state.rst
index 9d53cb22b007..905e3f0c4ee0 100644
--- a/Documentation/networking/xfrm/xfrm_migrate_state.rst
+++ b/Documentation/networking/xfrm/xfrm_migrate_state.rst
@@ -27,15 +27,18 @@ SA Identification
 =================
 
 The struct is defined in ``include/uapi/linux/xfrm.h``. The SA is looked
-up using ``xfrm_state_lookup()`` with ``id.spi``,
-``id.daddr``, ``id.proto``, ``id.family``, and
-``old_mark.v & old_mark.m`` as the mark key::
+up using ``xfrm_state_lookup_exact()`` with ``id.spi``, ``id.daddr``,
+``id.proto``, ``id.family``, and an exact match against ``old_mark.v``
+and ``old_mark.m``. Unlike the data path, which uses a masked
+comparison, this requires the SA's mark and mask to equal ``old_mark``
+exactly, so a broad-mask SA is never matched when a more specific one
+was intended. If no such SA exists, ``-ESRCH`` is returned.::
 
     struct xfrm_user_migrate_state {
         struct xfrm_usersa_id  id;       /* spi, daddr, proto, family */
         xfrm_address_t         new_daddr;
         xfrm_address_t         new_saddr;
-        struct xfrm_mark       old_mark; /* SA lookup: key = v & m */
+        struct xfrm_mark       old_mark; /* SA lookup key (exact v/m match) */
         struct xfrm_selector   new_sel;  /* new selector (see Flags) */
         __u32                  new_reqid;
         __u32                  flags;    /* XFRM_MIGRATE_STATE_* */
@@ -72,8 +75,8 @@ inherits the value from the existing SA (omit-to-inherit).
      - Description
    * - ``XFRMA_MARK``
      - Mark on the migrated SA (``struct xfrm_mark``). Absent inherits
-       ``old_mark``. To use no mark on the new SA, send ``XFRMA_MARK``
-       with ``{0, 0}``.
+       the mark of the existing SA. To use no mark on the new SA, send
+       ``XFRMA_MARK`` with ``{0, 0}``.
    * - ``XFRMA_ENCAP``
      - UDP encapsulation template; only ``UDP_ENCAP_ESPINUDP`` is supported.
        Set ``encap_type=0`` to remove encap.
@@ -259,8 +262,9 @@ Attributes in the notification
 Error Handling
 ==============
 
-If the target SA tuple (new daddr, SPI, proto, new family) is already
-occupied, the operation returns ``-EEXIST`` before the migration begins.
+If the target SA tuple (new daddr, SPI, proto, new family, mark) is
+already occupied, the operation returns ``-EEXIST`` before the migration
+begins.
 The old SA remains intact and the operation is safe to retry after
 resolving the conflict.
 

-- 
2.47.3

[PATCH ipsec 0/8] xfrm: state: exact mark/mask match for control-plane SA lookups

[Antony Antony]

While looking into a XFRM_MSG_MIGRATE_STATE issue reported by Sashiko,
we found the underlying problem generalizes: xfrm allows multiple SAs
to coexist for the same (SPI, daddr, proto) differing only in mark,
and every control-plane operation that resolves "which SA" - get,
delete, update, get_ae, new_ae, expire, migrate - uses the same
wildcard mark match the data path needs. A broader-mask SA can
silently shadow a more specific one:

  # ip xfrm state add ... spi 0x1000 mark 1 mask 1 (SA_target)
  # ip xfrm state add ... spi 0x1000 mark 0 mask 0
    (SA_decoy, catch-all, added after -> bucket head)
  # ip xfrm state delete dst ... proto esp spi 0x1000 mark 1 mask 1
    -> deletes SA_decoy; SA_target survives, untouched

xfrm policy had the same bug, fixed in commit 4f47e8ab6ab7
("xfrm: policy: match with both mark and mask on user interfaces").

Control-plane lookups need an exact mark/mask match; the wildcard
match stays for the data path and state_add only.
This series applies that fix across every affected method,
not just XFRM_MSG_MIGRATE_STATE.

More examples in the attached self tests.
This series not fixing likely isusses PF_KEY. As it
is no more receiving non critical fixes.

---
Antony Antony (8):
      xfrm: state: exact mark/mask match for SPI-keyed control-plane SA lookups
      xfrm: state: exact mark/mask match for by-address control-plane SA lookups
      selftests: net: xfrm_state: add mark shadowing tests for state lookups
      xfrm: fix use-after-free of migrated state in xfrm_do_migrate_state()
      xfrm: fix hw offload state leak on xfrm_do_migrate_state() error path
      xfrm: include mark in MIGRATE_STATE SA collision check
      xfrm: pass extack through to xfrm_init_replay() from xfrm_init_state()
      docs: xfrm: include mark in XFRM_MSG_MIGRATE_STATE EEXIST tuple

 .../networking/xfrm/xfrm_migrate_state.rst         |  20 ++--
 include/net/xfrm.h                                 |   5 +-
 net/ipv6/xfrm6_input.c                             |   2 +-
 net/xfrm/xfrm_state.c                              | 109 +++++++++++++----
 net/xfrm/xfrm_user.c                               |  49 +++++---
 tools/testing/selftests/net/xfrm_state.sh          | 130 ++++++++++++++++++++-
 6 files changed, 262 insertions(+), 53 deletions(-)
---
base-commit: 226f4a490d1a938fc838d8f8c46a4eca864c0d78
change-id: migrate-state-fixes-063ee0342611

Best regards,
--  
Antony Antony <antony.antony@secunet.com>