[PATCH net] xfrm: reject duplicate state resolution in xfrm_migrate()

[Jun Yang <juny24602@gmail.com>]

From: Jun Yang <junvyyang@tencent.com>

xfrm_migrate_check() rejects identical migration entries, but entries
with different old_reqid values can still resolve to the same state.
In particular, old_reqid == 0 is a wildcard, so it can match the same SA
as an entry carrying that SA's reqid.

xfrm_migrate() then stores the same state more than once in x_cur[].
After the lookup references are dropped, the first delete can release
the table's last reference. The garbage collector can free the state
before a subsequent delete accesses it.

Reject a state already present in x_cur[] before cloning it again. Drop
the extra lookup reference and use the existing rollback path to remove
any clones installed for earlier entries.

Fixes: 80c9abaabf42 ("[XFRM]: Extension for dynamic update of endpoint address(es)")
Cc: stable@kernel.org
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Signed-off-by: Jun Yang <junvyyang@tencent.com>
---
A KASAN reproducer for this issue is available if requested.

 net/xfrm/xfrm_policy.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 9ea60c7717ff..472f65616e28 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -4669,7 +4669,7 @@ int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		 struct xfrm_encap_tmpl *encap, u32 if_id,
 		 struct netlink_ext_ack *extack, struct xfrm_user_offload *xuo)
 {
-	int i, err, nx_cur = 0, nx_new = 0;
+	int i, j, err, nx_cur = 0, nx_new = 0;
 	struct xfrm_policy *pol = NULL;
 	struct xfrm_state *x, *xc;
 	struct xfrm_state *x_cur[XFRM_MAX_DEPTH];
@@ -4698,6 +4698,17 @@ int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 	/* Stage 2 - find and update state(s) */
 	for (i = 0, mp = m; i < num_migrate; i++, mp++) {
 		if ((x = xfrm_migrate_state_find(mp, net, if_id))) {
+			for (j = 0; j < nx_cur; j++) {
+				if (x_cur[j] == x)
+					break;
+			}
+			if (j < nx_cur) {
+				xfrm_state_put(x);
+				NL_SET_ERR_MSG(extack,
+					       "Multiple MIGRATE entries resolve to the same SA");
+				err = -EINVAL;
+				goto restore_state;
+			}
 			x_cur[nx_cur] = x;
 			nx_cur++;
 			mp->encap = encap;
-- 
2.43.7