Re: [PATCH v2 net] nfc: llcp: use disable_work_sync and timer_shutdown_sync in local_cleanup

[Simon Horman]

On Thu, Oct 29, 2026 at 04:24:19PM +0000, Tristan Madani wrote:
> From: Tristan Madani <tristan@xxxxxxxxxxxxxxxxxxx>
> 
> local_cleanup() tears down timers and work items before
> local_release_work() frees the nfc_llcp_local structure.  The current
> code has two ordering problems:
> 
> 1) timer_delete_sync() does not prevent timer callbacks from re-arming
>    the timer via mod_timer().  nfc_llcp_tx_work() unconditionally
>    re-arms link_timer, and nfc_llcp_sdreq_timeout_work() conditionally
>    re-arms sdreq_timer.  A work callback that runs during
>    cancel_work_sync() and calls mod_timer() causes the timer to fire
>    after the structure is freed.
> 
> 2) cancel_work_sync() does not prevent work items from being
>    re-queued.  nfc_llcp_rx_work() queues tx_work via
>    schedule_work(), and the timer callbacks queue their associated
>    work items.  Since tx_work is cancelled before rx_work, a
>    running rx_work instance can re-queue tx_work after its
>    cancel_work_sync() has returned.  The re-queued tx_work then
>    runs on the freed structure.
> 
> Switch the timers to timer_shutdown_sync() which rejects subsequent
> mod_timer() calls.  Switch the work items to disable_work_sync()
> which rejects subsequent schedule_work() calls.
> 
> Reorder the teardown sequence so that both timers are shut down
> first (their callbacks queue work items), rx_work is disabled
> before tx_work (rx_work queues tx_work), and data structures are
> freed only after all asynchronous callbacks are quiesced.
> 
> Fixes: d646960f7986 ("NFC: Initial LLCP support")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Tristan Madani <tristan@xxxxxxxxxxxxxxxxxxx>
> ---
> v2:
>   - Switch work items from cancel_work_sync() to disable_work_sync()
>     to prevent re-queuing after cancellation, as identified by the
>     automated review and confirmed by Simon Horman
>   - Reorder teardown: shut down both timers first (their callbacks
>     queue work items), then disable rx_work before tx_work (rx_work
>     queues tx_work), then disable remaining work items
>   - Move data structure cleanup (skb_queue_purge, kfree_skb) after
>     all asynchronous callbacks are quiesced

Thanks for the updates.

Reviewed-by: Simon Horman <horms@xxxxxxxxxx>