From: Sagi Grimberg <sagi@grimberg.me>
To: Andrew Stellman <astellman@stellman-greene.com>,
Christoph Hellwig <hch@lst.de>,
Chaitanya Kulkarni <kch@nvidia.com>
Cc: linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] nvmet: accept ANA group ID NVMET_MAX_ANAGRPS in configfs
Date: Sat, 12 Sep 2026 00:41:29 +0300 [thread overview]
Message-ID: <9cb2f10e-bea0-44b2-a7dc-e2eaeb5ca96e@grimberg.me> (raw)
In-Reply-To: <3a06fc29-4364-4573-b12c-39b42d95a4b3@grimberg.me>
On 12/09/2026 0:40, Sagi Grimberg wrote:
>
>
> On 10/09/2026 18:48, Andrew Stellman wrote:
>> nvmet_ns_ana_grpid_store() and nvmet_ana_groups_make_group() accept an
>> ANA group ID in the closed range 1..NVMET_MAX_ANAGRPS, but then pass it
>> through array_index_nospec() with NVMET_MAX_ANAGRPS as the size. That
>> helper treats the size as a half-open bound, so the maximum valid ID,
>> 128, is rewritten to 0.
>>
>> nvmet_ana_group_enabled[] is NVMET_MAX_ANAGRPS + 1 entries wide, so
>> index 128 is a valid slot, and the controller advertises ANAGRPMAX =
>> NVMET_MAX_ANAGRPS. NVMe Base Specification 2.4, section 8.1.1
>> (Asymmetric Namespace Access Reporting), "ANA Groups", defines a valid
>> ANA Group Identifier as "a non-zero value that is less than or equal to
>> ANAGRPMAX".
>>
>> Two visible effects. Writing 128 to a namespace's ana_grpid succeeds
>> but the namespace lands in the reserved group 0, which Identify
>> Namespace then reports. Creating and removing ports/N/ana_groups/128
>> increments slot 0 on create but decrements slot 128 on release, leaving
>> nvmet_ana_group_enabled[128] at 0xffffffff, so every subsequent ANA log
>> page reports a group 128 with no namespaces in the Inaccessible state.
>>
>> Use NVMET_MAX_ANAGRPS + 1 as the array_index_nospec() bound at both
>> sites, matching the array's actual size. A namespace assigned to group
>> 128 now follows the same path as any other group: its ANA state is
>> Inaccessible until ports/N/ana_groups/128 is created and configured,
>> where before it sat in group 0 with an uninitialized state and I/O was
>> allowed.
>>
>> Tested on 7.3.0-rc1-qpb-cc-base+ (unpatched) and
>> 7.3.0-rc1-qpb-cc-anagrpid+
>> (patched) in an arm64 QEMU guest with nvmet over NVMe/TCP to
>> 127.0.0.1. Before: ana_grpid written as 128 reads back 0, and the ANA
>> log after mkdir+rmdir of ana_groups/128 lists group 128 with nnsids 0,
>> state inaccessible. After: ana_grpid reads back 128 and the ANA log
>> lists only group 1.
>>
>> The issue was found by Claude Opus 5 running Quality Playbook, an
>> LLM-driven code review tool:
>> https://github.com/andrewstellman/quality-playbook
>>
>> Fixes: 20dc66f2d76b ("nvme: prevent potential spectre v1 gadget")
>> Assisted-by: Claude:claude-opus-5 [Quality Playbook]
>> Signed-off-by: Andrew Stellman <astellman@stellman-greene.com>
>> ---
>> 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 413ee2d16d29..0d4c69c4697a 100644
>> --- a/drivers/nvme/target/configfs.c
>> +++ b/drivers/nvme/target/configfs.c
>> @@ -698,7 +698,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);
>
> There is a condition above:
--
if (grpid <= 1 || grpid > NVMET_MAX_ANAGRPS)
goto out;
--
I assume that this patch was not tested?
prev parent reply other threads:[~2026-09-11 21:41 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-10 15:48 Andrew Stellman
2026-09-11 21:40 ` Sagi Grimberg
2026-09-11 21:41 ` Sagi Grimberg [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9cb2f10e-bea0-44b2-a7dc-e2eaeb5ca96e@grimberg.me \
--to=sagi@grimberg.me \
--cc=astellman@stellman-greene.com \
--cc=hch@lst.de \
--cc=kch@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®