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:

quoted

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 <redacted>

...

quoted

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.

quoted

+					   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.

quoted

 	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.

quoted

 	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));

...

quoted

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

...

quoted

+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

quoted

+{
+	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);
+}
+

...

quoted

+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.

quoted

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

I will send a new v2.

regards
-antony