[PATCH] nfc: rawsock: prevent TX work requeue after socket shutdown

[PATCH] nfc: rawsock: prevent TX work requeue after socket shutdown

[Yaohan Li]

rawsock_release() marks the socket for shutdown and cancels tx_work,
but a delayed data-exchange completion can still queue tx_work again
before the socket is finally freed. The new worker can then access
freed socket state.

Check SEND_SHUTDOWN under the write-queue lock before requeueing
transmit work in rawsock_data_exchange_complete(). Allow the completion
to clear the scheduling state without scheduling new work after the
release path has begun.

A KASAN test with a deferred NFC completion callback reproduced the
use-after-free on the unmodified kernel. The same injection-assisted
sequence completed without a report after the change.

Fixes: 23b7869c0fd08 ("NFC: add the NFC socket raw protocol")
Assisted-by: ZCode:GLM-4
Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
---
 net/nfc/rawsock.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/nfc/rawsock.c b/net/nfc/rawsock.c
index f7d7a599fade7..efdbd512d4600 100644
--- a/net/nfc/rawsock.c
+++ b/net/nfc/rawsock.c
@@ -168,7 +168,13 @@ 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))
+	/*
+	 * The release path sets SEND_SHUTDOWN before cancel_work_sync():
+	 * once it is set, do not requeue tx_work - the socket may be freed
+	 * right after the cancel returns.
+	 */
+	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;
-- 
2.34.1

Re: [PATCH] nfc: rawsock: prevent TX work requeue after socket shutdown

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH] nfc: rawsock: prevent TX work requeue after socket shutdown

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is still racy, so the use-after-free it claims to close
  remains reachable.
- [Low] The patch only works on top of commit d793458c45df2a ("nfc:
  rawsock: cancel tx_work before socket teardown", 2026-03-03), and the…

--

nfc: rawsock: prevent TX work requeue after socket shutdown

The patch makes rawsock_data_exchange_complete() check SEND_SHUTDOWN
under sk_write_queue.lock before it reschedules tx_work. The goal is to
stop a delayed data-exchange completion from requeueing the work after
rawsock_release() has started tearing the socket down.

> diff --git a/net/nfc/rawsock.c b/net/nfc/rawsock.c
> index f7d7a599fade7..efdbd512d4600 100644
> --- a/net/nfc/rawsock.c
> +++ b/net/nfc/rawsock.c
> @@ -168,7 +168,13 @@ 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))
> +	/*
> +	 * The release path sets SEND_SHUTDOWN before cancel_work_sync():
> +	 * once it is set, do not requeue tx_work - the socket may be freed
> +	 * right after the cancel returns.
> +	 */

[Severity: Low]
This comment relies on the SEND_SHUTDOWN / cancel_work_sync() block in
rawsock_release(). That block was added by commit d793458c45df2a ("nfc:
rawsock: cancel tx_work before socket teardown"). Before that commit,
rawsock_release() did not set SEND_SHUTDOWN and did not cancel tx_work.

The patch only carries:

    Fixes: 23b7869c0fd08 ("NFC: add the NFC socket raw protocol")

The lines it changes date back to 2011. It would apply cleanly to a
stable tree that lacks d793458c45df2a, and there the new check would do
nothing during release.

Should this have a Fixes: tag for d793458c45df ("nfc: rawsock: cancel
tx_work before socket teardown"), or at least name it as a prerequisite?

> +	if (!skb_queue_empty(&sk->sk_write_queue) &&
> +	    !(sk->sk_shutdown & SEND_SHUTDOWN))
>  		schedule_work(&nfc_rawsock(sk)->tx_work);

[Severity: High]
Is this check actually serialized with the release path? Only the
completion side takes sk_write_queue.lock here. rawsock_release() sets
the flag and cancels the work without holding that lock:

net/nfc/rawsock.c:rawsock_release() {
    ...
	if (sk->sk_state == TCP_ESTABLISHED) {
		...
		sk->sk_shutdown |= SEND_SHUTDOWN;
		cancel_work_sync(&nfc_rawsock(sk)->tx_work);
		rawsock_write_queue_purge(sk);
	}

	sock_orphan(sk);
	sock_put(sk);
    ...
}

Release only takes the lock later, inside rawsock_write_queue_purge().

Can the following interleaving still happen? CPU0 holds the reference
taken by sock_hold() in rawsock_tx_work():

CPU0 (rawsock_data_exchange_complete)   CPU1 (rawsock_release)
spin_lock_bh(&sk->sk_write_queue.lock)
queue non-empty, sk_shutdown == 0
                                        sk->sk_shutdown |= SEND_SHUTDOWN
                                        cancel_work_sync(&tx_work)
                                          tx_work idle, returns at once
schedule_work(&tx_work) succeeds
spin_unlock_bh()
sock_put(sk)
                                        rawsock_write_queue_purge(sk)
                                        sock_orphan(sk)
                                        sock_put(sk)
                                          last ref, socket freed

If tx_work is neither pending nor running, cancel_work_sync() returns
right away. __cancel_work_sync() also calls enable_work() before it
returns, so the later schedule_work() can still queue the work.

rawsock_destruct() does not cancel tx_work, and the queued work holds no
socket reference. When the worker runs, rawsock_tx_work() would read
freed memory:

	struct sock *sk = to_rawsock_sk(work);
	struct nfc_dev *dev = nfc_rawsock(sk)->dev;
	u32 target_idx = nfc_rawsock(sk)->target_idx;
	...
	if (sk->sk_shutdown & SEND_SHUTDOWN) {

Would it close this window to set SEND_SHUTDOWN under sk_write_queue.lock
in rawsock_release(), before cancel_work_sync()? Then either the
completion sees the flag, or its schedule_work() ran before release took
the lock, and cancel_work_sync() will cancel or wait for that work.

Separately, the unlocked read-modify-write of sk->sk_shutdown in
rawsock_release() races with the plain read here. It also races with the
plain store in rawsock_report_error():

	sk->sk_shutdown = SHUTDOWN_MASK;

KCSAN would likely report this as a data race. The rest of the
networking stack accesses sk_shutdown with READ_ONCE()/WRITE_ONCE().

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/179378590950.74297.14503648624087894519%40gmail.com

Re: [PATCH] nfc: rawsock: prevent TX work requeue after socket shutdown

[zhang san]

Hello,

I am resending my response in plain text because my earlier Gmail
reply contained an HTML part and was rejected by the netdev,
linux-kernel, and oe-linux-nfc mailing lists.

The issue was identified during AI-assisted source-code review and
investigated with a KASAN-enabled kernel and a deterministic
timing-injection harness using a virtual NFC device in QEMU. No
physical NFC hardware was used.

The High-severity finding on v1 is valid. rawsock_release() set
SEND_SHUTDOWN without holding sk_write_queue.lock, allowing a delayed
completion to requeue tx_work after cancel_work_sync() had returned.
In v2, release sets SEND_SHUTDOWN under the same lock used by
rawsock_data_exchange_complete() for the shutdown check and
schedule_work().

The Low-severity Fixes-tag finding is also valid. V2 uses Fixes:
d793458c45df ("nfc: rawsock: cancel tx_work before socket teardown").

The timing-injection test produced seven slab-use-after-free reports
with the v1 locking and none with the revised locking under the same
conditions. We do not claim that this patch eliminates all potential
data races involving sk_shutdown.

V2 was already sent as a separate thread:
https://lore.kernel.org/r/20261105113812.969651-1-shunsheng1218@gmail.com/

Best regards,
Yaohan Li

pw-bot: changes-requested

On Thu, 05 Nov 2026 09:58:39 +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 0 · Low: 1
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] The fix is still racy, so the use-after-free it claims to close
> remains reachable.
> - [Low] The patch only works on top of commit d793458c45df2a ("nfc:
> rawsock: cancel tx_work before socket teardown", 2026-03-03), and the…
>
> --
>
> nfc: rawsock: prevent TX work requeue after socket shutdown
>
> The patch makes rawsock_data_exchange_complete() check SEND_SHUTDOWN
> under sk_write_queue.lock before it reschedules tx_work. The goal is to
> stop a delayed data-exchange completion from requeueing the work after
> rawsock_release() has started tearing the socket down.
>
> > diff --git a/net/nfc/rawsock.c b/net/nfc/rawsock.c
> > index f7d7a599fade7..efdbd512d4600 100644
> > --- a/net/nfc/rawsock.c
> > +++ b/net/nfc/rawsock.c
> > @@ -168,7 +168,13 @@ 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))
> > + /*
> > + * The release path sets SEND_SHUTDOWN before cancel_work_sync():
> > + * once it is set, do not requeue tx_work - the socket may be freed
> > + * right after the cancel returns.
> > + */
>
> [Severity: Low]
> This comment relies on the SEND_SHUTDOWN / cancel_work_sync() block in
> rawsock_release(). That block was added by commit d793458c45df2a ("nfc:
> rawsock: cancel tx_work before socket teardown"). Before that commit,
> rawsock_release() did not set SEND_SHUTDOWN and did not cancel tx_work.
>
> The patch only carries:
>
> Fixes: 23b7869c0fd08 ("NFC: add the NFC socket raw protocol")
>
> The lines it changes date back to 2011. It would apply cleanly to a
> stable tree that lacks d793458c45df2a, and there the new check would do
> nothing during release.
>
> Should this have a Fixes: tag for d793458c45df ("nfc: rawsock: cancel
> tx_work before socket teardown"), or at least name it as a prerequisite?
>
> > + if (!skb_queue_empty(&sk->sk_write_queue) &&
> > + !(sk->sk_shutdown & SEND_SHUTDOWN))
> > schedule_work(&nfc_rawsock(sk)->tx_work);
>
> [Severity: High]
> Is this check actually serialized with the release path? Only the
> completion side takes sk_write_queue.lock here. rawsock_release() sets
> the flag and cancels the work without holding that lock:
>
> net/nfc/rawsock.c:rawsock_release() {
> ...
> if (sk->sk_state == TCP_ESTABLISHED) {
> ...
> sk->sk_shutdown |= SEND_SHUTDOWN;
> cancel_work_sync(&nfc_rawsock(sk)->tx_work);
> rawsock_write_queue_purge(sk);
> }
>
> sock_orphan(sk);
> sock_put(sk);
> ...
> }
>
> Release only takes the lock later, inside rawsock_write_queue_purge().
>
> Can the following interleaving still happen? CPU0 holds the reference
> taken by sock_hold() in rawsock_tx_work():
>
> CPU0 (rawsock_data_exchange_complete) CPU1 (rawsock_release)
> spin_lock_bh(&sk->sk_write_queue.lock)
> queue non-empty, sk_shutdown == 0
> sk->sk_shutdown |= SEND_SHUTDOWN
> cancel_work_sync(&tx_work)
> tx_work idle, returns at once
> schedule_work(&tx_work) succeeds
> spin_unlock_bh()
> sock_put(sk)
> rawsock_write_queue_purge(sk)
> sock_orphan(sk)
> sock_put(sk)
> last ref, socket freed
>
> If tx_work is neither pending nor running, cancel_work_sync() returns
> right away. __cancel_work_sync() also calls enable_work() before it
> returns, so the later schedule_work() can still queue the work.
>
> rawsock_destruct() does not cancel tx_work, and the queued work holds no
> socket reference. When the worker runs, rawsock_tx_work() would read
> freed memory:
>
> struct sock *sk = to_rawsock_sk(work);
> struct nfc_dev *dev = nfc_rawsock(sk)->dev;
> u32 target_idx = nfc_rawsock(sk)->target_idx;
> ...
> if (sk->sk_shutdown & SEND_SHUTDOWN) {
>
> Would it close this window to set SEND_SHUTDOWN under sk_write_queue.lock
> in rawsock_release(), before cancel_work_sync()? Then either the
> completion sees the flag, or its schedule_work() ran before release took
> the lock, and cancel_work_sync() will cancel or wait for that work.
>
> Separately, the unlocked read-modify-write of sk->sk_shutdown in
> rawsock_release() races with the plain read here. It also races with the
> plain store in rawsock_report_error():
>
> sk->sk_shutdown = SHUTDOWN_MASK;
>
> KCSAN would likely report this as a data race. The rest of the
> networking stack accesses sk_shutdown with READ_ONCE()/WRITE_ONCE().
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/179378590950.74297.14503648624087894519%40gmail.com