All of lore.kernel.org
 help / color / mirror / Atom feed
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?


      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 [PATCH] nvmet: accept ANA group ID NVMET_MAX_ANAGRPS in configfs 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 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.