Netdev List
 help / color / mirror / Atom feed
From: Antony Antony <antony.antony@secunet.com>
To: Cen Zhang <zzzccc427@gmail.com>
Cc: <steffen.klassert@secunet.com>, <herbert@gondor.apana.org.au>,
	<davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
	<pabeni@redhat.com>, <horms@kernel.org>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <antony.antony@secunet.com>,
	<baijiaju1990@gmail.com>, <jjzuming@gmail.com>
Subject: Re: [PATCH ipsec v2 2/2] xfrm: use cached offload ifindex when migrating an SA
Date: Tue, 3 Nov 2026 09:01:06 +0100	[thread overview]
Message-ID: <aumUwuVWsOOQdMG-@moon.secunet.de> (raw)
In-Reply-To: <pm-xfrm-core-objects-candidate-0050-v4-3c6b8c5369a802e9be0e-2@gmail.com>

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@gmail.com>

Reviewed-by: Antony Antony <antony.antony@secunet.com>

Thanks,
-antony

  parent reply	other threads:[~2026-11-03  8:01 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-31  3:28 [PATCH ipsec v2 0/2] xfrm: fix offload state use racing with device teardown Cen Zhang
2026-10-31  3:28 ` [PATCH ipsec v2 1/2] xfrm: protect offload statistics updates from " Cen Zhang
2026-11-01  3:30   ` netdev-bot+sashiko
2026-10-31  3:28 ` [PATCH ipsec v2 2/2] xfrm: use cached offload ifindex when migrating an SA Cen Zhang
2026-11-01  3:30   ` netdev-bot+sashiko
2026-11-03  8:01   ` Antony Antony [this message]
2026-11-03  8:11     ` Antony Antony

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aumUwuVWsOOQdMG-@moon.secunet.de \
    --to=antony.antony@secunet.com \
    --cc=baijiaju1990@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=horms@kernel.org \
    --cc=jjzuming@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=steffen.klassert@secunet.com \
    --cc=zzzccc427@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox