[PATCH ipsec v2 2/2] xfrm: use cached offload ifindex when migrating an SA

[Cen Zhang]

An xfrm_state reference keeps its offload record alive, but does not
prevent xfrm_dev_state_free() from clearing xso.dev and dropping the
device reference. xfrm_do_migrate_state() checks xso.dev before
xso_to_xuo() dereferences it to obtain the offload ifindex.

With XFRM offload and migration enabled, XFRM_MSG_MIGRATE_STATE inherits
the old SA's offload configuration if the request omits
XFRMA_OFFLOAD_DEV and does not set XFRM_MIGRATE_STATE_CLEAR_OFFLOAD.
Device unregister can flush that SA between the check and the read.
The supplied report used a scheduling probe to delay migration in this
interval while in-tree netdevsim was removed through sysfs.

    Migration (xfrm_cfg_mutex)       Device unregister (RTNL)
    xfrm_state_lookup(): hold x
    test x->xso.dev != NULL
                                    xfrm_dev_state_flush()
                                      xfrm_state_delete(x)
                                      xfrm_dev_state_free(x)
                                        xso->dev = NULL
                                        netdev_put(dev)
    xso_to_xuo(&x->xso, &xuo)
      xso->dev->ifindex: NULL deref

The netlink configuration mutex does not serialize with the notifier,
and migration takes the state lock only after this read. The result is
a NULL pointer dereference even though migration still holds the SA.

Use the ifindex cached in the offload record at attachment, as state
dumps already do. The SA reference protects this snapshot throughout
the read, so serialization no longer depends on the device lifetime.
Annotate the binding check with READ_ONCE() to match the concurrent
WRITE_ONCE() in xfrm_dev_state_free().

Oops report as below:

    Oops: general protection fault, probably for non-canonical address
    0xdffffc000000001c: 0000 [#1] SMP KASAN NOPTI
    KASAN: null-ptr-deref in range [0x00000000000000e0-0x00000000000000e7]
    RIP: 0010:xfrm_do_migrate_state+0x1f06/0x2c50
    Call Trace:
     <TASK>
     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>

Fixes: a9d155ea9b44 ("xfrm: add XFRM_MSG_MIGRATE_STATE for single SA migration")
Signed-off-by: Cen Zhang <zzzccc427@xxxxxxxxx>
---
Assisted-by: LLM

 net/xfrm/xfrm_user.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 781d4ec565aa..1cea24380797 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1240,7 +1240,7 @@ static void xso_to_xuo_ifindex(const struct xfrm_dev_offload *xso, int ifindex,
 static void xso_to_xuo(const struct xfrm_dev_offload *xso,
 		       struct xfrm_user_offload *xuo)
 {
-	xso_to_xuo_ifindex(xso, xso->dev->ifindex, xuo);
+	xso_to_xuo_ifindex(xso, READ_ONCE(xso->ifindex), xuo);
 }
 #endif
 
@@ -3462,7 +3462,7 @@ static int xfrm_do_migrate_state(struct sk_buff *skb, struct nlmsghdr *nlh,
 	} else {
 		bool inherit_offload = !(um->flags & XFRM_MIGRATE_STATE_CLEAR_OFFLOAD);
 
-		if (inherit_offload && x->xso.dev) {
+		if (inherit_offload && READ_ONCE(x->xso.dev)) {
 			xso_to_xuo(&x->xso, &xuo);
 			m.xuo = &xuo;
 		}