Re: [PATCH net-next] nfc: digital: fix use-after-free in nfc_digital_unregister_device()
Daniel Zahka <[email protected]> Tue, 21 Jul 2026 14:07:06 -0400
| Newsgroups | dev.linux.lists.oe-linux-nfc,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Weiming Shi <[email protected]>
> ---
> 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