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

[Antony Antony]

On Sat, Oct 31, 2026 at 11:28:49 +0800, Cen Zhang wrote:
> 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>

Reviewed-by: Antony Antony <antony.antony@xxxxxxxxxxx>

Thanks,
-antony