[PATCH] nvmet: clamp the last ANA group index against the real array size

[PATCH] nvmet: clamp the last ANA group index against the real array size

[Yuehong Wu]

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

Re: [PATCH] nvmet: clamp the last ANA group index against the real array size

[Yuehong Wu]

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.

Re: [PATCH] nvmet: clamp the last ANA group index against the real array size

[Andrew Stellman]

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.
>
>
>

Re: [PATCH] nvmet: clamp the last ANA group index against the real array size

[Christoph Hellwig]

Looks good:

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

Re: [PATCH] nvmet: clamp the last ANA group index against the real array size

[Nilay Shroff]

On 11/6/26 12:10 PM, Yuehong Wu wrote:
> 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>

Looks good to me:

Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>

Re: [PATCH] nvmet: clamp the last ANA group index against the real array size

[Hannes Reinecke]

On 11/6/26 7:40 AM, Yuehong Wu wrote:
> 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);
>   
Reviewed-by: Hannes Reinecke <hare@kernel.org>

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