[PATCH ipsec 2/2] xfrm: hold migrated states through rollback

[Cen Zhang <zzzccc427@gmail.com>]

xfrm_migrate() needs to keep each new state alive until it either commits
or rolls back the migration. xfrm_state_migrate() publishes the successor
using its initial reference as SAD ownership, then returns it without a
caller reference. The pointer saved in x_new[] can therefore outlive the
state.

A legacy migration can find an existing policy with no matching templates,
install a successor and then fail xfrm_policy_migrate() with -ENODATA. If
the successor inherits a finite hard lifetime and the migration task is
delayed before rollback, expiry and GC can finish first:

  Migration task                  Timer / GC
  xfrm_state_migrate()
    publish xc, arm mtimer
  save xc in x_new[]
  xfrm_policy_migrate()
    return -ENODATA
  <task delayed>                  xfrm_timer_handler(xc)
                                    __xfrm_state_delete(xc)
                                    drop SAD reference
                                  xfrm_state_gc_task()
                                    synchronize_rcu()
                                    free xc
  xfrm_states_delete(x_new)
    xfrm_state_delete(xc)
      spin_lock_bh(&xc->lock)

The control mutex does not serialize the timer or GC, and migration holds
no RCU read lock across this interval. Rollback consequently accesses the
freed state's lock. This also affects the PF_KEY migration entry, which
uses the same helpers.

Take a caller reference before installing the successor, and release it
on installation failure or after the transaction succeeds. On rollback,
delete the successors before dropping their caller references. This keeps
each allocation alive even if its timer has already removed it from the
SAD.

The reported execution used trace probes and paused the netlink migration
before the rollback lock access while the normal timer and GC ran. The
userspace setup installed an SA with a five-second hard lifetime and a
policy without templates using ordinary XFRM requests.

KASAN report as below:

    ==================================================================
    BUG: KASAN: slab-use-after-free in _raw_spin_lock_bh+0x38/0x50
    Read of size 1 at addr ffff8881076d6248 by task python3/417

    Call Trace:
     <TASK>
     dump_stack_lvl+0x93/0xd0
     print_report+0xce/0x630
     kasan_report+0xe0/0x110
     __kasan_check_byte+0x36/0x50
     lock_acquire+0x113/0x2f0
     _raw_spin_lock_bh+0x38/0x50
     xfrm_state_delete+0x26/0x60
     xfrm_migrate+0x11af/0x18a0
     xfrm_do_migrate+0x797/0xad0
     xfrm_user_rcv_msg+0x3b3/0xc20
     netlink_rcv_skb+0x133/0x390
     xfrm_netlink_rcv+0x76/0x90
     netlink_unicast+0x504/0x840
     netlink_sendmsg+0x7f7/0xcf0
     __sys_sendto+0x425/0x470
     __x64_sys_sendto+0xe5/0x1c0
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f
     </TASK>

    Allocated by task 417:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     __kasan_slab_alloc+0x89/0x90
     kmem_cache_alloc_noprof+0x20b/0x5f0
     xfrm_state_alloc+0x28/0x5d0
     xfrm_state_migrate_create+0x55/0x2470
     xfrm_state_migrate+0x37/0x450
     xfrm_migrate+0x80b/0x18a0
     xfrm_do_migrate+0x797/0xad0
     xfrm_user_rcv_msg+0x3b3/0xc20
     netlink_rcv_skb+0x133/0x390
     xfrm_netlink_rcv+0x76/0x90
     netlink_unicast+0x504/0x840
     netlink_sendmsg+0x7f7/0xcf0
     __sys_sendto+0x425/0x470
     __x64_sys_sendto+0xe5/0x1c0
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

    Freed by task 11:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     kasan_save_free_info+0x3b/0x60
     __kasan_slab_free+0x5f/0x80
     kmem_cache_free+0x242/0x580
     xfrm_state_gc_task+0x533/0x730
     process_one_work+0x908/0x19c0
     worker_thread+0x65c/0xe40
     kthread+0x34f/0x460
     ret_from_fork+0x659/0x940
     ret_from_fork_asm+0x1a/0x30

    The buggy address belongs to the object at ffff8881076d61c0
     which belongs to the cache xfrm_state of size 936
    The buggy address is located 136 bytes inside of
     freed 936-byte region [ffff8881076d61c0, ffff8881076d6568)
    ==================================================================

Fixes: 80c9abaabf42 ("[XFRM]: Extension for dynamic update of endpoint address(es)")
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
Assisted-by: LLM

 net/xfrm/xfrm_policy.c | 5 ++++-
 net/xfrm/xfrm_state.c  | 6 +++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 55f6bacc4e51..cced7928e980 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -4733,6 +4733,7 @@ int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 	/* Stage 5 - announce */
 	km_migrate(sel, dir, type, m, num_migrate, k, net, encap);
 
+	xfrm_states_put(x_new, nx_new);
 	xfrm_pol_put(pol);
 
 	return 0;
@@ -4744,8 +4745,10 @@ int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		xfrm_pol_put(pol);
 	if (nx_cur)
 		xfrm_states_put(x_cur, nx_cur);
-	if (nx_new)
+	if (nx_new) {
 		xfrm_states_delete(x_new, nx_new);
+		xfrm_states_put(x_new, nx_new);
+	}
 
 	return err;
 }
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 0555e5d796ed..b9db6e903292 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2238,8 +2238,12 @@ struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
 
 	xfrm_migrate_sync(xc, x);
 
-	if (xfrm_state_migrate_install(x, xc, m, extack) < 0)
+	/* Hold a reference for the caller before publishing the state. */
+	xfrm_state_hold(xc);
+	if (xfrm_state_migrate_install(x, xc, m, extack) < 0) {
+		xfrm_state_put(xc);
 		return NULL;
+	}
 
 	return xc;
 }