[PATCH ipsec-next 0/6] xfrm: XFRM_MSG_MIGRATE_STATE new netlink message

[PATCH ipsec-next 0/6] xfrm: XFRM_MSG_MIGRATE_STATE new netlink message

[Antony Antony]

    The current XFRM_MSG_MIGRATE interface is tightly coupled to policy and
    SA migration, and it lacks the information required to reliably migrate
    individual SAs. This makes it unsuitable for IKEv2 deployments,
    dual-stack setups (IPv4/IPv6), and scenarios where policies are managed
    externally (e.g., by other daemons than IKE daemon).

    Mandatory SA selector list
    The current API requires a non-empty SA selector list, which does not
    reflect IKEv2 use case.
    A single Child SA may correspond to multiple policies,
    and SA discovery already occurs via address and reqid matching. With
    dual-stack Child SAs this leads to excessive churn: the current method
    would have to be called up to six times (in/out/fwd × v4/v6) on SA,
    while the new method only requires two calls. While polices are
    migrated, first installing a block policy

    Selectors lack SPI (and marks)
    XFRM_MSG_MIGRATE cannot uniquely identify an SA when multiple SAs share
    the same policies (per-CPU SAs, SELinux label-based SAs, etc.). Without
    the SPI, the kernel may update the wrong SA instance.

    Reqid cannot be changed
    Some implementations allocate reqids based on traffic selectors. In
    host-to-host or selector-changing scenarios, the reqid must change,
    which the current API cannot express.

    Because strongSwan and other implementations manage policies
    independently of the kernel, an interface that updates only a specific
    SA — with complete and unambiguous identification — is required.

    XFRM_MSG_MIGRATE_STATE provides that interface. It supports migration
    of a single SA via xfrm_usersa_id (including SPI) and we fix
    encap removal in this patch set, reqid updates, address changes,
    and other SA-specific parameters. It avoids the structural limitations
    of
    XFRM_MSG_MIGRATE and provides a simpler, extensible mechanism for
    precise per-SA migration without involving policies.

Antony Antony (6):
  xfrm: remove redundent assignment
  xfrm: allow migration from UDP encapsulated to non-encapsulated ESP
  xfrm: rename reqid in xfrm_migrate
  xfrm: add XFRM_MSG_MIGRATE_STATE for single SA migration
  xfrm: reqid is invarient in old migration
  xfrm: check that SA is in VALID state before use

 include/net/xfrm.h          |   3 +-
 include/uapi/linux/xfrm.h   |  11 +++
 net/key/af_key.c            |  10 +--
 net/xfrm/xfrm_policy.c      |   4 +-
 net/xfrm/xfrm_state.c       |  41 ++++-----
 net/xfrm/xfrm_user.c        | 164 +++++++++++++++++++++++++++++++++++-
 security/selinux/nlmsgtab.c |   3 +-
 7 files changed, 206 insertions(+), 30 deletions(-)

--
2.39.5

[PATCH ipsec-next 1/6] xfrm: remove redundent assignment

[Antony Antony]

this assignmet is overwritten within the same function further down

80c9abaabf42 ("[XFRM]: Extension for dynamic update of endpoint address(es)")

x->props.family = m->new_family;

e03c3bba351f ("xfrm: Fix xfrm migrate issues when address family changes")
---
 net/xfrm/xfrm_state.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 9e14e453b55c..5ebb9f53956e 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1980,7 +1980,6 @@ static struct xfrm_state *xfrm_state_clone_and_setup(struct xfrm_state *orig,
 	x->props.mode = orig->props.mode;
 	x->props.replay_window = orig->props.replay_window;
 	x->props.reqid = orig->props.reqid;
-	x->props.family = orig->props.family;
 	x->props.saddr = orig->props.saddr;
 
 	if (orig->aalg) {
-- 
2.39.5

[PATCH ipsec-next 2/6] xfrm: allow migration from UDP encapsulated to non-encapsulated ESP

[Antony Antony]

The current code prevents migrating an SA from UDP encapsulation to
plain ESP. This is needed when moving from a NATed path to a non-NATed
one, for example when switching from IPv4+NAT to IPv6.

Only copy the existing encapsulation during migration if the encap
attribute is explicitly provided.

Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 net/xfrm/xfrm_state.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 5ebb9f53956e..e5e8342a4e0a 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2009,14 +2009,8 @@ static struct xfrm_state *xfrm_state_clone_and_setup(struct xfrm_state *orig,
 	}
 	x->props.calgo = orig->props.calgo;
 
-	if (encap || orig->encap) {
-		if (encap)
-			x->encap = kmemdup(encap, sizeof(*x->encap),
-					GFP_KERNEL);
-		else
-			x->encap = kmemdup(orig->encap, sizeof(*x->encap),
-					GFP_KERNEL);
-
+	if (encap) {
+		x->encap = kmemdup(encap, sizeof(*x->encap), GFP_KERNEL);
 		if (!x->encap)
 			goto error;
 	}
-- 
2.39.5

[PATCH ipsec-next 3/6] xfrm: rename reqid in xfrm_migrate

[Antony Antony]

In prepreation for the following patch rename s/reqid/old_reqid/.

Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 include/net/xfrm.h     |  2 +-
 net/key/af_key.c       | 10 +++++-----
 net/xfrm/xfrm_policy.c |  4 ++--
 net/xfrm/xfrm_state.c  |  6 +++---
 net/xfrm/xfrm_user.c   |  4 ++--
 5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 0a14daaa5dd4..05fa0552523d 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -685,7 +685,7 @@ struct xfrm_migrate {
 	u8			proto;
 	u8			mode;
 	u16			reserved;
-	u32			reqid;
+	u32			old_reqid;
 	u16			old_family;
 	u16			new_family;
 };
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 571200433aa9..7d5cf386654c 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2538,7 +2538,7 @@ static int ipsecrequests_to_migrate(struct sadb_x_ipsecrequest *rq1, int len,
 	if ((mode = pfkey_mode_to_xfrm(rq1->sadb_x_ipsecrequest_mode)) < 0)
 		return -EINVAL;
 	m->mode = mode;
-	m->reqid = rq1->sadb_x_ipsecrequest_reqid;
+	m->old_reqid = rq1->sadb_x_ipsecrequest_reqid;
 
 	return ((int)(rq1->sadb_x_ipsecrequest_len +
 		      rq2->sadb_x_ipsecrequest_len));
@@ -3634,15 +3634,15 @@ static int pfkey_send_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		if (mode < 0)
 			goto err;
 		if (set_ipsecrequest(skb, mp->proto, mode,
-				     (mp->reqid ?  IPSEC_LEVEL_UNIQUE : IPSEC_LEVEL_REQUIRE),
-				     mp->reqid, mp->old_family,
+				     (mp->old_reqid ?  IPSEC_LEVEL_UNIQUE : IPSEC_LEVEL_REQUIRE),
+				     mp->old_reqid, mp->old_family,
 				     &mp->old_saddr, &mp->old_daddr) < 0)
 			goto err;
 
 		/* new ipsecrequest */
 		if (set_ipsecrequest(skb, mp->proto, mode,
-				     (mp->reqid ? IPSEC_LEVEL_UNIQUE : IPSEC_LEVEL_REQUIRE),
-				     mp->reqid, mp->new_family,
+				     (mp->old_reqid ? IPSEC_LEVEL_UNIQUE : IPSEC_LEVEL_REQUIRE),
+				     mp->old_reqid, mp->new_family,
 				     &mp->new_saddr, &mp->new_daddr) < 0)
 			goto err;
 	}
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 62486f866975..854dfc16ed55 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -4530,7 +4530,7 @@ static int migrate_tmpl_match(const struct xfrm_migrate *m, const struct xfrm_tm
 	int match = 0;
 
 	if (t->mode == m->mode && t->id.proto == m->proto &&
-	    (m->reqid == 0 || t->reqid == m->reqid)) {
+	    (m->old_reqid == 0 || t->reqid == m->old_reqid)) {
 		switch (t->mode) {
 		case XFRM_MODE_TUNNEL:
 		case XFRM_MODE_BEET:
@@ -4624,7 +4624,7 @@ static int xfrm_migrate_check(const struct xfrm_migrate *m, int num_migrate,
 				    sizeof(m[i].old_saddr)) &&
 			    m[i].proto == m[j].proto &&
 			    m[i].mode == m[j].mode &&
-			    m[i].reqid == m[j].reqid &&
+			    m[i].old_reqid == m[j].old_reqid &&
 			    m[i].old_family == m[j].old_family) {
 				NL_SET_ERR_MSG(extack, "Entries in the MIGRATE attribute's list must be unique");
 				return -EINVAL;
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index e5e8342a4e0a..ef832ce477b6 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2081,14 +2081,14 @@ struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *n
 
 	spin_lock_bh(&net->xfrm.xfrm_state_lock);
 
-	if (m->reqid) {
+	if (m->old_reqid) {
 		h = xfrm_dst_hash(net, &m->old_daddr, &m->old_saddr,
-				  m->reqid, m->old_family);
+				  m->old_reqid, m->old_family);
 		hlist_for_each_entry(x, net->xfrm.state_bydst+h, bydst) {
 			if (x->props.mode != m->mode ||
 			    x->id.proto != m->proto)
 				continue;
-			if (m->reqid && x->props.reqid != m->reqid)
+			if (m->old_reqid && x->props.reqid != m->old_reqid)
 				continue;
 			if (if_id != 0 && x->if_id != if_id)
 				continue;
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 403b5ecac2c5..26b82d94acc1 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -3087,7 +3087,7 @@ static int copy_from_user_migrate(struct xfrm_migrate *ma,
 
 		ma->proto = um->proto;
 		ma->mode = um->mode;
-		ma->reqid = um->reqid;
+		ma->old_reqid = um->reqid;
 
 		ma->old_family = um->old_family;
 		ma->new_family = um->new_family;
@@ -3170,7 +3170,7 @@ static int copy_to_user_migrate(const struct xfrm_migrate *m, struct sk_buff *sk
 	memset(&um, 0, sizeof(um));
 	um.proto = m->proto;
 	um.mode = m->mode;
-	um.reqid = m->reqid;
+	um.reqid = m->old_reqid;
 	um.old_family = m->old_family;
 	memcpy(&um.old_daddr, &m->old_daddr, sizeof(um.old_daddr));
 	memcpy(&um.old_saddr, &m->old_saddr, sizeof(um.old_saddr));
-- 
2.39.5

[PATCH ipsec-next 4/6] xfrm: add XFRM_MSG_MIGRATE_STATE for single SA migration

[Antony Antony]

Add a new netlink method to migrate a single xfrm_state.
Unlike the existing migration mechanism (SA + policy), this
supports migrating only the SA and allows changing the reqid.

Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 include/net/xfrm.h          |   1 +
 include/uapi/linux/xfrm.h   |  11 +++
 net/xfrm/xfrm_state.c       |  21 +++--
 net/xfrm/xfrm_user.c        | 159 ++++++++++++++++++++++++++++++++++++
 security/selinux/nlmsgtab.c |   3 +-
 5 files changed, 187 insertions(+), 8 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 05fa0552523d..4147c5ba6093 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -686,6 +686,7 @@ struct xfrm_migrate {
 	u8			mode;
 	u16			reserved;
 	u32			old_reqid;
+	u32			new_reqid;
 	u16			old_family;
 	u16			new_family;
 };
diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index a23495c0e0a1..60b1f201b237 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -227,6 +227,9 @@ enum {
 #define XFRM_MSG_SETDEFAULT XFRM_MSG_SETDEFAULT
 	XFRM_MSG_GETDEFAULT,
 #define XFRM_MSG_GETDEFAULT XFRM_MSG_GETDEFAULT
+
+	XFRM_MSG_MIGRATE_STATE,
+#define XFRM_MSG_MIGRATE_STATE XFRM_MSG_MIGRATE_STATE
 	__XFRM_MSG_MAX
 };
 #define XFRM_MSG_MAX (__XFRM_MSG_MAX - 1)
@@ -507,6 +510,14 @@ struct xfrm_user_migrate {
 	__u16				new_family;
 };
 
+struct xfrm_user_migrate_state {
+	struct xfrm_usersa_id id;
+	xfrm_address_t new_saddr;
+	xfrm_address_t new_daddr;
+	__u16 new_family;
+	__u32 new_reqid;
+};
+
 struct xfrm_user_mapping {
 	struct xfrm_usersa_id		id;
 	__u32				reqid;
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index ef832ce477b6..04c893e42bc1 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1967,7 +1967,8 @@ static inline int clone_security(struct xfrm_state *x, struct xfrm_sec_ctx *secu
 
 static struct xfrm_state *xfrm_state_clone_and_setup(struct xfrm_state *orig,
 					   struct xfrm_encap_tmpl *encap,
-					   struct xfrm_migrate *m)
+					   struct xfrm_migrate *m,
+					   struct netlink_ext_ack *extack)
 {
 	struct net *net = xs_net(orig);
 	struct xfrm_state *x = xfrm_state_alloc(net);
@@ -1979,9 +1980,13 @@ static struct xfrm_state *xfrm_state_clone_and_setup(struct xfrm_state *orig,
 	memcpy(&x->lft, &orig->lft, sizeof(x->lft));
 	x->props.mode = orig->props.mode;
 	x->props.replay_window = orig->props.replay_window;
-	x->props.reqid = orig->props.reqid;
 	x->props.saddr = orig->props.saddr;
 
+	if (orig->props.reqid != m->new_reqid)
+		x->props.reqid = m->new_reqid;
+	else
+		x->props.reqid = orig->props.reqid;
+
 	if (orig->aalg) {
 		x->aalg = xfrm_algo_auth_clone(orig->aalg);
 		if (!x->aalg)
@@ -2059,7 +2064,6 @@ static struct xfrm_state *xfrm_state_clone_and_setup(struct xfrm_state *orig,
 			goto error;
 	}
 
-
 	x->props.family = m->new_family;
 	memcpy(&x->id.daddr, &m->new_daddr, sizeof(x->id.daddr));
 	memcpy(&x->props.saddr, &m->new_saddr, sizeof(x->props.saddr));
@@ -2134,7 +2138,7 @@ struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
 {
 	struct xfrm_state *xc;
 
-	xc = xfrm_state_clone_and_setup(x, encap, m);
+	xc = xfrm_state_clone_and_setup(x, encap, m, extack);
 	if (!xc)
 		return NULL;
 
@@ -2146,9 +2150,12 @@ struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
 		goto error;
 
 	/* add state */
-	if (xfrm_addr_equal(&x->id.daddr, &m->new_daddr, m->new_family)) {
-		/* a care is needed when the destination address of the
-		   state is to be updated as it is a part of triplet */
+	if (xfrm_addr_equal(&x->id.daddr, &m->new_daddr, m->new_family) ||
+	    x->props.reqid != xc->props.reqid) {
+		/*
+		 * a care is needed when the destination address or the reqid
+		 * of the state is to be updated as it is a part of triplet
+		 */
 		xfrm_state_insert(xc);
 	} else {
 		if (xfrm_state_add(xc) < 0)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 26b82d94acc1..358044fc2376 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -3052,6 +3052,22 @@ static int xfrm_add_acquire(struct sk_buff *skb, struct nlmsghdr *nlh,
 }
 
 #ifdef CONFIG_XFRM_MIGRATE
+static int copy_from_user_migrate_state(struct xfrm_migrate *ma,
+					struct xfrm_user_migrate_state *um)
+{
+	memcpy(&ma->old_daddr, &um->id.daddr, sizeof(ma->old_daddr));
+	memcpy(&ma->new_daddr, &um->new_daddr, sizeof(ma->new_daddr));
+	memcpy(&ma->new_saddr, &um->new_saddr, sizeof(ma->new_saddr));
+
+	ma->proto = um->id.proto;
+	ma->new_reqid = um->new_reqid;
+
+	ma->old_family = um->id.family;
+	ma->new_family = um->new_family;
+
+	return 0;
+}
+
 static int copy_from_user_migrate(struct xfrm_migrate *ma,
 				  struct xfrm_kmaddress *k,
 				  struct nlattr **attrs, int *num,
@@ -3154,7 +3170,148 @@ static int xfrm_do_migrate(struct sk_buff *skb, struct nlmsghdr *nlh,
 	kfree(xuo);
 	return err;
 }
+
+static int build_migrate_state(struct sk_buff *skb,
+			       const struct xfrm_user_migrate_state *m,
+			       const struct xfrm_encap_tmpl *encap,
+			       const struct xfrm_user_offload *xuo)
+{
+	int err;
+	struct nlmsghdr *nlh;
+	struct xfrm_user_migrate_state *um;
+
+	nlh = nlmsg_put(skb, 0, 0, XFRM_MSG_MIGRATE_STATE,
+			sizeof(struct xfrm_user_migrate_state), 0);
+	if (!nlh)
+		return -EMSGSIZE;
+
+	um = nlmsg_data(nlh);
+	memset(um, 0, sizeof(*um));
+	memcpy(um, m, sizeof(*um));
+
+	if (encap) {
+		err = nla_put(skb, XFRMA_ENCAP, sizeof(*encap), encap);
+		if (err)
+			goto out_cancel;
+	}
+
+	if (xuo) {
+		err = nla_put(skb, XFRMA_OFFLOAD_DEV, sizeof(*xuo), xuo);
+		if (err)
+			goto out_cancel;
+	}
+
+	nlmsg_end(skb, nlh);
+	return 0;
+
+out_cancel:
+	nlmsg_cancel(skb, nlh);
+	return err;
+}
+
+static inline unsigned int xfrm_migrate_state_msgsize(bool with_encap, bool with_xuo)
+{
+	return NLMSG_ALIGN(sizeof(struct xfrm_user_migrate_state)) +
+		(with_encap ? nla_total_size(sizeof(struct xfrm_encap_tmpl)) : 0) +
+		(with_xuo ? nla_total_size(sizeof(struct xfrm_user_offload)) : 0);
+}
+
+static int xfrm_send_migrate_state(const struct xfrm_user_migrate_state *um,
+				   const struct xfrm_encap_tmpl *encap,
+				   const struct xfrm_user_offload *xuo)
+{
+	int err;
+	struct sk_buff *skb;
+	struct net *net = &init_net;
+
+	skb = nlmsg_new(xfrm_migrate_state_msgsize(!!encap, !!xuo), GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	err = build_migrate_state(skb, um, encap, xuo);
+	if (err < 0) {
+		WARN_ON(1);
+		return err;
+	}
+
+	return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_MIGRATE);
+}
+
+static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
+				 struct nlattr **attrs, struct netlink_ext_ack *extack)
+{
+	int err = -ESRCH;
+	struct xfrm_state *x;
+	struct xfrm_encap_tmpl *encap = NULL;
+	struct xfrm_user_offload *xuo = NULL;
+	struct xfrm_migrate m  = { .old_saddr.a4 = 0,};
+	struct net *net = sock_net(skb->sk);
+	struct xfrm_user_migrate_state *um = nlmsg_data(nlh);
+
+	if (!um->id.spi)  {
+		NL_SET_ERR_MSG(extack, "Invalid SPI 0x0");
+		return -EINVAL;
+	}
+
+	err = copy_from_user_migrate_state(&m, um);
+	if (err)
+		return err;
+
+	x = xfrm_user_state_lookup(net, &um->id, attrs, &err);
+
+	if (x) {
+		struct xfrm_state *xc;
+
+		if (!x->dir) {
+			NL_SET_ERR_MSG(extack, "State direction is invalid");
+			err = -EINVAL;
+			goto error;
+		}
+
+		if (attrs[XFRMA_ENCAP]) {
+			encap = kmemdup(nla_data(attrs[XFRMA_ENCAP]),
+					sizeof(*encap), GFP_KERNEL);
+			if (!encap) {
+				err = -ENOMEM;
+				goto error;
+			}
+		}
+		if (attrs[XFRMA_OFFLOAD_DEV]) {
+			xuo = kmemdup(nla_data(attrs[XFRMA_OFFLOAD_DEV]),
+				      sizeof(*xuo), GFP_KERNEL);
+			if (!xuo) {
+				err = -ENOMEM;
+				goto error;
+			}
+		}
+		xc = xfrm_state_migrate(x, &m, encap, net, xuo, extack);
+		if (xc) {
+			xfrm_state_delete(x);
+			xfrm_send_migrate_state(um, encap, xuo);
+		} else {
+			if (extack && !extack->_msg)
+				NL_SET_ERR_MSG(extack, "State migration clone failed");
+			err = -EINVAL;
+		}
+	} else {
+		NL_SET_ERR_MSG(extack, "Can not find state");
+		return err;
+	}
+error:
+	xfrm_state_put(x);
+	kfree(encap);
+	kfree(xuo);
+	return err;
+}
+
 #else
+static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
+				 struct nlattr **attrs, struct netlink_ext_ack *extack)
+{
+	NL_SET_ERR_MSG(extack, "XFRM_MSG_MIGRATE_STATE is not supported");
+	return -ENOPROTOOPT;
+}
+
 static int xfrm_do_migrate(struct sk_buff *skb, struct nlmsghdr *nlh,
 			   struct nlattr **attrs, struct netlink_ext_ack *extack)
 {
@@ -3307,6 +3464,7 @@ const int xfrm_msg_min[XFRM_NR_MSGTYPES] = {
 	[XFRM_MSG_GETSPDINFO  - XFRM_MSG_BASE] = sizeof(u32),
 	[XFRM_MSG_SETDEFAULT  - XFRM_MSG_BASE] = XMSGSIZE(xfrm_userpolicy_default),
 	[XFRM_MSG_GETDEFAULT  - XFRM_MSG_BASE] = XMSGSIZE(xfrm_userpolicy_default),
+	[XFRM_MSG_MIGRATE_STATE - XFRM_MSG_BASE] = XMSGSIZE(xfrm_user_migrate_state),
 };
 EXPORT_SYMBOL_GPL(xfrm_msg_min);
 
@@ -3400,6 +3558,7 @@ static const struct xfrm_link {
 	[XFRM_MSG_GETSPDINFO  - XFRM_MSG_BASE] = { .doit = xfrm_get_spdinfo   },
 	[XFRM_MSG_SETDEFAULT  - XFRM_MSG_BASE] = { .doit = xfrm_set_default   },
 	[XFRM_MSG_GETDEFAULT  - XFRM_MSG_BASE] = { .doit = xfrm_get_default   },
+	[XFRM_MSG_MIGRATE_STATE - XFRM_MSG_BASE] = { .doit = xfrm_do_migrate_state },
 };
 
 static int xfrm_reject_unused_attr(int type, struct nlattr **attrs,
diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
index 2c0b07f9fbbd..9cec74c317f0 100644
--- a/security/selinux/nlmsgtab.c
+++ b/security/selinux/nlmsgtab.c
@@ -128,6 +128,7 @@ static const struct nlmsg_perm nlmsg_xfrm_perms[] = {
 	{ XFRM_MSG_MAPPING, NETLINK_XFRM_SOCKET__NLMSG_READ },
 	{ XFRM_MSG_SETDEFAULT, NETLINK_XFRM_SOCKET__NLMSG_WRITE },
 	{ XFRM_MSG_GETDEFAULT, NETLINK_XFRM_SOCKET__NLMSG_READ },
+	{ XFRM_MSG_MIGRATE_STATE, NETLINK_XFRM_SOCKET__NLMSG_WRITE  },
 };
 
 static const struct nlmsg_perm nlmsg_audit_perms[] = {
@@ -203,7 +204,7 @@ int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm)
 		 * structures at the top of this file with the new mappings
 		 * before updating the BUILD_BUG_ON() macro!
 		 */
-		BUILD_BUG_ON(XFRM_MSG_MAX != XFRM_MSG_GETDEFAULT);
+		BUILD_BUG_ON(XFRM_MSG_MAX != XFRM_MSG_MIGRATE_STATE);
 
 		if (selinux_policycap_netlink_xperm()) {
 			*perm = NETLINK_XFRM_SOCKET__NLMSG;
-- 
2.39.5

[PATCH ipsec-next 5/6] xfrm: reqid is invarient in old migration

[Antony Antony]

During the XFRM_MSG_MIGRATE the reqid remains invariant.

Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 net/xfrm/xfrm_user.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 358044fc2376..13364d45a7b6 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -3104,6 +3104,7 @@ static int copy_from_user_migrate(struct xfrm_migrate *ma,
 		ma->proto = um->proto;
 		ma->mode = um->mode;
 		ma->old_reqid = um->reqid;
+		ma->new_reqid = um->reqid; /* reqid is invariant in XFRM_MSG_MIGRATE */
 
 		ma->old_family = um->old_family;
 		ma->new_family = um->new_family;
-- 
2.39.5

[PATCH ipsec-next 6/6] xfrm: check that SA is in VALID state before use

[Antony Antony]

During migration, an SA may be deleted or become invalid while skb(s)
still hold a reference to it. Using such an SA in the output path may
result in IV reuse with AES-GCM.

Reject use of states that are not in XFRM_STATE_VALID.
This applies to both output and input data paths.

The check is performed in xfrm_state_check_expire(), which is called
from xfrm_output_one().

Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 net/xfrm/xfrm_state.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 04c893e42bc1..ca628262087f 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2278,6 +2278,9 @@ EXPORT_SYMBOL(xfrm_state_update);
 
 int xfrm_state_check_expire(struct xfrm_state *x)
 {
+	if (x->km.state != XFRM_STATE_VALID)
+		return -EINVAL;
+
 	/* All counters which are needed to decide if state is expired
 	 * are handled by SW for non-packet offload modes. Simply skip
 	 * the following update and save extra boilerplate in drivers.
-- 
2.39.5

Re: [PATCH ipsec-next 4/6] xfrm: add XFRM_MSG_MIGRATE_STATE for single SA migration

[Simon Horman]

On Fri, Jan 09, 2026 at 02:38:05PM +0100, Antony Antony wrote:
> Add a new netlink method to migrate a single xfrm_state.
> Unlike the existing migration mechanism (SA + policy), this
> supports migrating only the SA and allows changing the reqid.
> 
> Signed-off-by: Antony Antony <antony.antony@secunet.com>

...

> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index ef832ce477b6..04c893e42bc1 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -1967,7 +1967,8 @@ static inline int clone_security(struct xfrm_state *x, struct xfrm_sec_ctx *secu
>  
>  static struct xfrm_state *xfrm_state_clone_and_setup(struct xfrm_state *orig,
>  					   struct xfrm_encap_tmpl *encap,
> -					   struct xfrm_migrate *m)
> +					   struct xfrm_migrate *m,

Hi Antony,

Not strictly related to this patch, but FWIIW, it seems that m could be
const in this call stack.  And, moreover, I think there would be some value
in constifying parameters throughout xfrm.

> +					   struct netlink_ext_ack *extack)
>  {
>  	struct net *net = xs_net(orig);
>  	struct xfrm_state *x = xfrm_state_alloc(net);
> @@ -1979,9 +1980,13 @@ static struct xfrm_state *xfrm_state_clone_and_setup(struct xfrm_state *orig,
>  	memcpy(&x->lft, &orig->lft, sizeof(x->lft));
>  	x->props.mode = orig->props.mode;
>  	x->props.replay_window = orig->props.replay_window;
> -	x->props.reqid = orig->props.reqid;
>  	x->props.saddr = orig->props.saddr;
>  
> +	if (orig->props.reqid != m->new_reqid)
> +		x->props.reqid = m->new_reqid;
> +	else
> +		x->props.reqid = orig->props.reqid;
> +

Claude Code with Review Prompts [1] flags that until the next
patch of this series m->new_reqid is used uninitialised in the following
call stack:

xfrm_do_migrate -> xfrm_migrate -> xfrm_state_migrate -> xfrm_state_clone_and_setup

Also, while I could have missed something, it seems to me that it is
also uninitialised in this call stack:

pfkey_migrate -> xfrm_migrate -> xfrm_state_migrate -> xfrm_state_clone_and_setup

[1] https://github.com/masoncl/review-prompts/

>  	if (orig->aalg) {
>  		x->aalg = xfrm_algo_auth_clone(orig->aalg);
>  		if (!x->aalg)
> @@ -2059,7 +2064,6 @@ static struct xfrm_state *xfrm_state_clone_and_setup(struct xfrm_state *orig,
>  			goto error;
>  	}
>  
> -

nit: this hunk doesn't seem related to the rest of the patch.

>  	x->props.family = m->new_family;
>  	memcpy(&x->id.daddr, &m->new_daddr, sizeof(x->id.daddr));
>  	memcpy(&x->props.saddr, &m->new_saddr, sizeof(x->props.saddr));

...

> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c

...

> +static inline unsigned int xfrm_migrate_state_msgsize(bool with_encap, bool with_xuo)

Please don't use the inline keyword in .c files unless there is a
demonstrable - usually performance - reason to do so.
Rather, please let the compiler inline (or not) code as it sees fit.

> +{
> +	return NLMSG_ALIGN(sizeof(struct xfrm_user_migrate_state)) +
> +		(with_encap ? nla_total_size(sizeof(struct xfrm_encap_tmpl)) : 0) +
> +		(with_xuo ? nla_total_size(sizeof(struct xfrm_user_offload)) : 0);
> +}
> +

...

> +static int xfrm_send_migrate_state(const struct xfrm_user_migrate_state *um,
> +				   const struct xfrm_encap_tmpl *encap,
> +				   const struct xfrm_user_offload *xuo)
> +{
> +	int err;
> +	struct sk_buff *skb;
> +	struct net *net = &init_net;
> +
> +	skb = nlmsg_new(xfrm_migrate_state_msgsize(!!encap, !!xuo), GFP_ATOMIC);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	err = build_migrate_state(skb, um, encap, xuo);
> +	if (err < 0) {
> +		WARN_ON(1);
> +		return err;

skb seems to be leaked here.

Also flagged by Review Prompts.

> +	}
> +
> +	return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_MIGRATE);
> +}

...

Re: [PATCH ipsec-next 1/6] xfrm: remove redundent assignment

[Simon Horman]

On Fri, Jan 09, 2026 at 02:37:08PM +0100, Antony Antony wrote:
> this assignmet is overwritten within the same function further down

nit: This assignment...

Also, redundant is misspelt in the subject.
> 
> 80c9abaabf42 ("[XFRM]: Extension for dynamic update of endpoint address(es)")
> 
> x->props.family = m->new_family;
> 
> e03c3bba351f ("xfrm: Fix xfrm migrate issues when address family changes")

And your Signed-off-by line is missing.

...

Re: [devel-ipsec] Re: [PATCH ipsec-next 4/6] xfrm: add XFRM_MSG_MIGRATE_STATE for single SA migration

[Antony Antony]

Hi Simon,

On Tue, Jan 13, 2026 at 02:57:16PM +0000, Simon Horman via Devel wrote:
> On Fri, Jan 09, 2026 at 02:38:05PM +0100, Antony Antony wrote:
> > Add a new netlink method to migrate a single xfrm_state.
> > Unlike the existing migration mechanism (SA + policy), this
> > supports migrating only the SA and allows changing the reqid.
> > 
> > Signed-off-by: Antony Antony <antony.antony@secunet.com>
> 
> ...
> 
> > diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> > index ef832ce477b6..04c893e42bc1 100644
> > --- a/net/xfrm/xfrm_state.c
> > +++ b/net/xfrm/xfrm_state.c
> > @@ -1967,7 +1967,8 @@ static inline int clone_security(struct xfrm_state *x, struct xfrm_sec_ctx *secu
> >  
> >  static struct xfrm_state *xfrm_state_clone_and_setup(struct xfrm_state *orig,
> >  					   struct xfrm_encap_tmpl *encap,
> > -					   struct xfrm_migrate *m)
> > +					   struct xfrm_migrate *m,
> 
> Hi Antony,
> 
> Not strictly related to this patch, but FWIIW, it seems that m could be
> const in this call stack.  And, moreover, I think there would be some value
> in constifying parameters throughout xfrm.

thanks. It is good advise. I sprinkled a couple of const.

> 
> > +					   struct netlink_ext_ack *extack)
> >  {
> >  	struct net *net = xs_net(orig);
> >  	struct xfrm_state *x = xfrm_state_alloc(net);
> > @@ -1979,9 +1980,13 @@ static struct xfrm_state *xfrm_state_clone_and_setup(struct xfrm_state *orig,
> >  	memcpy(&x->lft, &orig->lft, sizeof(x->lft));
> >  	x->props.mode = orig->props.mode;
> >  	x->props.replay_window = orig->props.replay_window;
> > -	x->props.reqid = orig->props.reqid;
> >  	x->props.saddr = orig->props.saddr;
> >  
> > +	if (orig->props.reqid != m->new_reqid)
> > +		x->props.reqid = m->new_reqid;
> > +	else
> > +		x->props.reqid = orig->props.reqid;
> > +
> 
> Claude Code with Review Prompts [1] flags that until the next
> patch of this series m->new_reqid is used uninitialised in the following
> call stack:
> 
> xfrm_do_migrate -> xfrm_migrate -> xfrm_state_migrate -> xfrm_state_clone_and_setup
> 
> Also, while I could have missed something, it seems to me that it is
> also uninitialised in this call stack:
> 
> pfkey_migrate -> xfrm_migrate -> xfrm_state_migrate -> xfrm_state_clone_and_setup

thanks. I fxied this by squashing the next patch to this one.
 
> [1] https://github.com/masoncl/review-prompts/

thanks! that looks interesting.

> 
> >  	if (orig->aalg) {
> >  		x->aalg = xfrm_algo_auth_clone(orig->aalg);
> >  		if (!x->aalg)
> > @@ -2059,7 +2064,6 @@ static struct xfrm_state *xfrm_state_clone_and_setup(struct xfrm_state *orig,
> >  			goto error;
> >  	}
> >  
> > -
> 
> nit: this hunk doesn't seem related to the rest of the patch.

fixed.

> 
> >  	x->props.family = m->new_family;
> >  	memcpy(&x->id.daddr, &m->new_daddr, sizeof(x->id.daddr));
> >  	memcpy(&x->props.saddr, &m->new_saddr, sizeof(x->props.saddr));
> 
> ...
> 
> > diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> 
> ...
> 
> > +static inline unsigned int xfrm_migrate_state_msgsize(bool with_encap, bool with_xuo)
> 
> Please don't use the inline keyword in .c files unless there is a
> demonstrable - usually performance - reason to do so.
> Rather, please let the compiler inline (or not) code as it sees fit.

removed

> 
> > +{
> > +	return NLMSG_ALIGN(sizeof(struct xfrm_user_migrate_state)) +
> > +		(with_encap ? nla_total_size(sizeof(struct xfrm_encap_tmpl)) : 0) +
> > +		(with_xuo ? nla_total_size(sizeof(struct xfrm_user_offload)) : 0);
> > +}
> > +
> 
> ...
> 
> > +static int xfrm_send_migrate_state(const struct xfrm_user_migrate_state *um,
> > +				   const struct xfrm_encap_tmpl *encap,
> > +				   const struct xfrm_user_offload *xuo)
> > +{
> > +	int err;
> > +	struct sk_buff *skb;
> > +	struct net *net = &init_net;
> > +
> > +	skb = nlmsg_new(xfrm_migrate_state_msgsize(!!encap, !!xuo), GFP_ATOMIC);
> > +	if (!skb)
> > +		return -ENOMEM;
> > +
> > +	err = build_migrate_state(skb, um, encap, xuo);
> > +	if (err < 0) {
> > +		WARN_ON(1);
> > +		return err;
> 
> skb seems to be leaked here.
> 
> Also flagged by Review Prompts.

I don't see a skb leak. It also looks similar to the functions above.

> 
> > +	}
> > +
> > +	return xfrm_nlmsg_multicast(net, skb, 0, XFRMNLGRP_MIGRATE);
> > +}

I will send a new v2.

regards
-antony

Re: [devel-ipsec] Re: [PATCH ipsec-next 4/6] xfrm: add XFRM_MSG_MIGRATE_STATE for single SA migration

[Simon Horman]

On Wed, Jan 14, 2026 at 05:09:20PM +0100, Antony Antony wrote:

Hi Antony,

> Hi Simon,
> 
> On Tue, Jan 13, 2026 at 02:57:16PM +0000, Simon Horman via Devel wrote:
> > On Fri, Jan 09, 2026 at 02:38:05PM +0100, Antony Antony wrote:

...

> > > +static int xfrm_send_migrate_state(const struct xfrm_user_migrate_state *um,
> > > +				   const struct xfrm_encap_tmpl *encap,
> > > +				   const struct xfrm_user_offload *xuo)
> > > +{
> > > +	int err;
> > > +	struct sk_buff *skb;
> > > +	struct net *net = &init_net;
> > > +
> > > +	skb = nlmsg_new(xfrm_migrate_state_msgsize(!!encap, !!xuo), GFP_ATOMIC);
> > > +	if (!skb)
> > > +		return -ENOMEM;
> > > +
> > > +	err = build_migrate_state(skb, um, encap, xuo);
> > > +	if (err < 0) {
> > > +		WARN_ON(1);
> > > +		return err;
> > 
> > skb seems to be leaked here.
> > 
> > Also flagged by Review Prompts.
> 
> I don't see a skb leak. It also looks similar to the functions above.

xfrm_get_ae() is the previous caller of nlmsg_new() in this file.
It calls BUG_ON() on error, so leaking is not an issue there.

The caller before that is xfrm_get_default() which calls kfree_skb() in
it's error path. Maybe I'm missing something obvious, but I was thinking
that approach is appropriate here too.

...

Re: [devel-ipsec] Re: [PATCH ipsec-next 4/6] xfrm: add XFRM_MSG_MIGRATE_STATE for single SA migration

[Antony Antony]

On Thu, Jan 15, 2026 at 01:44:50PM +0000, Simon Horman via Devel wrote:
> On Wed, Jan 14, 2026 at 05:09:20PM +0100, Antony Antony wrote:
> 
> Hi Antony,
> 
> > Hi Simon,
> > 
> > On Tue, Jan 13, 2026 at 02:57:16PM +0000, Simon Horman via Devel wrote:
> > > On Fri, Jan 09, 2026 at 02:38:05PM +0100, Antony Antony wrote:
> 
> ...
> 
> > > > +static int xfrm_send_migrate_state(const struct xfrm_user_migrate_state *um,
> > > > +				   const struct xfrm_encap_tmpl *encap,
> > > > +				   const struct xfrm_user_offload *xuo)
> > > > +{
> > > > +	int err;
> > > > +	struct sk_buff *skb;
> > > > +	struct net *net = &init_net;
> > > > +
> > > > +	skb = nlmsg_new(xfrm_migrate_state_msgsize(!!encap, !!xuo), GFP_ATOMIC);
> > > > +	if (!skb)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	err = build_migrate_state(skb, um, encap, xuo);
> > > > +	if (err < 0) {
> > > > +		WARN_ON(1);

kfree_skb(skb); replace the above line; explained bellow

> > > > +		return err;
> > > 
> > > skb seems to be leaked here.
> > > 
> > > Also flagged by Review Prompts.
> > 
> > I don't see a skb leak. It also looks similar to the functions above.
> 
> xfrm_get_ae() is the previous caller of nlmsg_new() in this file.
> It calls BUG_ON() on error, so leaking is not an issue there.
> 
> The caller before that is xfrm_get_default() which calls kfree_skb() in
> it's error path. Maybe I'm missing something obvious, but I was thinking
> that approach is appropriate here too.

You’re right. There is a leak in the error path.

The new helper I added is similar to build_migrate(), but that code uses
BUG_ON() on the error path. That feels too extreme here (even though there
are other instances of it in the same file).

I’ll follow the pattern in xfrm_get_default(): handle the error by freeing
the skb (kfree_skb()) and returning an error. And no WARN_ON().

I’ll send v3 shortly.

thanks,
-antony

Re: [devel-ipsec] Re: [PATCH ipsec-next 4/6] xfrm: add XFRM_MSG_MIGRATE_STATE for single SA migration

[Simon Horman]

On Fri, Jan 16, 2026 at 12:02:12PM +0100, Antony Antony wrote:
> On Thu, Jan 15, 2026 at 01:44:50PM +0000, Simon Horman via Devel wrote:
> > On Wed, Jan 14, 2026 at 05:09:20PM +0100, Antony Antony wrote:
> > 
> > Hi Antony,
> > 
> > > Hi Simon,
> > > 
> > > On Tue, Jan 13, 2026 at 02:57:16PM +0000, Simon Horman via Devel wrote:
> > > > On Fri, Jan 09, 2026 at 02:38:05PM +0100, Antony Antony wrote:
> > 
> > ...
> > 
> > > > > +static int xfrm_send_migrate_state(const struct xfrm_user_migrate_state *um,
> > > > > +				   const struct xfrm_encap_tmpl *encap,
> > > > > +				   const struct xfrm_user_offload *xuo)
> > > > > +{
> > > > > +	int err;
> > > > > +	struct sk_buff *skb;
> > > > > +	struct net *net = &init_net;
> > > > > +
> > > > > +	skb = nlmsg_new(xfrm_migrate_state_msgsize(!!encap, !!xuo), GFP_ATOMIC);
> > > > > +	if (!skb)
> > > > > +		return -ENOMEM;
> > > > > +
> > > > > +	err = build_migrate_state(skb, um, encap, xuo);
> > > > > +	if (err < 0) {
> > > > > +		WARN_ON(1);
> 
> kfree_skb(skb); replace the above line; explained bellow
> 
> > > > > +		return err;
> > > > 
> > > > skb seems to be leaked here.
> > > > 
> > > > Also flagged by Review Prompts.
> > > 
> > > I don't see a skb leak. It also looks similar to the functions above.
> > 
> > xfrm_get_ae() is the previous caller of nlmsg_new() in this file.
> > It calls BUG_ON() on error, so leaking is not an issue there.
> > 
> > The caller before that is xfrm_get_default() which calls kfree_skb() in
> > it's error path. Maybe I'm missing something obvious, but I was thinking
> > that approach is appropriate here too.
> 
> You’re right. There is a leak in the error path.
> 
> The new helper I added is similar to build_migrate(), but that code uses
> BUG_ON() on the error path. That feels too extreme here (even though there
> are other instances of it in the same file).

FWIIW, the use of BUG_ON in xfrm_get_ae() did give me pause for thought.

> 
> I’ll follow the pattern in xfrm_get_default(): handle the error by freeing
> the skb (kfree_skb()) and returning an error. And no WARN_ON().
> 
> I’ll send v3 shortly.

Thanks!