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

[Tristan Madani]

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
 net/nfc/llcp_core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index 74bf817007cf2..984e7088288f0 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -173,15 +173,15 @@ static struct nfc_llcp_local *nfc_llcp_local_get(struct nfc_llcp_local *local)
 static void local_cleanup(struct nfc_llcp_local *local)
 {
 	nfc_llcp_socket_release(local, false, ENXIO);
-	timer_delete_sync(&local->link_timer);
+	timer_shutdown_sync(&local->link_timer);
+	timer_shutdown_sync(&local->sdreq_timer);
+	disable_work_sync(&local->rx_work);
+	disable_work_sync(&local->tx_work);
+	disable_work_sync(&local->timeout_work);
+	disable_work_sync(&local->sdreq_timeout_work);
 	skb_queue_purge(&local->tx_queue);
-	cancel_work_sync(&local->tx_work);
-	cancel_work_sync(&local->rx_work);
-	cancel_work_sync(&local->timeout_work);
 	kfree_skb(local->rx_pending);
 	local->rx_pending = NULL;
-	timer_delete_sync(&local->sdreq_timer);
-	cancel_work_sync(&local->sdreq_timeout_work);
 	nfc_llcp_free_sdp_tlv_list(&local->pending_sdreqs);
 }
 
-- 
2.47.3