Netdev List
 help / color / mirror / Atom feed
From: Daniel Zahka <daniel.zahka@gmail.com>
To: Weiming Shi <bestswngs@gmail.com>,
	David Heidelberg <david@ixit.cz>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>
Cc: oe-linux-nfc@lists.linux.dev, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Xiang Mei <xmei5@asu.edu>
Subject: Re: [PATCH net-next] nfc: digital: fix use-after-free in nfc_digital_unregister_device()
Date: Tue, 21 Jul 2026 14:07:06 -0400	[thread overview]
Message-ID: <7fd2e4f3-ccfe-4e09-8f24-8dcf89b4774d@gmail.com> (raw)
In-Reply-To: <20260721163632.1570651-1-bestswngs@gmail.com>



On 7/21/26 12:36 PM, Weiming Shi wrote:
> nfc_digital_unregister_device() cancels cmd_work and cmd_complete_work
> once each and then frees the command queue.  The two works re-arm each
> other: digital_wq_cmd_complete() ends with schedule_work(&ddev->cmd_work),
> and digital_wq_cmd() hands a command to the driver whose asynchronous
> completion schedules cmd_complete_work.  cancel_work_sync() only waits for
> the instance it cancels; it does not stop the work from being queued again.
> A work re-armed after its cancel_work_sync() therefore runs concurrently
> with the cmd_queue cleanup and dereferences a digital_cmd the cleanup has
> already freed.  digital_wq_cmd() widens the window by dropping cmd_lock
> before using the command it took from the queue, while the cleanup loop
> frees the commands without holding cmd_lock.
> 
> It is reproducible with the software NFC simulator (CONFIG_NFC_SIM): start
> an NFC-DEP exchange between the two nfcsim devices and unload the module
> while it is running.
> 
>   BUG: KASAN: slab-use-after-free in digital_wq_cmd (net/nfc/digital_core.c:174)
>   Read of size 1 by task kworker/1:5
>   Workqueue: events digital_wq_cmd
>    digital_wq_cmd (net/nfc/digital_core.c:174)
>    process_one_work
>    worker_thread
>    kthread
> 
>   Allocated by task 5124:
>    digital_send_cmd (net/nfc/digital_core.c:234)
>    digital_in_send_sdd_req
>    digital_in_recv_sens_res
>    digital_wq_cmd_complete (net/nfc/digital_core.c:134)
> 
>   Freed by task 4994:
>    kfree
>    nfc_digital_unregister_device (net/nfc/digital_core.c:859)
>    nfcsim_device_free [nfcsim]
>    nfcsim_exit [nfcsim]
>    __do_sys_delete_module
> 
> Use disable_work_sync() instead of cancel_work_sync() for the two command
> works.  disable_work_sync() cancels the work and disables it, so any later
> schedule_work() -- whether from the sibling work re-arming it or from the
> driver's completion callback -- becomes a no-op.  Once both works are
> disabled no work can run, and the cleanup loop frees the queue with no work
> able to reach a freed command.
> 
> Fixes: 59ee2361c924 ("NFC Digital: Implement driver commands mechanism")

Fix for this commit should target the net tree instead of net-next.

> Reported-by: Xiang Mei <xmei5@asu.edu>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
>   net/nfc/digital_core.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/nfc/digital_core.c b/net/nfc/digital_core.c
> index 7cb1e6aaae90..6def5132a4a6 100644
> --- a/net/nfc/digital_core.c
> +++ b/net/nfc/digital_core.c
> @@ -843,8 +843,8 @@ void nfc_digital_unregister_device(struct nfc_digital_dev *ddev)
>   	mutex_unlock(&ddev->poll_lock);
>   
>   	cancel_delayed_work_sync(&ddev->poll_work);
> -	cancel_work_sync(&ddev->cmd_work);
> -	cancel_work_sync(&ddev->cmd_complete_work);
> +	disable_work_sync(&ddev->cmd_work);
> +	disable_work_sync(&ddev->cmd_complete_work);
>   
>   	list_for_each_entry_safe(cmd, n, &ddev->cmd_queue, queue) {
>   		list_del(&cmd->queue);
> 
> base-commit: d4932951a19a5f1ec93200260b85e1a4c080ff77


  reply	other threads:[~2026-07-21 18:07 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-21 16:36 [PATCH net-next] nfc: digital: fix use-after-free in nfc_digital_unregister_device() Weiming Shi
2026-07-21 18:07 ` Daniel Zahka [this message]
2026-07-21 18:31   ` Weiming Shi

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=7fd2e4f3-ccfe-4e09-8f24-8dcf89b4774d@gmail.com \
    --to=daniel.zahka@gmail.com \
    --cc=bestswngs@gmail.com \
    --cc=davem@davemloft.net \
    --cc=david@ixit.cz \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=oe-linux-nfc@lists.linux.dev \
    --cc=pabeni@redhat.com \
    --cc=xmei5@asu.edu \
    /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