[PATCH net v2] nfc: rawsock: serialize SEND_SHUTDOWN with the TX work requeue

[PATCH net v2] nfc: rawsock: serialize SEND_SHUTDOWN with the TX work requeue

[Yaohan Li]

rawsock_release() sets SEND_SHUTDOWN before cancelling tx_work, but it
updated the shutdown flag without the write-queue lock that
rawsock_data_exchange_complete() holds while it tests the flag and
re-queues tx_work.  A deferred completion could pass that check before
the flag was set and still schedule the work after cancel_work_sync()
had returned; the worker then ran on a socket that was freed right
after release dropped its reference.  KASAN reports the
slab-use-after-free in rawsock_tx_work().

Set SEND_SHUTDOWN under the write-queue lock, so the completion either
queues the work before the flag becomes visible (and cancel_work_sync()
then covers it) or observes the flag and does not queue.  The
completion-side check from the previous submission is kept.

The remaining requeue window after the cancel-based teardown was
previously raised in review of that change (CVE-2026-23372): an
asynchronous NFC completion can queue tx_work after
cancel_work_sync() has returned.  This change closes that window and
does not claim to be the first report of a rawsock teardown
use-after-free.

This issue was found during an AI-assisted source review of the socket
release, TX work and deferred data-exchange completion paths.  It was
reproduced on a KASAN kernel with a deterministic timing-injection
harness for a deferred NFC completion callback: with only the
completion-side check, stalling the completion between its shutdown
check and schedule_work() while release ran produced seven
slab-use-after-free reports in rawsock_tx_work(); with this change the
same sequence and the original purge-window injection produce no
reports.  The tests run in QEMU with a virtual NFC device; no NFC
hardware is involved.

Link: https://lists.openwall.net/netdev/2026/03/24/271
Fixes: d793458c45df ("nfc: rawsock: cancel tx_work before socket teardown")
Cc: stable@vger.kernel.org
Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
---

v2:
- Set SEND_SHUTDOWN under the write-queue lock, so a completion that
  tested the flag before it was visible can no longer requeue tx_work
  after cancel_work_sync() returned (v1 only added the completion-side
  check, which left this ordering window open).
- Added deterministic A/B testing of that window: stalling the
  completion between its shutdown check and schedule_work() while
  release runs reproduces the use-after-free on the v1 code and is
  clean with this change.
- Corrected the Fixes tag to the cancel-based teardown commit and
  linked the earlier review thread for the remaining window.

v1: https://lore.kernel.org/r/179378590950.74297.14503648624087894519@gmail.com/
 net/nfc/rawsock.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/net/nfc/rawsock.c b/net/nfc/rawsock.c
index f7d7a599fade7..dd8401e3402d3 100644
--- a/net/nfc/rawsock.c
+++ b/net/nfc/rawsock.c
@@ -68,12 +68,23 @@ static int rawsock_release(struct socket *sock)
 		nfc_sock_unlink(&raw_sk_list, sk);
 
 	if (sk->sk_state == TCP_ESTABLISHED) {
-		/* Prevent rawsock_tx_work from starting new transmits and
+		/*
+		 * Prevent rawsock_tx_work from starting new transmits and
 		 * wait for any in-progress work to finish.  This must happen
 		 * before the socket is orphaned to avoid a race where
 		 * rawsock_tx_work runs after the NCI device has been freed.
+		 *
+		 * Set SEND_SHUTDOWN under the write-queue lock so it is
+		 * ordered against rawsock_data_exchange_complete(), which
+		 * tests the flag and re-queues tx_work under the same lock:
+		 * either the completion queued the work before we take the
+		 * lock (cancel_work_sync() below then covers it), or it
+		 * sees SEND_SHUTDOWN and does not queue.
 		 */
+		spin_lock_bh(&sk->sk_write_queue.lock);
 		sk->sk_shutdown |= SEND_SHUTDOWN;
+		spin_unlock_bh(&sk->sk_write_queue.lock);
+
 		cancel_work_sync(&nfc_rawsock(sk)->tx_work);
 		rawsock_write_queue_purge(sk);
 	}
@@ -168,7 +179,14 @@ static void rawsock_data_exchange_complete(void *context, struct sk_buff *skb,
 		goto error_skb;
 
 	spin_lock_bh(&sk->sk_write_queue.lock);
-	if (!skb_queue_empty(&sk->sk_write_queue))
+	/*
+	 * Once the release path has set SEND_SHUTDOWN, do not requeue
+	 * tx_work: the socket may be freed right after cancel_work_sync()
+	 * returns.  The flag is checked under the same lock that the
+	 * release path uses to set it.
+	 */
+	if (!skb_queue_empty(&sk->sk_write_queue) &&
+	    !(sk->sk_shutdown & SEND_SHUTDOWN))
 		schedule_work(&nfc_rawsock(sk)->tx_work);
 	else
 		nfc_rawsock(sk)->tx_work_scheduled = false;

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.34.1

Re: [PATCH net v2] nfc: rawsock: serialize SEND_SHUTDOWN with the TX work requeue

[Simon Horman]

On Thu, Nov 05, 2026 at 07:38:12PM +0800, Yaohan Li wrote:
> rawsock_release() sets SEND_SHUTDOWN before cancelling tx_work, but it
> updated the shutdown flag without the write-queue lock that
> rawsock_data_exchange_complete() holds while it tests the flag and
> re-queues tx_work.  A deferred completion could pass that check before
> the flag was set and still schedule the work after cancel_work_sync()
> had returned; the worker then ran on a socket that was freed right
> after release dropped its reference.  KASAN reports the
> slab-use-after-free in rawsock_tx_work().
> 
> Set SEND_SHUTDOWN under the write-queue lock, so the completion either
> queues the work before the flag becomes visible (and cancel_work_sync()
> then covers it) or observes the flag and does not queue.  The
> completion-side check from the previous submission is kept.
> 
> The remaining requeue window after the cancel-based teardown was
> previously raised in review of that change (CVE-2026-23372): an
> asynchronous NFC completion can queue tx_work after
> cancel_work_sync() has returned.  This change closes that window and
> does not claim to be the first report of a rawsock teardown
> use-after-free.
> 
> This issue was found during an AI-assisted source review of the socket
> release, TX work and deferred data-exchange completion paths.  It was
> reproduced on a KASAN kernel with a deterministic timing-injection
> harness for a deferred NFC completion callback: with only the
> completion-side check, stalling the completion between its shutdown
> check and schedule_work() while release ran produced seven
> slab-use-after-free reports in rawsock_tx_work(); with this change the
> same sequence and the original purge-window injection produce no
> reports.  The tests run in QEMU with a virtual NFC device; no NFC
> hardware is involved.
> 
> Link: https://lists.openwall.net/netdev/2026/03/24/271
> Fixes: d793458c45df ("nfc: rawsock: cancel tx_work before socket teardown")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
> ---
> 
> v2:
> - Set SEND_SHUTDOWN under the write-queue lock, so a completion that
>   tested the flag before it was visible can no longer requeue tx_work
>   after cancel_work_sync() returned (v1 only added the completion-side
>   check, which left this ordering window open).
> - Added deterministic A/B testing of that window: stalling the
>   completion between its shutdown check and schedule_work() while
>   release runs reproduces the use-after-free on the v1 code and is
>   clean with this change.
> - Corrected the Fixes tag to the cancel-based teardown commit and
>   linked the earlier review thread for the remaining window.
> 
> v1: https://lore.kernel.org/r/179378590950.74297.14503648624087894519@gmail.com/

Thanks for the updates.

>  net/nfc/rawsock.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/net/nfc/rawsock.c b/net/nfc/rawsock.c
> index f7d7a599fade7..dd8401e3402d3 100644
> --- a/net/nfc/rawsock.c
> +++ b/net/nfc/rawsock.c
> @@ -68,12 +68,23 @@ static int rawsock_release(struct socket *sock)
>  		nfc_sock_unlink(&raw_sk_list, sk);
>  
>  	if (sk->sk_state == TCP_ESTABLISHED) {
> -		/* Prevent rawsock_tx_work from starting new transmits and
> +		/*
> +		 * Prevent rawsock_tx_work from starting new transmits and

I don't think there is a need to respin just because of it.
But if there is a v3 for some other reason then I think the change
above can be dropped. Perhaps tooling flaged the existing style,
but it is acceptable for Networking code. And, for many years
it was required.

>  		 * wait for any in-progress work to finish.  This must happen
>  		 * before the socket is orphaned to avoid a race where
>  		 * rawsock_tx_work runs after the NCI device has been freed.
> +		 *
> +		 * Set SEND_SHUTDOWN under the write-queue lock so it is
> +		 * ordered against rawsock_data_exchange_complete(), which
> +		 * tests the flag and re-queues tx_work under the same lock:
> +		 * either the completion queued the work before we take the
> +		 * lock (cancel_work_sync() below then covers it), or it
> +		 * sees SEND_SHUTDOWN and does not queue.
>  		 */
> +		spin_lock_bh(&sk->sk_write_queue.lock);
>  		sk->sk_shutdown |= SEND_SHUTDOWN;
> +		spin_unlock_bh(&sk->sk_write_queue.lock);
> +
>  		cancel_work_sync(&nfc_rawsock(sk)->tx_work);
>  		rawsock_write_queue_purge(sk);
>  	}

...

Overall this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>