Thread (2 messages) flat view 2 messages, 2 authors, 17d ago

Re: [PATCH net v2] net: mana: fix reset work race with device removal

From: Simon Horman <horms@kernel.org>
Date: 2026-09-08 11:23:06
Also in: linux-hyperv, lkml, stable

On Sat, Sep 05, 2026 at 02:36:02AM +0000, Fan Wu wrote:
The reset service work is queued on the system workqueue and can
outlive mana_gd_remove(), which frees the GDMA context. It may
then dereference gc through the stale service work.

Embed the service work in gdma_context and make GC_IN_SERVICE describe
whether it may still access gc. Removal and probe unwind close new
admission with GC_REMOVING and wait for an admitted cycle to retire
before clearing drvdata and freeing gc. Service exits retire before
taking the PCI rescan/remove lock, avoiding a lock-cycle with remove.

Do not admit service work while probe is still constructing or
unwinding the device. Latch reset events seen during probe and let
the probe rollback/recovery path handle them; a boundary recheck
preserves events racing probe completion.

The service work stays on the system workqueue because a reset cycle
destroys and re-creates gc->service_wq.

This issue was found by an in-house static analysis tool.

Fixes: fbe346ce9d62 ("net: mana: Handle Reset Request from MANA NIC")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Co-developed-by: Song Li <redacted>
Signed-off-by: Song Li <redacted>
Signed-off-by: Fan Wu <redacted>
---

Changes since v1 ([ref],
https://lore.kernel.org/netdev/20260805143812.220509-1-fanwu01@zju.edu.cn (local)):

- Dropped the device_lock() serialisation: holding the driver-core
  device lock across mana_gd_suspend() + msleep() + mana_gd_resume()
  blocks unbind, reboot, device PM and all PCI hotplug for up to a
  full reset cycle, and can deadlock against the
  flush_workqueue()/destroy_workqueue() of gc->service_wq.

- Replaced it with admission/drain gates: GC_REMOVING closes new
  admission and the freeing paths wait for an admitted cycle to
  retire (clear_bit_unlock/test_bit_acquire pairing) before clearing
  drvdata and calling vfree(); the failed-resume rescan no longer
  reopens admission, and mana_tx_timeout() also skips queue-reset
  work during removal.

- No longer admit service work before the probe completes (the
  reset-event handler no longer overloads GC_PROBE_SUCCEEDED with a
  mid-probe latch); the FPGA reconfig exit and the probe-failure
  recovery path are gated as well.

- Reference series for the HWC lifecycle model:
  https://lore.kernel.org/netdev/20260813174243.3044348-1-longli@microsoft.com (local)
Thanks for the updates and reference.

...
quoted hunk ↗ jump to hunk
@@ -2558,19 +2611,36 @@ static int mana_gd_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	err = mana_rdma_probe(&gc->mana_ib);
 	if (err)
-		goto cleanup_mana;
+		goto service_quiesce;
 
 	/*
 	 * If a hardware reset event has occurred over HWC during probe,
-	 * rollback and perform hardware reset procedure.
+	 * rollback and perform hardware reset procedure.  Storing the
+	 * success bit before the latch check pairs with the handler's
+	 * latch-then-recheck, so an event racing probe completion is
+	 * admitted rather than lost.
 	 */
-	if (test_and_set_bit(GC_PROBE_SUCCEEDED, &gc->flags)) {
+	set_bit(GC_PROBE_SUCCEEDED, &gc->flags);
+	if (test_and_set_bit(GC_SERVICE_DURING_PROBE, &gc->flags)) {
 		err = -EPROTO;
-		goto cleanup_mana_rdma;
+		goto service_quiesce;
 	}
 
 	return 0;
 
+service_quiesce:
+	/* The stats work can admit a service cycle once mana_probe()
+	 * has run: close admission and retire an in-flight cycle
+	 * before any teardown, like mana_gd_remove() does.  Earlier
+	 * failure points cannot have admitted service work.
+	 */
+	set_bit(GC_REMOVING, &gc->flags);
+	/* Pairs with the ordered test_and_set_bit(GC_IN_SERVICE) in
+	 * mana_schedule_serv_work().
+	 */
+	smp_mb__after_atomic();
+	wait_var_event(&gc->flags,
+		       !test_bit_acquire(GC_IN_SERVICE, &gc->flags));
 cleanup_mana_rdma:
 	mana_rdma_remove(&gc->mana_ib);
 cleanup_mana:
The cleanup_mana_rdma and cleanup_mana labels are now unused.
They should be removed.

With that addressed feel free to add:
Reviewed-by: Simon Horman <horms@kernel.org>

...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help