[PATCH ipsec-next v4 1/5] xfrm: add missing __rcu annotation to nlsk

[PATCH ipsec-next v4 1/5] xfrm: add missing __rcu annotation to nlsk

[Antony Antony]

The nlsk field in struct netns_xfrm is RCU-protected, as seen by
the use of rcu_assign_pointer() and RCU_INIT_POINTER() when updating
it.
However, the field lacks the __rcu annotation, and most read-side
accesses don't use rcu_dereference().

Add the missing __rcu annotation and convert all read-side accesses to
use rcu_dereference() for correctness and to silence sparse warnings.

Sparse warning reported by NIPA allmodconfig test when modifying
net/xfrm/xfrm_user.c. The warning is a pre-existing issue in
xfrm_nlmsg_multicast(). This series added a new call to this function
and NIPA testing reported a new warning was added by this series.

To reproduce (requires sparse):
make C=2 net/xfrm/
net/xfrm/xfrm_user.c:1574:29: error: incompatible types in comparison
expression (different address spaces):
net/xfrm/xfrm_user.c:1574:29:    struct sock [noderef] __rcu *
net/xfrm/xfrm_user.c:1574:29:    struct sock *

Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
 include/net/netns/xfrm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
index 23dd647fe024..ed8eee81bbb0 100644
--- a/include/net/netns/xfrm.h
+++ b/include/net/netns/xfrm.h
@@ -59,7 +59,7 @@ struct netns_xfrm {
 	struct list_head	inexact_bins;


-	struct sock		*nlsk;
+	struct sock __rcu	*nlsk;
 	struct sock		*nlsk_stash;

 	u32			sysctl_aevent_etime;
--
2.39.5

[PATCH ipsec-next v4 2/5] xfrm: remove redundant assignments

[Antony Antony]

This assignments are overwritten within the same function further down

commit e8961c50ee9cc ("xfrm: Refactor migration setup
during the cloning process")
x->props.family = m->new_family;

Which actually moved it in the
commit e03c3bba351f9 ("xfrm: Fix xfrm migrate issues
when address family changes")

And the initial
commit 80c9abaabf428 ("[XFRM]: Extension for dynamic
update of endpoint address(es)")

added x->props.saddr = orig->props.saddr; and
memcpy(&xc->props.saddr, &m->new_saddr, sizeof(xc->props.saddr));

Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
v1->v2: remove extra saddr copy, previous line
---
 net/xfrm/xfrm_state.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 9e14e453b55c..4fd73a970a7a 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1980,8 +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) {
 		x->aalg = xfrm_algo_auth_clone(orig->aalg);
--
2.39.5

[PATCH ipsec-next v4 3/5] 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.

Note: PF_KEY's SADB_X_MIGRATE always passes encap=NULL and never
supported encapsulation in migration. PF_KEY is deprecated and was
in feature freeze when UDP encapsulation was added to xfrm.

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 4fd73a970a7a..f5f699f5f98e 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2008,14 +2008,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 v4 4/5] xfrm: rename reqid in xfrm_migrate

[Antony Antony]

In preparation 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..a856bdd0c0d7 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 f5f699f5f98e..fe595d7f4398 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2080,14 +2080,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 v4 5/5] 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.

The reqid is invariant in old migration.

Signed-off-by: Antony Antony <antony.antony@secunet.com>
---
v1->v2: merged next patch here to fix use uninitialized value
	- removed unnecessary inline
        - added const when possible

v2->v3: free the skb on the error path

v3->v4: preserve reqid invariant for each state migrated
---
 include/net/xfrm.h          |   1 +
 include/uapi/linux/xfrm.h   |  11 +++
 net/xfrm/xfrm_policy.c      |   1 +
 net/xfrm/xfrm_state.c       |  16 ++--
 net/xfrm/xfrm_user.c        | 159 ++++++++++++++++++++++++++++++++++++
 security/selinux/nlmsgtab.c |   3 +-
 6 files changed, 183 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_policy.c b/net/xfrm/xfrm_policy.c
index 854dfc16ed55..72678053bd69 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -4672,6 +4672,7 @@ int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		if ((x = xfrm_migrate_state_find(mp, net, if_id))) {
 			x_cur[nx_cur] = x;
 			nx_cur++;
+			mp->new_reqid = x->props.reqid; /* reqid is invariant in XFRM_MSG_MIGRATE */
 			xc = xfrm_state_migrate(x, mp, encap, net, xuo, extack);
 			if (xc) {
 				x_new[nx_new] = xc;
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index fe595d7f4398..8d4f82bab8fc 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1966,8 +1966,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)
+					   const struct xfrm_encap_tmpl *encap,
+					   const struct xfrm_migrate *m)
 {
 	struct net *net = xs_net(orig);
 	struct xfrm_state *x = xfrm_state_alloc(net);
@@ -1979,7 +1979,6 @@ 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;

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

-
+	x->props.reqid = m->new_reqid;
 	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));
@@ -2145,9 +2144,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..a7ab43b16e0a 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,
+					const 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);
+	*um = *m;
+
+	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 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) {
+		kfree_skb(skb);
+		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 net *net = sock_net(skb->sk);
+	struct xfrm_encap_tmpl *encap = NULL;
+	struct xfrm_user_offload *xuo = NULL;
+	struct xfrm_migrate m = { .old_saddr.a4 = 0,};
+	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);
+			err = 0;
+		} 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..655d2616c9d2 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

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

[Steffen Klassert]

On Mon, Jan 19, 2026 at 09:54:55AM +0100, Antony Antony wrote:
> +
> +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 net *net = sock_net(skb->sk);
> +	struct xfrm_encap_tmpl *encap = NULL;
> +	struct xfrm_user_offload *xuo = NULL;
> +	struct xfrm_migrate m = { .old_saddr.a4 = 0,};
> +	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);

This xfrm_state_delete() comes too late. You need to synchronize this
with xfrm_state_clone_and_setup() by holding the state lock.
Otherwise we might hand out the same sequence number etc. more than
one time. Unfortunately xfrm_migrate() has the same problem, the
error recovery mechanism in xfrm_migrate() looks broken.

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

[Steffen Klassert]

On Thu, Jan 22, 2026 at 11:33:42AM +0100, Steffen Klassert wrote:
> On Mon, Jan 19, 2026 at 09:54:55AM +0100, Antony Antony wrote:
> > +
> > +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 net *net = sock_net(skb->sk);
> > +	struct xfrm_encap_tmpl *encap = NULL;
> > +	struct xfrm_user_offload *xuo = NULL;
> > +	struct xfrm_migrate m = { .old_saddr.a4 = 0,};
> > +	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);
> 
> This xfrm_state_delete() comes too late. You need to synchronize this
> with xfrm_state_clone_and_setup() by holding the state lock.
> Otherwise we might hand out the same sequence number etc. more than
> one time. Unfortunately xfrm_migrate() has the same problem, the
> error recovery mechanism in xfrm_migrate() looks broken.

The xfrm_migrate() problem is nasty, looks like we can't recover
if the state migration fails. We probably have to delete the
states and wait for the userspace to insert new ones.

Another thing, xfrm_user_state_lookup() returns a reference on x,
you should drop it somewhere.

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

[Antony Antony]

On Thu, Jan 22, 2026 at 11:50:16AM +0100, Steffen Klassert via Devel wrote:
> On Thu, Jan 22, 2026 at 11:33:42AM +0100, Steffen Klassert wrote:
> > On Mon, Jan 19, 2026 at 09:54:55AM +0100, Antony Antony wrote:
> > > +
> > > +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 net *net = sock_net(skb->sk);
> > > +	struct xfrm_encap_tmpl *encap = NULL;
> > > +	struct xfrm_user_offload *xuo = NULL;
> > > +	struct xfrm_migrate m = { .old_saddr.a4 = 0,};
> > > +	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);
> > 
> > This xfrm_state_delete() comes too late. You need to synchronize this
> > with xfrm_state_clone_and_setup() by holding the state lock.
> > Otherwise we might hand out the same sequence number etc. more than
> > one time. Unfortunately xfrm_migrate() has the same problem, the
> > error recovery mechanism in xfrm_migrate() looks broken.
> 
> The xfrm_migrate() problem is nasty, looks like we can't recover
> if the state migration fails. We probably have to delete the
> states and wait for the userspace to insert new ones.

userspace should notice the inconsistany. Most of them are ENOMEM or EMSGSIZE

> 
> Another thing, xfrm_user_state_lookup() returns a reference on x,
> you should drop it somewhere.

There is a call a few lines bellow, the label was "error:"  I renamed it to  
"out:" for clariy.

+out:
+       xfrm_state_put(x);
+       kfree(encap);
+       kfree(xuo);
+       return err;

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

[Steffen Klassert]

On Fri, Jan 23, 2026 at 06:58:22AM +0100, Antony Antony wrote:
> On Thu, Jan 22, 2026 at 11:50:16AM +0100, Steffen Klassert via Devel wrote:
> > On Thu, Jan 22, 2026 at 11:33:42AM +0100, Steffen Klassert wrote:
> > > On Mon, Jan 19, 2026 at 09:54:55AM +0100, Antony Antony wrote:
> > > > +
> > > > +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 net *net = sock_net(skb->sk);
> > > > +	struct xfrm_encap_tmpl *encap = NULL;
> > > > +	struct xfrm_user_offload *xuo = NULL;
> > > > +	struct xfrm_migrate m = { .old_saddr.a4 = 0,};
> > > > +	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);
> > > 
> > > This xfrm_state_delete() comes too late. You need to synchronize this
> > > with xfrm_state_clone_and_setup() by holding the state lock.
> > > Otherwise we might hand out the same sequence number etc. more than
> > > one time. Unfortunately xfrm_migrate() has the same problem, the
> > > error recovery mechanism in xfrm_migrate() looks broken.
> > 
> > The xfrm_migrate() problem is nasty, looks like we can't recover
> > if the state migration fails. We probably have to delete the
> > states and wait for the userspace to insert new ones.
> 
> userspace should notice the inconsistany. Most of them are ENOMEM or EMSGSIZE
> 
> > 
> > Another thing, xfrm_user_state_lookup() returns a reference on x,
> > you should drop it somewhere.
> 
> There is a call a few lines bellow, the label was "error:"  I renamed it to  
> "out:" for clariy.
> 
> +out:
> +       xfrm_state_put(x);

Yes, this makes it more clear that we hit this even without error.

Maybe you should do a 'if (!X)' check on the retuen value of
xfrm_user_state_lookup(). This woulds make the structure of
the function even clearer.

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

[Antony Antony]

On Thu, Jan 22, 2026 at 11:33:42AM +0100, Steffen Klassert via Devel wrote:
> On Mon, Jan 19, 2026 at 09:54:55AM +0100, Antony Antony wrote:
> > +
> > +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 net *net = sock_net(skb->sk);
> > +	struct xfrm_encap_tmpl *encap = NULL;
> > +	struct xfrm_user_offload *xuo = NULL;
> > +	struct xfrm_migrate m = { .old_saddr.a4 = 0,};
> > +	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);
> 
> This xfrm_state_delete() comes too late. You need to synchronize this
> with xfrm_state_clone_and_setup() by holding the state lock.

yes. I added a lock, x->lock, and within the lock call __xfrm_state_delete()
That seems to work, not encountring a deadlock.

> Otherwise we might hand out the same sequence number etc. more than
> one time. Unfortunately xfrm_migrate() has the same problem, the
> error recovery mechanism in xfrm_migrate() looks broken.