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

[Antony Antony]

On Tue, Nov 03, 2026 at 09:01:14 +0100, Antony Antony wrote:
> 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>

I forgot to mention another thought. If you send a new version may 
be remove xso_to_xuo(), and directly call 
xso_to_xuo_ifindex(xso, READ_ONCE(xso->ifindex), xuo); ?