[PATCH v2] nvme: stop asynchronous event work before ANA teardown

[PATCH v2] nvme: stop asynchronous event work before ANA teardown

[Yaohan Li]

NVMe ANA work has several producers, including ANA AER completion,
namespace scanning and failover processing.

An AER completion can queue ana_work for an ANA notice and can
separately queue async_event_work, which sends the AEN uevent and
submits the next asynchronous event request.  These work items access
controller-owned state during asynchronous processing.

Controller teardown must ensure that both the event work and ANA
processing have completed before their associated resources are
released.  Cancel async_event_work during controller uninitialization,
after the controller has stopped accepting new events, and cancel
ana_work again before releasing its ANA resources.  Draining only
ana_work is not sufficient: an ANA notice completion can still queue it
while teardown is in progress.

ana_work is initialized in nvme_mpath_init_ctrl() during
nvme_init_ctrl(), before any teardown path can run.  During controller
removal its producers are dead before the final cancel in
nvme_mpath_uninit(): namespace scanning is flushed in
nvme_remove_namespaces(), path clearing removes the failover source,
the transport's delete_ctrl tears down the admin queue so no further
AEN completions can queue work, and nvme_enable_aen() is only
reachable from nvme_start_ctrl().

nvme_mpath_uninit() also runs from nvme_mpath_init_identify(): from
the ANA log growth path, where nvme_mpath_stop() has already
cancelled ana_work, and from its out_uninit error exit, which can be
reached without nvme_mpath_stop() when reading the ANA log fails.
nvme_mpath_init_identify() is called from nvme_init_ctrl_finish(),
while the controller is CONNECTING or RESETTING (LIVE is set at the
end of nvme_start_ctrl()), and nvme_ana_work() returns immediately
unless the controller is LIVE, so in both cases the additional cancel
only drains work that would be a no-op and cannot race a work using
the ANA buffer.

This issue was investigated and the fix was developed with AI
assistance (ZCode/GLM-4).

The race was reproduced with the in-tree NVMe loop target and
scheduling injection.  Cancelling ANA work alone left reports;
draining the event work and the ANA work together removed the
reported use-after-free under the same test conditions.

Fixes: 0d0b660f214dc ("nvme: add ANA support")
Cc: stable@vger.kernel.org
Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
---

Changes since v1:
- Corrected the causal description: an AER completion queues ana_work
  (via the ANA notice handler) and separately queues
  async_event_work, which sends the AEN uevent and submits the next
  request.  v1 wrongly attributed ana_work re-queueing to the event
  worker itself.
- Noted that ana_work is initialized in nvme_mpath_init_ctrl() during
  nvme_init_ctrl(), before any teardown path, and documented all
  ana_work producers in comments.
- Audited the producer shutdown order for the final controller
  removal: nvme_remove_namespaces() flushes scan_work, path clearing
  removes the failover source, the transport's delete_ctrl tears down
  the admin queue, and nvme_enable_aen() is only reachable from
  nvme_start_ctrl().
- Corrected the error-path claim: nvme_mpath_init_identify()'s
  out_uninit exit is reachable without nvme_mpath_stop() (when reading
  the ANA log fails without growing the buffer).  nvme_ana_work()
  returns immediately unless the controller is LIVE, and
  nvme_init_ctrl_finish() runs while the controller is CONNECTING or
  RESETTING, so the extra cancel on those paths only drains a no-op
  work.  No additional cancel_work_sync() calls were added.
- Addressed the Sashiko review note on the explanatory text: the
  comment in nvme_uninit_ctrl() and the commit message now describe the
  actual queueing paths — an AER completion queues async_event_work
  (uevent plus the next request) and separately queues ana_work via the
  ANA notice handler, while namespace scanning and the failover path are
  the other ana_work producers — instead of attributing ana_work
  re-queueing to nvme_async_event_work().
- No functional changes beyond v1.
v1: https://lore.kernel.org/r/179378587000.73780.18152328009023418065@gmail.com/

 drivers/nvme/host/core.c      | 10 ++++++++++
 drivers/nvme/host/multipath.c |  9 +++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index a7a4d09644c13..8efa306205e19 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -5182,6 +5182,16 @@ EXPORT_SYMBOL_GPL(nvme_start_ctrl);
 void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
 {
 	nvme_stop_keep_alive(ctrl);
+
+	/*
+	 * An AER completion can queue async_event_work; drain it
+	 * before the controller's final release.  Note that ana_work
+	 * has additional producers (namespace scanning via
+	 * nvme_scan_work, and the failover path in
+	 * nvme_failover_req), which is why nvme_mpath_uninit()
+	 * separately cancels ana_work after this point.
+	 */
+	cancel_work_sync(&ctrl->async_event_work);
 	nvme_hwmon_exit(ctrl);
 	nvme_fault_inject_fini(&ctrl->fault_inject);
 	dev_pm_qos_hide_latency_tolerance(ctrl->device);
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 9e0a0fd77c56e..4ced8136d5f63 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -1610,6 +1610,15 @@ int nvme_mpath_init_identify(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
 
 void nvme_mpath_uninit(struct nvme_ctrl *ctrl)
 {
+	/*
+	 * ANA work can be queued by AER completion (via
+	 * nvme_handle_aen_notice), namespace scanning
+	 * (nvme_scan_work), and the failover path
+	 * (nvme_failover_req).  Drain any remaining work before
+	 * releasing the ANA resources.
+	 */
+	cancel_work_sync(&ctrl->ana_work);
+
 	kvfree(ctrl->ana_log_buf);
 	ctrl->ana_log_buf = NULL;
 	ctrl->ana_log_size = 0;

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.34.1