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

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

[Yaohan Li]

nvme_mpath_stop() cancels ana_work, but the asynchronous event worker
can enqueue ANA processing again while controller teardown is in
progress. An ANA work item can therefore survive the final
controller release and access freed memory.

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 while the event worker can requeue it.

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

Fixes: 0d0b660f214dc ("nvme: add ANA support")
Assisted-by: ZCode:GLM-4
Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
---
 drivers/nvme/host/core.c      | 9 +++++++++
 drivers/nvme/host/multipath.c | 7 +++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index a7a4d09644c13..017aae1189de1 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -5182,6 +5182,15 @@ EXPORT_SYMBOL_GPL(nvme_start_ctrl);
 void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
 {
 	nvme_stop_keep_alive(ctrl);
+
+	/*
+	 * Stop the async event worker before the final put: it processes
+	 * ANA change events by re-queueing ctrl->ana_work, which is
+	 * embedded in ctrl and cancelled in nvme_free_ctrl(); a worker
+	 * running past that point would re-queue it onto freed memory.
+	 */
+	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..44bd1a1ad5940 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -1610,6 +1610,13 @@ int nvme_mpath_init_identify(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
 
 void nvme_mpath_uninit(struct nvme_ctrl *ctrl)
 {
+	/*
+	 * nvme_mpath_stop() already cancelled ana_work, but an ANA async
+	 * event completing after nvme_stop_ctrl() can have re-queued it.
+	 * It is embedded in ctrl, which is freed after uninit.
+	 */
+	cancel_work_sync(&ctrl->ana_work);
+
 	kvfree(ctrl->ana_log_buf);
 	ctrl->ana_log_buf = NULL;
 	ctrl->ana_log_size = 0;
-- 
2.34.1

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

[Christoph Hellwig]

The fix looks good, but it would be nice to get the Sashiko nit about the
comment fixed.

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

[Hannes Reinecke]

On 11/4/26 10:51 AM, Yaohan Li wrote:
> nvme_mpath_stop() cancels ana_work, but the asynchronous event worker
> can enqueue ANA processing again while controller teardown is in
> progress. An ANA work item can therefore survive the final
> controller release and access freed memory.
> 
> 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 while the event worker can requeue it.
> 
> The race was reproduced with the in-tree NVMe loop target and
> scheduling injection. Cancelling ANA work alone left reports; draining
> the event source and the ANA work together removed the reported
> use-after-free under the same test conditions.
> 
> Fixes: 0d0b660f214dc ("nvme: add ANA support")
> Assisted-by: ZCode:GLM-4
> Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
> ---
>   drivers/nvme/host/core.c      | 9 +++++++++
>   drivers/nvme/host/multipath.c | 7 +++++++
>   2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index a7a4d09644c13..017aae1189de1 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -5182,6 +5182,15 @@ EXPORT_SYMBOL_GPL(nvme_start_ctrl);
>   void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
>   {
>   	nvme_stop_keep_alive(ctrl);
> +
> +	/*
> +	 * Stop the async event worker before the final put: it processes
> +	 * ANA change events by re-queueing ctrl->ana_work, which is
> +	 * embedded in ctrl and cancelled in nvme_free_ctrl(); a worker
> +	 * running past that point would re-queue it onto freed memory.
> +	 */
> +	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..44bd1a1ad5940 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -1610,6 +1610,13 @@ int nvme_mpath_init_identify(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
>   
>   void nvme_mpath_uninit(struct nvme_ctrl *ctrl)
>   {
> +	/*
> +	 * nvme_mpath_stop() already cancelled ana_work, but an ANA async
> +	 * event completing after nvme_stop_ctrl() can have re-queued it.
> +	 * It is embedded in ctrl, which is freed after uninit.
> +	 */
> +	cancel_work_sync(&ctrl->ana_work);
> +
>   	kvfree(ctrl->ana_log_buf);
>   	ctrl->ana_log_buf = NULL;
>   	ctrl->ana_log_size = 0;

Why can we schedule AENs when a controller teardown is active?
Shouldn't we rather disallow that instead of fixing the fallout?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

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

[Yaohan Li]

Hi Hannes,

That's a fair point. I agree that we shouldn't submit new AER requests once teardown has started.

The case I was trying to cover is a completion for an AER that was already outstanding. nvme_stop_ctrl() flushes async_event_work, but a completion can still queue it again before the transport has finished shutting down the admin queue.

In v2, I added the final cancellation in nvme_uninit_ctrl(), after the transport teardown, and kept the separate cancellation of ana_work before releasing its resources. Keith has since applied that version to nvme-7.4.

I agree that stopping the producer is preferable where possible. I don't think the final cancellation should be treated as a substitute for quiescing the completion path, and I'll check whether any transport can still deliver a callback beyond that point.

Thanks,
Yaohan

[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

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

[Christoph Hellwig]

On Thu, Nov 05, 2026 at 07:41:49PM +0800, Yaohan Li wrote:
> 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.

I think we can cut the commit log after this..

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

[Keith Busch]

On Thu, 5 Nov 2026 19:41:49 +0800, Yaohan Li wrote:
> 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.
> 
> [...]

Thanks, applied to nvme-7.4.

[1/1] nvme: stop asynchronous event work before ANA teardown
      commit: 9b657c565984f28976b1e471e3ce2f81cc269238
--
Keith Busch <kbusch@kernel.org>