[PATCH v2 0/2] nvmet: fix subtle race in I/O processing and ns configuration

[PATCH v2 0/2] nvmet: fix subtle race in I/O processing and ns configuration

[Nilay Shroff]

Hi,

This series addresses two race conditions in the nvmet core code that
handles I/O submission and namespace configuration.

The first patch addresses a race where target namespace attributes
could be changed while I/O is in progress. The second patch addresses
a subtle race that could allow I/O to be admitted even after the
percpu reference count has been killed or marked dead.

As usual, feedback, comments, and suggestions are most welcome!

Thanks!

Changes from v1:
  - Replace the XArray mark used to determine whether I/O should be
    admitted with an atomic bit. Using an XArray mark would touch
    significantly more cache lines in the I/O hot path than a simple
    bit test (hch)
  - Introduce the NVMET_NS_IO_LIVE namespace flag to address the cache
    line overhead of using an XArray mark.

Nilay Shroff (2):
  nvmet: defer setting ns->enabled to false in nvmet_ns_disable()
  nvmet: don't allow I/O admission after percpu ns reference is killed

 drivers/nvme/target/core.c  | 28 ++++++++++++++++++----------
 drivers/nvme/target/nvmet.h |  2 ++
 2 files changed, 20 insertions(+), 10 deletions(-)

-- 
2.53.0

[PATCH v2 1/2] nvmet: defer setting ns->enabled to false in nvmet_ns_disable()

[Nilay Shroff]

nvmet_ns_disable() currently clears ns->enabled before draining
in-flight I/O references. This allows namespace configuration to be
changed while existing I/O requests can still hold a reference to the
namespace.

This can race with configuration of namespace attributes such as the
device path, UUID, NGUID etc. These attributes can be accessed by I/O
requests without holding subsys->lock and must not be modified while
such requests are still using the namespace.

In nvmet_ns_disable(), keep ns->enabled set while existing namespace
references are being drained, so namespace configuration remains blocked
until all in-flight I/O has completed. Set ns->enabled to false only
after the namespace references have been drained and the namespace
device has been disabled.

Introduce the NVMET_NS_IO_LIVE flag, which is set after the namespace
is successfully enabled in nvmet_ns_enable(). When nvmet_ns_disable()
starts, clear NVMET_NS_IO_LIVE so that the I/O path stops admitting new
I/O once the flag is cleared. Using test_and_clear_bit() in
nvmet_ns_disable() also prevents concurrent callers from starting a
second disable operation.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/target/core.c  | 12 +++++++-----
 drivers/nvme/target/nvmet.h |  2 ++
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 43871a8f56ca..1d4b4936bcac 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -448,7 +448,8 @@ u16 nvmet_req_find_ns(struct nvmet_req *req)
 	struct nvmet_subsys *subsys = nvmet_req_subsys(req);
 
 	req->ns = xa_load(&subsys->namespaces, nsid);
-	if (unlikely(!req->ns || !req->ns->enabled)) {
+	if (unlikely(!req->ns) ||
+	    !test_bit(NVMET_NS_IO_LIVE, &req->ns->flags)) {
 		req->error_loc = offsetof(struct nvme_common_command, nsid);
 		if (!req->ns) /* ns doesn't exist! */
 			return NVME_SC_INVALID_NS | NVME_STATUS_DNR;
@@ -623,6 +624,7 @@ int nvmet_ns_enable(struct nvmet_ns *ns)
 	ns->enabled = true;
 	xa_set_mark(&subsys->namespaces, ns->nsid, NVMET_NS_ENABLED);
 	nvmet_debugfs_ns_setup(ns);
+	set_bit(NVMET_NS_IO_LIVE, &ns->flags);
 	ret = 0;
 out_unlock:
 	mutex_unlock(&subsys->lock);
@@ -643,11 +645,11 @@ void nvmet_ns_disable(struct nvmet_ns *ns)
 	struct nvmet_subsys *subsys = ns->subsys;
 	struct nvmet_ctrl *ctrl;
 
+	if (!test_and_clear_bit(NVMET_NS_IO_LIVE, &ns->flags))
+		return;
+
 	mutex_lock(&subsys->lock);
-	if (!ns->enabled)
-		goto out_unlock;
 
-	ns->enabled = false;
 	xa_clear_mark(&subsys->namespaces, ns->nsid, NVMET_NS_ENABLED);
 	nvmet_debugfs_ns_free(ns);
 
@@ -675,7 +677,7 @@ void nvmet_ns_disable(struct nvmet_ns *ns)
 	mutex_lock(&subsys->lock);
 	nvmet_ns_changed(subsys, ns->nsid);
 	nvmet_ns_dev_disable(ns);
-out_unlock:
+	ns->enabled = false;
 	mutex_unlock(&subsys->lock);
 }
 
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index dbda55895f4f..162e2fdd848e 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -112,6 +112,8 @@ struct nvmet_ns {
 
 	bool			buffered_io;
 	bool			enabled;
+#define NVMET_NS_IO_LIVE	0
+	unsigned long		flags;
 	struct nvmet_subsys	*subsys;
 	const char		*device_path;
 
-- 
2.53.0

[PATCH v2 2/2] nvmet: don't allow I/O admission after percpu ns reference is killed

[Nilay Shroff]

nvmet_req_find_ns() uses percpu_ref_get() to obtain a reference to
the namespace. However, percpu_ref_get() can acquire a reference even
after the namespace reference has been killed (or marked DEAD).

This is undesirable during namespace disable because nvmet_ns_disable()
kills the namespace reference and then waits for all outstanding
references to drain. Acquiring a new reference after the reference is
killed can therefore extend the namespace drain period.

Replace percpu_ref_get() in nvmet_req_find_ns() with
percpu_ref_tryget_live_rcu(), which only acquires a reference while
the namespace reference is still live. This handles the race where
nvmet_req_find_ns() finds ns is admitting I/O (or it's live) but before
it acquires the reference to ns, its reference is killed in
nvmet_ns_disable(). For instance check this race:

CPU0                              CPU1
nvmet_req_find_ns():              nvmet_ns_disable():
xa_load() -> ns
IO_LIVE == set

                                  xa_clear_mark()
                                  percpu_ref_kill() // DEAD

percpu_ref_get()                  synchronize_rcu()
        |                         wait_for_completion()
        +-- succeed

Replacing percpu_ref_get() with percpu_ref_tryget_live_rcu() prevents
the I/O request from acquiring a namespace reference once the
reference has been marked DEAD.

Perform the namespace lookup and reference acquisition in
nvmet_req_find_ns() within an RCU read-side critical section.
nvmet_ns_disable() uses synchronize_rcu() before draining and exiting
the namespace reference, ensuring that RCU readers which may be
acquiring the namespace reference have completed before the reference
is exited.

Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
---
 drivers/nvme/target/core.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 1d4b4936bcac..01bb42a3e42a 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -446,21 +446,27 @@ u16 nvmet_req_find_ns(struct nvmet_req *req)
 {
 	u32 nsid = le32_to_cpu(req->cmd->common.nsid);
 	struct nvmet_subsys *subsys = nvmet_req_subsys(req);
+	u16 status = NVME_SC_SUCCESS;
 
+	rcu_read_lock();
 	req->ns = xa_load(&subsys->namespaces, nsid);
 	if (unlikely(!req->ns) ||
-	    !test_bit(NVMET_NS_IO_LIVE, &req->ns->flags)) {
+	    !test_bit(NVMET_NS_IO_LIVE, &req->ns->flags) ||
+	    !percpu_ref_tryget_live_rcu(&req->ns->ref)) {
 		req->error_loc = offsetof(struct nvme_common_command, nsid);
-		if (!req->ns) /* ns doesn't exist! */
-			return NVME_SC_INVALID_NS | NVME_STATUS_DNR;
+		if (!req->ns) { /* ns doesn't exist! */
+			status = NVME_SC_INVALID_NS | NVME_STATUS_DNR;
+			goto unlock;
+		}
 
 		/* ns exists but it's disabled */
 		req->ns = NULL;
-		return NVME_SC_INTERNAL_PATH_ERROR;
+		status = NVME_SC_INTERNAL_PATH_ERROR;
 	}
+unlock:
+	rcu_read_unlock();
 
-	percpu_ref_get(&req->ns->ref);
-	return NVME_SC_SUCCESS;
+	return status;
 }
 
 static void nvmet_destroy_namespace(struct percpu_ref *ref)
-- 
2.53.0

Re: [PATCH v2 1/2] nvmet: defer setting ns->enabled to false in nvmet_ns_disable()

[Christoph Hellwig]

Looks good:

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

Re: [PATCH v2 2/2] nvmet: don't allow I/O admission after percpu ns reference is killed

[Christoph Hellwig]

Looks good:

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

Re: [PATCH v2 0/2] nvmet: fix subtle race in I/O processing and ns configuration

[Keith Busch]

On Fri, Sep 18, 2026 at 09:57:29PM +0530, Nilay Shroff wrote:
> Hi,
> 
> This series addresses two race conditions in the nvmet core code that
> handles I/O submission and namespace configuration.
> 
> The first patch addresses a race where target namespace attributes
> could be changed while I/O is in progress. The second patch addresses
> a subtle race that could allow I/O to be admitted even after the
> percpu reference count has been killed or marked dead.

Thanks, series applied to nvme-7.4.

Re: [PATCH v2 0/2] nvmet: fix subtle race in I/O processing and ns configuration

[Keith Busch]

On Tue, Sep 22, 2026 at 08:59:54AM -0600, Keith Busch wrote:
> Thanks, series applied to nvme-7.4.

Err, I mean 7.3 for this one since these are fixes.

Re: [PATCH v2 1/2] nvmet: defer setting ns->enabled to false in nvmet_ns_disable()

[Shin'ichiro Kawasaki]

On Sep 18, 2026 / 21:57, Nilay Shroff wrote:
> nvmet_ns_disable() currently clears ns->enabled before draining
> in-flight I/O references. This allows namespace configuration to be
> changed while existing I/O requests can still hold a reference to the
> namespace.
> 
> This can race with configuration of namespace attributes such as the
> device path, UUID, NGUID etc. These attributes can be accessed by I/O
> requests without holding subsys->lock and must not be modified while
> such requests are still using the namespace.
> 
> In nvmet_ns_disable(), keep ns->enabled set while existing namespace
> references are being drained, so namespace configuration remains blocked
> until all in-flight I/O has completed. Set ns->enabled to false only
> after the namespace references have been drained and the namespace
> device has been disabled.
> 
> Introduce the NVMET_NS_IO_LIVE flag, which is set after the namespace
> is successfully enabled in nvmet_ns_enable(). When nvmet_ns_disable()
> starts, clear NVMET_NS_IO_LIVE so that the I/O path stops admitting new
> I/O once the flag is cleared. Using test_and_clear_bit() in
> nvmet_ns_disable() also prevents concurrent callers from starting a
> second disable operation.
> 
> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>

Recent blktests-ci trial runs for nvme-7.3 branch reported failure of nvme/052
[*]. It is required to repeat the test case 3 to 20 times to recreate the
failure on my test system. I bisected and find this patch is the trigger of the
failure.

Nilay, may I ask you to take a look in the failure? I'm not sure if this should
be addressed in kernel side of blktests side.

[*]

  nvme/052 (tr=loop) (Test file-ns creation/deletion under one subsystem) [failed]
      runtime    ...  12.165s
      --- tests/nvme/052.out    2026-09-29 00:36:09.236884509 +0000
      +++ /home/runner/blktests/results/nodev_tr_loop/nvme/052.out.bad  2026-09-29 01:06:09.513359622 +0000
      @@ -1,2 +1,4 @@
       Running nvme/052
      +namespace with uuid "492157d2-1f3b-4bc2-b717-e6234d0fa466" not  created within 5 seconds
      +FAIL
       Test complete

Re: [PATCH v2 1/2] nvmet: defer setting ns->enabled to false in nvmet_ns_disable()

[Nilay Shroff]

On 10/1/26 11:25 AM, Shin'ichiro Kawasaki wrote:
> On Sep 18, 2026 / 21:57, Nilay Shroff wrote:
>> nvmet_ns_disable() currently clears ns->enabled before draining
>> in-flight I/O references. This allows namespace configuration to be
>> changed while existing I/O requests can still hold a reference to the
>> namespace.
>>
>> This can race with configuration of namespace attributes such as the
>> device path, UUID, NGUID etc. These attributes can be accessed by I/O
>> requests without holding subsys->lock and must not be modified while
>> such requests are still using the namespace.
>>
>> In nvmet_ns_disable(), keep ns->enabled set while existing namespace
>> references are being drained, so namespace configuration remains blocked
>> until all in-flight I/O has completed. Set ns->enabled to false only
>> after the namespace references have been drained and the namespace
>> device has been disabled.
>>
>> Introduce the NVMET_NS_IO_LIVE flag, which is set after the namespace
>> is successfully enabled in nvmet_ns_enable(). When nvmet_ns_disable()
>> starts, clear NVMET_NS_IO_LIVE so that the I/O path stops admitting new
>> I/O once the flag is cleared. Using test_and_clear_bit() in
>> nvmet_ns_disable() also prevents concurrent callers from starting a
>> second disable operation.
>>
>> Signed-off-by: Nilay Shroff<nilay@linux.ibm.com>
> Recent blktests-ci trial runs for nvme-7.3 branch reported failure of nvme/052
> [*]. It is required to repeat the test case 3 to 20 times to recreate the
> failure on my test system. I bisected and find this patch is the trigger of the
> failure.
> 
> Nilay, may I ask you to take a look in the failure? I'm not sure if this should
> be addressed in kernel side of blktests side.

Thanks for the report! I looked into the failure and I think I found the root cause.
I couldn't reproduce it on my system, but I see there is a narrow race window that can
potentially trigger the observed failure.

I don't think this is a new race introduced by this patch. However, the changes in this
patch may have altered the timing enough to make the race easier to trigger.

In nvmet_ns_enable(), we currently queue the asynchronous namespace-change event
before setting ns->enabled and the corresponding NVMET_NS_ENABLED xarray mark.
Therefore, if the asynchronous namespace-change event is received and processed
by the host before the namespace is fully enabled on the target, the host may not
find the namespace while scanning the active namespace list.

To close this race, I think we should send the namespace-change notification only
after the namespace has been fully enabled in nvmet_ns_enable(). In particular,
we can move nvmet_ns_changed() after setting ns->enabled, the NVMET_NS_ENABLED
xarray mark, and NVMET_NS_IO_LIVE.

Could you please try the following change on your test system and let me know if
it fixes the failure? If this resolves the issue, I'll send a formal patch:

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 8eea0a504308..da98e5cae7f6 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -626,11 +626,11 @@ int nvmet_ns_enable(struct nvmet_ns *ns)
         if (ret)
                 goto out_pr_exit;

-       nvmet_ns_changed(subsys, ns->nsid);
         ns->enabled = true;
         xa_set_mark(&subsys->namespaces, ns->nsid, NVMET_NS_ENABLED);
         nvmet_debugfs_ns_setup(ns);
         set_bit(NVMET_NS_IO_LIVE, &ns->flags);
+       nvmet_ns_changed(subsys, ns->nsid);
         ret = 0;
  out_unlock:
         mutex_unlock(&subsys->lock);