All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nvmet: clamp the last ANA group index against the real array size
@ 2026-11-06  6:40 Yuehong Wu
  2026-11-06  7:03 ` Yuehong Wu
  2026-11-12 21:21 ` Christoph Hellwig
  0 siblings, 2 replies; 4+ messages in thread
From: Yuehong Wu @ 2026-11-06  6:40 UTC (permalink / raw)
  To: linux-nvme
  Cc: Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni, Keith Busch,
	Maurizio Lombardi, Mohamed Khalfella, Nitesh Shetty,
	Hannes Reinecke, Randy Dunlap, Aurelien Aptel, Nilay Shroff,
	Jesse Taube, Rongzheng Wang, Kees Cook, Rosen Penev, Guixin Liu,
	Qing Zhang, Eric Biggers, Julian Sun, Yu Zhang, linux-kernel

NVMET_MAX_ANAGRPS is the highest valid ANA group id, not the size of
nvmet_ana_group_enabled[]: the array has NVMET_MAX_ANAGRPS + 1 entries
and the controller advertises anagrpmax == NVMET_MAX_ANAGRPS.  Both
array_index_nospec() calls however pass NVMET_MAX_ANAGRPS as the size,
so for the last group the mask is 0 and the index quietly becomes 0:

 - nvmet_ana_groups_make_group() increments enabled[0] while the group
   keeps grpid == 128, so the later nvmet_ana_group_release() decrements
   enabled[128], which underflows to 0xffffffff.  The ANA log page then
   reports group 128 as inaccessible forever even though the configfs
   group no longer exists, and the increment on enabled[0] is never
   undone.

 - nvmet_ns_ana_grpid_store() accepts 128, stores 0 in ns->anagrpid and
   returns success, so the namespace silently stays in the default ANA
   group instead of the one the user asked for.

Both failures are silent, and they change what the target reports in its
ANA log page, which is what a host uses to select paths.

Pass the real array size in both calls, so the masking is kept but the
last group is no longer folded onto index 0.

Fixes: 20dc66f2d76b ("nvme: prevent potential spectre v1 gadget")
Cc: stable@vger.kernel.org
Signed-off-by: Yuehong Wu <wuyuehong@cmss.chinamobile.com>
---
Verified with nvme-loop in QEMU/KVM on 7.3.0-next-20261027, with two
kernels built from the same tree and differing only in these two lines
(no instrumentation):

 - before the patch, a group created as ports/1/ana_groups/128 is absent
   from the ANA log page (ngrps=1) even though configfs accepted it, and
   it is reported as expected afterwards (ngrps=2, group 128 optimized);
 - 128 written to a namespace ana_grpid reads back as 0 before the patch
   and as 128 after it, and the namespace is accounted to group 128;
 - removing the group again adds a phantom group 128 (state inaccessible,
   0 namespaces) to the ANA log page before the patch, and leaves no
   trace after it, even when creating and removing it repeatedly.

 drivers/nvme/target/configfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 8aea64d..48d3f68 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -701,7 +701,7 @@ static ssize_t nvmet_ns_ana_grpid_store(struct config_item *item,
 
 	down_write(&nvmet_ana_sem);
 	oldgrpid = ns->anagrpid;
-	newgrpid = array_index_nospec(newgrpid, NVMET_MAX_ANAGRPS);
+	newgrpid = array_index_nospec(newgrpid, NVMET_MAX_ANAGRPS + 1);
 	nvmet_ana_group_enabled[newgrpid]++;
 	ns->anagrpid = newgrpid;
 	nvmet_ana_group_enabled[oldgrpid]--;
@@ -2074,7 +2074,7 @@ static struct config_group *nvmet_ana_groups_make_group(
 	grp->grpid = grpid;
 
 	down_write(&nvmet_ana_sem);
-	grpid = array_index_nospec(grpid, NVMET_MAX_ANAGRPS);
+	grpid = array_index_nospec(grpid, NVMET_MAX_ANAGRPS + 1);
 	nvmet_ana_group_enabled[grpid]++;
 	up_write(&nvmet_ana_sem);
 
-- 
2.43.0




^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] nvmet: clamp the last ANA group index against the real array size
  2026-11-06  6:40 [PATCH] nvmet: clamp the last ANA group index against the real array size Yuehong Wu
@ 2026-11-06  7:03 ` Yuehong Wu
  2026-11-06 13:47   ` Andrew Stellman
  2026-11-12 21:21 ` Christoph Hellwig
  1 sibling, 1 reply; 4+ messages in thread
From: Yuehong Wu @ 2026-11-06  7:03 UTC (permalink / raw)
  To: linux-nvme
  Cc: Christoph Hellwig, Sagi Grimberg, Andrew Stellman,
	Chaitanya Kulkarni, Keith Busch, Maurizio Lombardi,
	Mohamed Khalfella, Nitesh Shetty, Hannes Reinecke, Randy Dunlap,
	Aurelien Aptel, Nilay Shroff, Jesse Taube, Rongzheng Wang,
	Kees Cook, Rosen Penev, Guixin Liu, Qing Zhang, Eric Biggers,
	Julian Sun, Yu Zhang, linux-kernel

I missed that this was already reported: Andrew posted the same two-line
change on 2026-09-10

  https://lore.kernel.org/linux-nvme/20260910154849.66095-1-astellman@stellman-greene.com/

and Sagi asked in that thread whether it had been tested.  That patch
reasons the same way, and it also cites the spec wording for a valid ANA
group identifier (non-zero and <= ANAGRPMAX), which mine should have
quoted as well.

What I can add is the evidence that was asked for.  On
7.3.0-next-20261027, with two kernels built from the same tree that
differ only in these two lines and no instrumentation, the nvme-loop
transport shows: a group created as ports/1/ana_groups/128 is
absent from the ANA log page before the patch (ngrps=1) and present
after it (ngrps=2); 128 written to a namespace ana_grpid reads back as 0
before the patch and as 128 after it, with the namespace accounted to
group 128; and removing the group again adds a phantom group 128 (state
inaccessible, 0 namespaces) to the ANA log page only in the unpatched
kernel.  The exact steps are in the --- section of my patch.

Andrew's patch is against a September tree and no longer applies, which
is why I sent a rebased version.  If you would rather take his original,
I have no objection.





^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] nvmet: clamp the last ANA group index against the real array size
  2026-11-06  7:03 ` Yuehong Wu
@ 2026-11-06 13:47   ` Andrew Stellman
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Stellman @ 2026-11-06 13:47 UTC (permalink / raw)
  To: Yuehong Wu
  Cc: linux-nvme, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni,
	Keith Busch, Maurizio Lombardi, Mohamed Khalfella, Nitesh Shetty,
	Hannes Reinecke, Randy Dunlap, Aurelien Aptel, Nilay Shroff,
	Jesse Taube, Rongzheng Wang, Kees Cook, Rosen Penev, Guixin Liu,
	Qing Zhang, Eric Biggers, Julian Sun, Yu Zhang, linux-kernel

Thanks, Yuehong. I’ve sent a v2 of my original that answers Sagi’s
question and links to your testing:
https://lore.kernel.org/r/20261106134313.34829-1-astellman@stellman-greene.com



On Fri, Nov 6, 2026 at 2:04 AM Yuehong Wu
<wuyuehong@cmss.chinamobile.com> wrote:
>
> I missed that this was already reported: Andrew posted the same two-line
> change on 2026-09-10
>
>   https://lore.kernel.org/linux-nvme/20260910154849.66095-1-astellman@stellman-greene.com/
>
> and Sagi asked in that thread whether it had been tested.  That patch
> reasons the same way, and it also cites the spec wording for a valid ANA
> group identifier (non-zero and <= ANAGRPMAX), which mine should have
> quoted as well.
>
> What I can add is the evidence that was asked for.  On
> 7.3.0-next-20261027, with two kernels built from the same tree that
> differ only in these two lines and no instrumentation, the nvme-loop
> transport shows: a group created as ports/1/ana_groups/128 is
> absent from the ANA log page before the patch (ngrps=1) and present
> after it (ngrps=2); 128 written to a namespace ana_grpid reads back as 0
> before the patch and as 128 after it, with the namespace accounted to
> group 128; and removing the group again adds a phantom group 128 (state
> inaccessible, 0 namespaces) to the ANA log page only in the unpatched
> kernel.  The exact steps are in the --- section of my patch.
>
> Andrew's patch is against a September tree and no longer applies, which
> is why I sent a rebased version.  If you would rather take his original,
> I have no objection.
>
>
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] nvmet: clamp the last ANA group index against the real array size
  2026-11-06  6:40 [PATCH] nvmet: clamp the last ANA group index against the real array size Yuehong Wu
  2026-11-06  7:03 ` Yuehong Wu
@ 2026-11-12 21:21 ` Christoph Hellwig
  1 sibling, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2026-11-12 21:21 UTC (permalink / raw)
  To: Yuehong Wu
  Cc: linux-nvme, Sagi Grimberg, Chaitanya Kulkarni, Keith Busch,
	Maurizio Lombardi, Mohamed Khalfella, Nitesh Shetty,
	Hannes Reinecke, Randy Dunlap, Aurelien Aptel, Nilay Shroff,
	Jesse Taube, Rongzheng Wang, Kees Cook, Rosen Penev, Guixin Liu,
	Qing Zhang, Eric Biggers, Julian Sun, Yu Zhang, linux-kernel

Looks good:

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



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-12 21:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-06  6:40 [PATCH] nvmet: clamp the last ANA group index against the real array size Yuehong Wu
2026-11-06  7:03 ` Yuehong Wu
2026-11-06 13:47   ` Andrew Stellman
2026-11-12 21:21 ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.