[PATCH v2 0/2] nvme: fix removing namespaces under SRCU readers

[PATCH v2 0/2] nvme: fix removing namespaces under SRCU readers

[Qiliang Yuan]

nvme_remove_namespaces() splices the namespace list with
list_splice_init_rcu() and synchronize_rcu(), but the list is read
under SRCU. A reader still on the last namespace can then walk onto the
on-stack list head, which KASAN reports as a stack-out-of-bounds read.

Patch 1 adds list_splice_init_srcu() for lists read under SRCU, and
patch 2 uses it in nvme. Both are needed for the fix.

Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
V1 -> V2:
- Add list_splice_init_srcu() instead of open-coding the splice in nvme
- Put it in srcu.h, as rculist.h doesn't include srcu.h
- Update the numbers from a new run, median of 3 runs per boot

v1: https://lore.kernel.org/r/20261103-bug-nvme-remove-ns-srcu-v1-1-e239087d63ac@gmail.com

---
Qiliang Yuan (2):
      srcu: Add list_splice_init_srcu()
      nvme: wait for SRCU readers before splicing the namespace list

 drivers/nvme/host/core.c |  3 +--
 include/linux/srcu.h     | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261102-bug-nvme-remove-ns-srcu-93d8a75a9b1b

Best regards,
-- 
Qiliang Yuan <odys.yuan@gmail.com>

[PATCH v2 1/2] srcu: Add list_splice_init_srcu()

[Qiliang Yuan]

list_splice_init_rcu() makes a list look empty to new readers, waits for
the current ones with the given sync function, and only then links the
entries into the new list. The sync function takes no argument, so it
can wait for RCU readers but not for the readers of an srcu_struct.

Passing synchronize_rcu() for a list read under SRCU still builds, but
leaves SRCU readers on the entries while the last one's next pointer is
moved onto the new list head. nvme_remove_namespaces() does this today.

Add list_splice_init_srcu(), which waits for the readers of the given
srcu_struct instead.

Cc: stable@vger.kernel.org
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
 include/linux/srcu.h | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 1a8a465a5650d..d4dcaeb83e2b6 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -18,6 +18,7 @@
 
 #include <linux/mutex.h>
 #include <linux/rcupdate.h>
+#include <linux/rculist.h>
 #include <linux/workqueue.h>
 #include <linux/rcu_segcblist.h>
 
@@ -118,6 +119,41 @@ void cleanup_srcu_struct(struct srcu_struct *ssp);
 void synchronize_srcu(struct srcu_struct *ssp);
 void synchronize_srcu_atomic(struct srcu_struct *ssp);
 
+/**
+ * list_splice_init_srcu - splice an SRCU-protected list into an existing list
+ * @list:	the SRCU-protected list to splice
+ * @head:	the place in the existing list to splice the first list into
+ * @ssp:	the srcu_struct that readers of @list use
+ *
+ * Like list_splice_init_rcu(), but wait for the SRCU readers of @list
+ * instead of RCU ones, so that none of them can follow a next pointer
+ * onto @head.
+ *
+ * Note that this function blocks, and that the caller must prevent any
+ * other updates to @list.
+ */
+static inline void list_splice_init_srcu(struct list_head *list,
+					 struct list_head *head,
+					 struct srcu_struct *ssp)
+{
+	struct list_head *first = list->next;
+	struct list_head *last = list->prev;
+	struct list_head *next = head->next;
+
+	if (list_empty(list))
+		return;
+
+	INIT_LIST_HEAD_RCU(list);
+	synchronize_srcu(ssp);
+	ASSERT_EXCLUSIVE_ACCESS(*first);
+	ASSERT_EXCLUSIVE_ACCESS(*last);
+
+	last->next = next;
+	rcu_assign_pointer(list_next_rcu(head), first);
+	first->prev = head;
+	next->prev = last;
+}
+
 #define SRCU_GET_STATE_COMPLETED 0x1
 
 /**

-- 
2.43.0

Re: [PATCH v2 1/2] srcu: Add list_splice_init_srcu()

[Paul E. McKenney]

On Wed, Nov 04, 2026 at 01:30:39AM -0500, Qiliang Yuan wrote:
> list_splice_init_rcu() makes a list look empty to new readers, waits for
> the current ones with the given sync function, and only then links the
> entries into the new list. The sync function takes no argument, so it
> can wait for RCU readers but not for the readers of an srcu_struct.
> 
> Passing synchronize_rcu() for a list read under SRCU still builds, but
> leaves SRCU readers on the entries while the last one's next pointer is
> moved onto the new list head. nvme_remove_namespaces() does this today.
> 
> Add list_splice_init_srcu(), which waits for the readers of the given
> srcu_struct instead.
> 
> Cc: stable@vger.kernel.org
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>

I am not inalterably opposed to adding a list_splice_init_srcu(),
especially if quite a few more users show up, but here is what I was
expecting people to do:

	static void synchronize_my_srcu(void)
	{
		synchronize_srcu(&my_srcu);
	}

	...

	list_splice_init_rcu(l, h, synchronize_my_srcu);

We need at least six uses of the above idiom to break even on the line
of code required for introducing a list_splice_init_srcu().

							Thanx, Paul

> ---
>  include/linux/srcu.h | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> index 1a8a465a5650d..d4dcaeb83e2b6 100644
> --- a/include/linux/srcu.h
> +++ b/include/linux/srcu.h
> @@ -18,6 +18,7 @@
>  
>  #include <linux/mutex.h>
>  #include <linux/rcupdate.h>
> +#include <linux/rculist.h>
>  #include <linux/workqueue.h>
>  #include <linux/rcu_segcblist.h>
>  
> @@ -118,6 +119,41 @@ void cleanup_srcu_struct(struct srcu_struct *ssp);
>  void synchronize_srcu(struct srcu_struct *ssp);
>  void synchronize_srcu_atomic(struct srcu_struct *ssp);
>  
> +/**
> + * list_splice_init_srcu - splice an SRCU-protected list into an existing list
> + * @list:	the SRCU-protected list to splice
> + * @head:	the place in the existing list to splice the first list into
> + * @ssp:	the srcu_struct that readers of @list use
> + *
> + * Like list_splice_init_rcu(), but wait for the SRCU readers of @list
> + * instead of RCU ones, so that none of them can follow a next pointer
> + * onto @head.
> + *
> + * Note that this function blocks, and that the caller must prevent any
> + * other updates to @list.
> + */
> +static inline void list_splice_init_srcu(struct list_head *list,
> +					 struct list_head *head,
> +					 struct srcu_struct *ssp)
> +{
> +	struct list_head *first = list->next;
> +	struct list_head *last = list->prev;
> +	struct list_head *next = head->next;
> +
> +	if (list_empty(list))
> +		return;
> +
> +	INIT_LIST_HEAD_RCU(list);
> +	synchronize_srcu(ssp);
> +	ASSERT_EXCLUSIVE_ACCESS(*first);
> +	ASSERT_EXCLUSIVE_ACCESS(*last);
> +
> +	last->next = next;
> +	rcu_assign_pointer(list_next_rcu(head), first);
> +	first->prev = head;
> +	next->prev = last;
> +}
> +
>  #define SRCU_GET_STATE_COMPLETED 0x1
>  
>  /**
> 
> -- 
> 2.43.0
> 

Re: [PATCH v2 1/2] srcu: Add list_splice_init_srcu()

[Qiliang Yuan]

On Wed, Nov 04, 2026 at 09:14:12AM -0800, Paul E. McKenney wrote:
> I am not inalterably opposed to adding a list_splice_init_srcu(),
> especially if quite a few more users show up, but here is what I was
> expecting people to do:
>
> 	static void synchronize_my_srcu(void)
> 	{
> 		synchronize_srcu(&my_srcu);
> 	}
>
> 	...
>
> 	list_splice_init_rcu(l, h, synchronize_my_srcu);
>
> We need at least six uses of the above idiom to break even on the line
> of code required for introducing a list_splice_init_srcu().

That works for a global srcu_struct, but nvme has one per controller,
next to the list it protects:

	struct nvme_ctrl {
		...
		struct list_head namespaces;
		struct mutex namespaces_lock;
		struct srcu_struct srcu;
		...
	};

nvme_init_ctrl() calls init_srcu_struct(&ctrl->srcu) for every
controller, and readers walk the list under that controller's srcu,
e.g. nvme_wait_freeze():

	srcu_idx = srcu_read_lock(&ctrl->srcu);
	list_for_each_entry_srcu(ns, &ctrl->namespaces, list,
				 srcu_read_lock_held(&ctrl->srcu))
		blk_mq_freeze_queue_wait(ns->queue);
	srcu_read_unlock(&ctrl->srcu, srcu_idx);

nvme_remove_namespaces() thus has to wait on the srcu of the controller
being deleted, which a sync function that takes no argument can't get
at.

So the choice here is between list_splice_init_srcu(), as Christoph
suggested on v1, and open-coding the splice in nvme_remove_namespaces()
as v1 did. Which would you prefer?

Thanks,
Qiliang

Re: [PATCH v2 1/2] srcu: Add list_splice_init_srcu()

[Paul E. McKenney]

On Wed, Nov 04, 2026 at 08:22:42PM -0500, Qiliang Yuan wrote:
> On Wed, Nov 04, 2026 at 09:14:12AM -0800, Paul E. McKenney wrote:
> > I am not inalterably opposed to adding a list_splice_init_srcu(),
> > especially if quite a few more users show up, but here is what I was
> > expecting people to do:
> >
> > 	static void synchronize_my_srcu(void)
> > 	{
> > 		synchronize_srcu(&my_srcu);
> > 	}
> >
> > 	...
> >
> > 	list_splice_init_rcu(l, h, synchronize_my_srcu);
> >
> > We need at least six uses of the above idiom to break even on the line
> > of code required for introducing a list_splice_init_srcu().
> 
> That works for a global srcu_struct, but nvme has one per controller,
> next to the list it protects:
> 
> 	struct nvme_ctrl {
> 		...
> 		struct list_head namespaces;
> 		struct mutex namespaces_lock;
> 		struct srcu_struct srcu;
> 		...
> 	};
> 
> nvme_init_ctrl() calls init_srcu_struct(&ctrl->srcu) for every
> controller, and readers walk the list under that controller's srcu,
> e.g. nvme_wait_freeze():
> 
> 	srcu_idx = srcu_read_lock(&ctrl->srcu);
> 	list_for_each_entry_srcu(ns, &ctrl->namespaces, list,
> 				 srcu_read_lock_held(&ctrl->srcu))
> 		blk_mq_freeze_queue_wait(ns->queue);
> 	srcu_read_unlock(&ctrl->srcu, srcu_idx);
> 
> nvme_remove_namespaces() thus has to wait on the srcu of the controller
> being deleted, which a sync function that takes no argument can't get
> at.
> 
> So the choice here is between list_splice_init_srcu(), as Christoph
> suggested on v1, and open-coding the splice in nvme_remove_namespaces()
> as v1 did. Which would you prefer?

Sigh!  I hereby withdraw all objections to list_splice_init_srcu(),
and thank you for bearing with me.

(Yes, you could squirrel away a pointer to the srcu_struct structure
in the task_struct structure or some such, but that cure is worse than
the disease.)

							Thanx, Paul

Re: [PATCH v2 1/2] srcu: Add list_splice_init_srcu()

[Qiliang Yuan]

On Wed, Nov 04, 2026 at 09:14:12AM -0800, Paul E. McKenney wrote:
> I am not inalterably opposed to adding a list_splice_init_srcu(),
> especially if quite a few more users show up, but here is what I was
> expecting people to do:
>
> 	static void synchronize_my_srcu(void)
> 	{
> 		synchronize_srcu(&my_srcu);
> 	}
>
> 	...
>
> 	list_splice_init_rcu(l, h, synchronize_my_srcu);
>
> We need at least six uses of the above idiom to break even on the line
> of code required for introducing a list_splice_init_srcu().

Christoph, as Paul would rather not add a helper for a single user, and
the idiom above doesn't fit nvme's per-controller srcu_struct, would you
be fine with taking v1, which open-codes the splice in
nvme_remove_namespaces()?

  https://lore.kernel.org/r/20261103-bug-nvme-remove-ns-srcu-v1-1-e239087d63ac@gmail.com

It fixes a crash and is meant for stable: a reader in nvme_wait_freeze()
can walk onto the on-stack list head and use stack memory as a struct
nvme_ns, which KASAN reports as a stack-out-of-bounds read, followed by
a page fault. A shared helper can follow if more SRCU users of
list_splice_init_rcu() show up.

Thanks,
Qiliang

Re: [PATCH v2 1/2] srcu: Add list_splice_init_srcu()

[Paul E. McKenney]

On Wed, Nov 04, 2026 at 09:14:13AM -0800, Paul E. McKenney wrote:
> On Wed, Nov 04, 2026 at 01:30:39AM -0500, Qiliang Yuan wrote:
> > list_splice_init_rcu() makes a list look empty to new readers, waits for
> > the current ones with the given sync function, and only then links the
> > entries into the new list. The sync function takes no argument, so it
> > can wait for RCU readers but not for the readers of an srcu_struct.
> > 
> > Passing synchronize_rcu() for a list read under SRCU still builds, but
> > leaves SRCU readers on the entries while the last one's next pointer is
> > moved onto the new list head. nvme_remove_namespaces() does this today.
> > 
> > Add list_splice_init_srcu(), which waits for the readers of the given
> > srcu_struct instead.
> > 
> > Cc: stable@vger.kernel.org
> > Suggested-by: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
> 
> I am not inalterably opposed to adding a list_splice_init_srcu(),
> especially if quite a few more users show up, but here is what I was
> expecting people to do:
> 
> 	static void synchronize_my_srcu(void)
> 	{
> 		synchronize_srcu(&my_srcu);
> 	}
> 
> 	...
> 
> 	list_splice_init_rcu(l, h, synchronize_my_srcu);
> 
> We need at least six uses of the above idiom to break even on the line
> of code required for introducing a list_splice_init_srcu().

Given Qiliang's response:

Reviewed-by: Paul E. McKenney <paulmck@kernel.org>

							Thanx, Paul

> > ---
> >  include/linux/srcu.h | 36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > index 1a8a465a5650d..d4dcaeb83e2b6 100644
> > --- a/include/linux/srcu.h
> > +++ b/include/linux/srcu.h
> > @@ -18,6 +18,7 @@
> >  
> >  #include <linux/mutex.h>
> >  #include <linux/rcupdate.h>
> > +#include <linux/rculist.h>
> >  #include <linux/workqueue.h>
> >  #include <linux/rcu_segcblist.h>
> >  
> > @@ -118,6 +119,41 @@ void cleanup_srcu_struct(struct srcu_struct *ssp);
> >  void synchronize_srcu(struct srcu_struct *ssp);
> >  void synchronize_srcu_atomic(struct srcu_struct *ssp);
> >  
> > +/**
> > + * list_splice_init_srcu - splice an SRCU-protected list into an existing list
> > + * @list:	the SRCU-protected list to splice
> > + * @head:	the place in the existing list to splice the first list into
> > + * @ssp:	the srcu_struct that readers of @list use
> > + *
> > + * Like list_splice_init_rcu(), but wait for the SRCU readers of @list
> > + * instead of RCU ones, so that none of them can follow a next pointer
> > + * onto @head.
> > + *
> > + * Note that this function blocks, and that the caller must prevent any
> > + * other updates to @list.
> > + */
> > +static inline void list_splice_init_srcu(struct list_head *list,
> > +					 struct list_head *head,
> > +					 struct srcu_struct *ssp)
> > +{
> > +	struct list_head *first = list->next;
> > +	struct list_head *last = list->prev;
> > +	struct list_head *next = head->next;
> > +
> > +	if (list_empty(list))
> > +		return;
> > +
> > +	INIT_LIST_HEAD_RCU(list);
> > +	synchronize_srcu(ssp);
> > +	ASSERT_EXCLUSIVE_ACCESS(*first);
> > +	ASSERT_EXCLUSIVE_ACCESS(*last);
> > +
> > +	last->next = next;
> > +	rcu_assign_pointer(list_next_rcu(head), first);
> > +	first->prev = head;
> > +	next->prev = last;
> > +}
> > +
> >  #define SRCU_GET_STATE_COMPLETED 0x1
> >  
> >  /**
> > 
> > -- 
> > 2.43.0
> > 

Re: [PATCH v2 1/2] srcu: Add list_splice_init_srcu()

[Qiliang Yuan]

On Wed, Nov 04, 2026 at 05:46:45PM -0800, Paul E. McKenney wrote:
> On Wed, Nov 04, 2026 at 09:14:13AM -0800, Paul E. McKenney wrote:
> > I am not inalterably opposed to adding a list_splice_init_srcu(),
> > especially if quite a few more users show up, but here is what I was
> > expecting people to do:
> >
> > 	static void synchronize_my_srcu(void)
> > 	{
> > 		synchronize_srcu(&my_srcu);
> > 	}
> >
> > 	...
> >
> > 	list_splice_init_rcu(l, h, synchronize_my_srcu);
> >
> > We need at least six uses of the above idiom to break even on the line
> > of code required for introducing a list_splice_init_srcu().
>
> Given Qiliang's response:
>
> Reviewed-by: Paul E. McKenney <paulmck@kernel.org>

Thanks, Paul.

Christoph, with that, please disregard my suggestion to take v1
instead; v2 is the one to apply.

Thanks,
Qiliang

Re: [PATCH v2 1/2] srcu: Add list_splice_init_srcu()

[Christoph Hellwig]

On Wed, Nov 04, 2026 at 05:46:45PM -0800, Paul E. McKenney wrote:
> Given Qiliang's response:
> 
> Reviewed-by: Paul E. McKenney <paulmck@kernel.org>

Thanks.  I assume you're fine if we carry this in the nvme tree?

Re: [PATCH v2 1/2] srcu: Add list_splice_init_srcu()

[Paul E. McKenney]

On Thu, Nov 05, 2026 at 08:45:54AM +0100, Christoph Hellwig wrote:
> On Wed, Nov 04, 2026 at 05:46:45PM -0800, Paul E. McKenney wrote:
> > Given Qiliang's response:
> > 
> > Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
> 
> Thanks.  I assume you're fine if we carry this in the nvme tree?

Please do!

							Thanx, Paul

Re: [PATCH v2 1/2] srcu: Add list_splice_init_srcu()

[Christoph Hellwig]

Looks good:

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

[PATCH v2 2/2] nvme: wait for SRCU readers before splicing the namespace list

[Qiliang Yuan]

Readers walk ctrl->namespaces under ctrl->srcu, and some of them sleep
there, e.g. nvme_wait_freeze() waiting for I/O to drain.

nvme_remove_namespaces() moves the list onto its on-stack ns_list with
list_splice_init_rcu(), which only waits for RCU readers before
relinking the entries. A reader still on the last namespace then follows
its next pointer to ns_list instead of ctrl->namespaces and treats the
stack as a namespace. Deleting a controller while a Format NVM
passthrough waits for a delayed read on the last namespace gives:

  BUG: KASAN: stack-out-of-bounds in nvme_wait_freeze+0x102/0x120
  Read of size 8 at addr ff110001d2f3fc38 by task nvme/4413
  Call Trace:
   nvme_wait_freeze
   nvme_passthru_start
   nvme_submit_user_cmd
   nvme_user_cmd
   nvme_dev_ioctl
  BUG: unable to handle page fault for address: ffffffff8af04850

Use list_splice_init_srcu() instead, which waits for the SRCU readers.
This also takes the RCU grace period out of every controller deletion.
Deleting 64 tcp controllers with one namespace each, median of 3 runs on
each of two boots per kernel:

                                      before        after
  nvme disconnect-all (ms)            3491 / 3509   2996 / 3165
  nvme_remove_namespaces() avg (ms)   39.0 / 40.5   31.9 / 34.7

Fixes: be647e2c76b2 ("nvme: use srcu for iterating namespace list")
Cc: stable@vger.kernel.org
Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
 drivers/nvme/host/core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index a7a4d09644c13..cdb576db23809 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4755,9 +4755,8 @@ void nvme_remove_namespaces(struct nvme_ctrl *ctrl)
 	nvme_change_ctrl_state(ctrl, NVME_CTRL_DELETING_NOIO);
 
 	mutex_lock(&ctrl->namespaces_lock);
-	list_splice_init_rcu(&ctrl->namespaces, &ns_list, synchronize_rcu);
+	list_splice_init_srcu(&ctrl->namespaces, &ns_list, &ctrl->srcu);
 	mutex_unlock(&ctrl->namespaces_lock);
-	synchronize_srcu(&ctrl->srcu);
 
 	list_for_each_entry_safe(ns, next, &ns_list, list)
 		nvme_ns_remove(ns);

-- 
2.43.0

Re: [PATCH v2 2/2] nvme: wait for SRCU readers before splicing the namespace list

[Christoph Hellwig]

Looks good:

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