[v1] blkdev: Annotate struct request_queue with __counted_by_ptr

[PATCH] blkdev: Annotate struct request_queue with __counted_by_ptr

Posted by Bill Wendling 1 month, 2 weeks ago

The queue_hw_ctx field in struct request_queue is an array of pointers to
struct blk_mq_hw_ctx. The number of elements in this array is tracked by
the nr_hw_queues field.

The array is allocated in __blk_mq_realloc_hw_ctxs() using kcalloc_node()
with set->nr_hw_queues elements. q->nr_hw_queues is subsequently updated
to set->nr_hw_queues.

When growing the array, the new array is assigned to queue_hw_ctx before
nr_hw_queues is updated. This is safe because nr_hw_queues (the old
smaller count) is used for bounds checking, which is within the new
larger allocation.

When shrinking the array, nr_hw_queues is updated to the smaller value,
while queue_hw_ctx retains the larger allocation. This is also safe as
the count is within the allocation bounds.

Annotating queue_hw_ctx with __counted_by_ptr(nr_hw_queues) allows the
compiler (with kSAN) to verify that accesses to queue_hw_ctx are within
the valid range defined by nr_hw_queues.

This patch was generated by Gemini and reviewed by Bill Wendling.
Tested with bootup and running selftests.

Signed-off-by: Bill Wendling <morbo@google.com>
---
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Kees Cook <kees@kernel.org>
Cc: Gogul Balakrishnan <bgogul@google.com>
Cc: Arman Hasanzadeh <armanihm@google.com>
Cc: linux-block@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 include/linux/blkdev.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d463b9b5a0a5..540c2c6c9afd 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -502,7 +502,7 @@ struct request_queue {
 
 	/* hw dispatch queues */
 	unsigned int		nr_hw_queues;
-	struct blk_mq_hw_ctx * __rcu *queue_hw_ctx;
+	struct blk_mq_hw_ctx * __rcu *queue_hw_ctx __counted_by_ptr(nr_hw_queues);
 
 	struct percpu_ref	q_usage_counter;
 	struct lock_class_key	io_lock_cls_key;
-- 
2.43.0

Re: [PATCH] blkdev: Annotate struct request_queue with __counted_by_ptr

Posted by Daniel Wagner 1 month, 2 weeks ago

On Thu, Feb 19, 2026 at 12:49:35AM +0000, Bill Wendling wrote:
> The queue_hw_ctx field in struct request_queue is an array of pointers to
> struct blk_mq_hw_ctx. The number of elements in this array is tracked by
> the nr_hw_queues field.
> 
> The array is allocated in __blk_mq_realloc_hw_ctxs() using kcalloc_node()
> with set->nr_hw_queues elements. q->nr_hw_queues is subsequently updated
> to set->nr_hw_queues.
> 
> When growing the array, the new array is assigned to queue_hw_ctx before
> nr_hw_queues is updated. This is safe because nr_hw_queues (the old
> smaller count) is used for bounds checking, which is within the new
> larger allocation.
> 
> When shrinking the array, nr_hw_queues is updated to the smaller value,
> while queue_hw_ctx retains the larger allocation. This is also safe as
> the count is within the allocation bounds.
> 
> Annotating queue_hw_ctx with __counted_by_ptr(nr_hw_queues) allows the
> compiler (with kSAN) to verify that accesses to queue_hw_ctx are within
> the valid range defined by nr_hw_queues.
> 
> This patch was generated by Gemini and reviewed by Bill Wendling.
> Tested with bootup and running selftests.

There are some tests in blktests nvme/* which do change the number of
queues during runtime. Not sure if selftests have anything which is
related to this code path.

Re: [PATCH] blkdev: Annotate struct request_queue with __counted_by_ptr

Posted by Bill Wendling 1 month, 2 weeks ago

On Thu, Feb 19, 2026 at 4:14 AM Daniel Wagner <dwagner@suse.de> wrote:
>
> On Thu, Feb 19, 2026 at 12:49:35AM +0000, Bill Wendling wrote:
> > The queue_hw_ctx field in struct request_queue is an array of pointers to
> > struct blk_mq_hw_ctx. The number of elements in this array is tracked by
> > the nr_hw_queues field.
> >
> > The array is allocated in __blk_mq_realloc_hw_ctxs() using kcalloc_node()
> > with set->nr_hw_queues elements. q->nr_hw_queues is subsequently updated
> > to set->nr_hw_queues.
> >
> > When growing the array, the new array is assigned to queue_hw_ctx before
> > nr_hw_queues is updated. This is safe because nr_hw_queues (the old
> > smaller count) is used for bounds checking, which is within the new
> > larger allocation.
> >
> > When shrinking the array, nr_hw_queues is updated to the smaller value,
> > while queue_hw_ctx retains the larger allocation. This is also safe as
> > the count is within the allocation bounds.
> >
> > Annotating queue_hw_ctx with __counted_by_ptr(nr_hw_queues) allows the
> > compiler (with kSAN) to verify that accesses to queue_hw_ctx are within
> > the valid range defined by nr_hw_queues.
> >
> > This patch was generated by Gemini and reviewed by Bill Wendling.
> > Tested with bootup and running selftests.
>
> There are some tests in blktests nvme/* which do change the number of
> queues during runtime. Not sure if selftests have anything which is
> related to this code path.
>
It's normally fine to change the queue count just as long as either
(1) the pointer to the queues is also reallocated, or (2) the count
never goes over the original allocated value. (The second one is more
difficult to check, of course.) The bounds safety features that Apple
developed, and which are slowly being sent upstream, enforces (1).

I'll run the other tests, but I'm not familiar with the blktests (I
downloaded them but haven't looked too deeply into them). Do you have
some pointers on how to run them with a newly built kernel?

-bw

Re: [PATCH] blkdev: Annotate struct request_queue with __counted_by_ptr

Posted by Daniel Wagner 1 month, 2 weeks ago

On Thu, Feb 19, 2026 at 09:02:12AM -0800, Bill Wendling wrote:
> It's normally fine to change the queue count just as long as either
> (1) the pointer to the queues is also reallocated, or (2) the count
> never goes over the original allocated value. (The second one is more
> difficult to check, of course.) The bounds safety features that Apple
> developed, and which are slowly being sent upstream, enforces (1).
> 
> I'll run the other tests, but I'm not familiar with the blktests (I
> downloaded them but haven't looked too deeply into them). Do you have
> some pointers on how to run them with a newly built kernel?

blktests has feature detection and every tests figures out, if it can
run or not. It will print some info when it's not possible.

The block test suite has at least a couple related tests:

./check block/029 block/040

This suite wants null_blk driver.

And for nvme/048 the fabrics parts needs to be used:

  NVMET_TRTYPES="tcp" ./check nvme/048

and the nvme-fabrics part nvme-tcp and nvmet-tcp.

If you need the exact config option I can compile you a list.

Re: [PATCH] blkdev: Annotate struct request_queue with __counted_by_ptr

Posted by Bill Wendling 1 month, 1 week ago

On Thu, Feb 19, 2026 at 9:24 AM Daniel Wagner <dwagner@suse.de> wrote:
>
> On Thu, Feb 19, 2026 at 09:02:12AM -0800, Bill Wendling wrote:
> > It's normally fine to change the queue count just as long as either
> > (1) the pointer to the queues is also reallocated, or (2) the count
> > never goes over the original allocated value. (The second one is more
> > difficult to check, of course.) The bounds safety features that Apple
> > developed, and which are slowly being sent upstream, enforces (1).
> >
> > I'll run the other tests, but I'm not familiar with the blktests (I
> > downloaded them but haven't looked too deeply into them). Do you have
> > some pointers on how to run them with a newly built kernel?
>
> blktests has feature detection and every tests figures out, if it can
> run or not. It will print some info when it's not possible.
>
> The block test suite has at least a couple related tests:
>
> ./check block/029 block/040
>
> This suite wants null_blk driver.
>
> And for nvme/048 the fabrics parts needs to be used:
>
>   NVMET_TRTYPES="tcp" ./check nvme/048
>
> and the nvme-fabrics part nvme-tcp and nvmet-tcp.
>
> If you need the exact config option I can compile you a list.
>
Okay. I successfully ran the tests you indicated above and they passed.

-bw

Re: [PATCH] blkdev: Annotate struct request_queue with __counted_by_ptr

Posted by Daniel Wagner 1 month, 1 week ago

On Mon, Feb 23, 2026 at 02:47:47PM -0800, Bill Wendling wrote:
> Okay. I successfully ran the tests you indicated above and they passed.

Ah great. Thanks a lot!

Reviewed-by: Daniel Wagner <dwagner@suse.de>

Re: [PATCH] blkdev: Annotate struct request_queue with __counted_by_ptr

Posted by Bill Wendling 1 month, 1 week ago

On Tue, Feb 24, 2026 at 12:55 AM Daniel Wagner <dwagner@suse.de> wrote:
>
> On Mon, Feb 23, 2026 at 02:47:47PM -0800, Bill Wendling wrote:
> > Okay. I successfully ran the tests you indicated above and they passed.
>
> Ah great. Thanks a lot!
>
> Reviewed-by: Daniel Wagner <dwagner@suse.de>

Daniel,

Could you please change "Gemini" to "CodeMender" in the patch commit
message? If you like, I can send a v2.

-bw

Re: [PATCH] blkdev: Annotate struct request_queue with __counted_by_ptr

Posted by Jens Axboe 1 month, 1 week ago

On 2/25/26 11:26 AM, Bill Wendling wrote:
> On Tue, Feb 24, 2026 at 12:55 AM Daniel Wagner <dwagner@suse.de> wrote:
>>
>> On Mon, Feb 23, 2026 at 02:47:47PM -0800, Bill Wendling wrote:
>>> Okay. I successfully ran the tests you indicated above and they passed.
>>
>> Ah great. Thanks a lot!
>>
>> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> 
> Daniel,
> 
> Could you please change "Gemini" to "CodeMender" in the patch commit
> message? If you like, I can send a v2.

Not sure how you think this works, but since Daniel isn't the one
applying the patch, not sure how he'd change your patch for you.
Just send a v2 if you want some random name change done in it.

-- 
Jens Axboe

Re: [PATCH] blkdev: Annotate struct request_queue with __counted_by_ptr

Posted by Bill Wendling 1 month, 1 week ago

On Wed, Feb 25, 2026 at 10:54 AM Jens Axboe <axboe@kernel.dk> wrote:
> On 2/25/26 11:26 AM, Bill Wendling wrote:
> > On Tue, Feb 24, 2026 at 12:55 AM Daniel Wagner <dwagner@suse.de> wrote:
> >>
> >> On Mon, Feb 23, 2026 at 02:47:47PM -0800, Bill Wendling wrote:
> >>> Okay. I successfully ran the tests you indicated above and they passed.
> >>
> >> Ah great. Thanks a lot!
> >>
> >> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> >
> > Daniel,
> >
> > Could you please change "Gemini" to "CodeMender" in the patch commit
> > message? If you like, I can send a v2.
>
> Not sure how you think this works, but since Daniel isn't the one
> applying the patch, not sure how he'd change your patch for you.
> Just send a v2 if you want some random name change done in it.
>
I wasn't aware that Daniel wasn't the person applying the patch. I'll send a v2.

-bw

Re: [PATCH] blkdev: Annotate struct request_queue with __counted_by_ptr

Posted by Daniel Wagner 1 month, 1 week ago

On Wed, Feb 25, 2026 at 11:53:56AM -0700, Jens Axboe wrote:
> On 2/25/26 11:26 AM, Bill Wendling wrote:
> > Could you please change "Gemini" to "CodeMender" in the patch commit
> > message? If you like, I can send a v2.
> 
> Not sure how you think this works, but since Daniel isn't the one
> applying the patch, not sure how he'd change your patch for you.
> Just send a v2 if you want some random name change done in it.

I could do 'C-x M-x butterfly' :)

(https://xkcd.com/378/)

[PATCH v2] blkdev: Annotate struct request_queue with __counted_by_ptr

Posted by Bill Wendling 1 month, 1 week ago

The queue_hw_ctx field in struct request_queue is an array of pointers to
struct blk_mq_hw_ctx. The number of elements in this array is tracked by
the nr_hw_queues field.

The array is allocated in __blk_mq_realloc_hw_ctxs() using kcalloc_node()
with set->nr_hw_queues elements. q->nr_hw_queues is subsequently updated
to set->nr_hw_queues.

When growing the array, the new array is assigned to queue_hw_ctx before
nr_hw_queues is updated. This is safe because nr_hw_queues (the old
smaller count) is used for bounds checking, which is within the new
larger allocation.

When shrinking the array, nr_hw_queues is updated to the smaller value,
while queue_hw_ctx retains the larger allocation. This is also safe as
the count is within the allocation bounds.

Annotating queue_hw_ctx with __counted_by_ptr(nr_hw_queues) allows the
compiler (with kSAN) to verify that accesses to queue_hw_ctx are within
the valid range defined by nr_hw_queues.

This patch was generated by CodeMender and reviewed by Bill Wendling.
Tested by running blktests.

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Bill Wendling <morbo@google.com>
---
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Kees Cook <kees@kernel.org>
Cc: Gogul Balakrishnan <bgogul@google.com>
Cc: Arman Hasanzadeh <armanihm@google.com>
Cc: linux-block@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
v2 - Reworded the commit message to better indicate the AI agent involved and
     how it was tested.
---
 include/linux/blkdev.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d463b9b5a0a5..540c2c6c9afd 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -502,7 +502,7 @@ struct request_queue {
 
 	/* hw dispatch queues */
 	unsigned int		nr_hw_queues;
-	struct blk_mq_hw_ctx * __rcu *queue_hw_ctx;
+	struct blk_mq_hw_ctx * __rcu *queue_hw_ctx __counted_by_ptr(nr_hw_queues);
 
 	struct percpu_ref	q_usage_counter;
 	struct lock_class_key	io_lock_cls_key;
-- 
2.43.0

Re: [PATCH v2] blkdev: Annotate struct request_queue with __counted_by_ptr

Posted by Jens Axboe 1 month, 1 week ago

On Wed, 25 Feb 2026 20:51:05 +0000, Bill Wendling wrote:
> The queue_hw_ctx field in struct request_queue is an array of pointers to
> struct blk_mq_hw_ctx. The number of elements in this array is tracked by
> the nr_hw_queues field.
> 
> The array is allocated in __blk_mq_realloc_hw_ctxs() using kcalloc_node()
> with set->nr_hw_queues elements. q->nr_hw_queues is subsequently updated
> to set->nr_hw_queues.
> 
> [...]

Applied, thanks!

[1/1] blkdev: Annotate struct request_queue with __counted_by_ptr
      (no commit info)

Best regards,
-- 
Jens Axboe