[PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr

[PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr

[Bill Wendling]

Add the '__counted_by_ptr' attribute to the 'regions' field of 'struct
memblock_type'. The 'regions' field is an array of 'struct
memblock_region' and its size is tracked by the 'max' field, which
represents the total number of allocated regions.

This annotation allows the Kernel Address Sanitizer (KASAN) to detect
out-of-bounds accesses to the 'regions' array.

Cc: Kees Cook <kees@kernel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <nick.desaulniers+lkml@gmail.com>
Cc: Justin Stitt <justinstitt@google.com>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Marc Herbert <Marc.Herbert@linux.intel.com>
Cc: Uros Bizjak <ubizjak@gmail.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Jeff Xu <jeffxu@chromium.org>
Cc: "Michal Koutný" <mkoutny@suse.com>
Cc: Shakeel Butt <shakeel.butt@linux.dev>
Cc: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
Cc: John Stultz <jstultz@google.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-hardening@vger.kernel.org
Cc: llvm@lists.linux.dev
Signed-off-by: Bill Wendling <morbo@google.com>
---
 include/linux/memblock.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 221118b5a16e..ba7f7c999a45 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -91,7 +91,7 @@ struct memblock_type {
 	unsigned long cnt;
 	unsigned long max;
 	phys_addr_t total_size;
-	struct memblock_region *regions;
+	struct memblock_region *regions __counted_by_ptr(max);
 	char *name;
 };
 
-- 
2.52.0.rc2.455.g230fcf2819-goog

Re: [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr

[Kees Cook]

On Fri, Nov 21, 2025 at 07:39:44PM +0000, Bill Wendling wrote:
> Add the '__counted_by_ptr' attribute to the 'regions' field of 'struct
> memblock_type'. The 'regions' field is an array of 'struct
> memblock_region' and its size is tracked by the 'max' field, which
> represents the total number of allocated regions.

As part of any counted_by annotation patch, there needs to be discussion
in the commit log about how it's been shown to be a safe annotation
to make. e.g. in this case, if all allocations of "regions" have a
corresponding "max" assignment, etc. If just "git grep" can't find them
all, using something like Coccinelle or CodeQL to search for struct
memblock_type::regions assignments can work.

Here's what I used in the past for flexible arrays, but it was slow
due to Coccinelle needing --recursive-includes to see the structs,
but should be adaptable for counted_by on pointers:

@flex_match@
identifier STRUCT, COUNTED, ARRAY;
type COUNTED_TYPE, ARRAY_TYPE;
attribute name __counted_by;
@@

        struct STRUCT {
                ...
                COUNTED_TYPE COUNTED;
                ...
                ARRAY_TYPE ARRAY[] __counted_by(COUNTED);
        };

@missed_counted_assignment@
identifier flex_match.STRUCT;
struct STRUCT *P;
identifier flex_match.COUNTED;
identifier flex_match.ARRAY;
identifier ALLOC =~ ".*alloc.*";
@@

        P = ALLOC(...);
        ... when != P->COUNTED
*       P->ARRAY


> This annotation allows the Kernel Address Sanitizer (KASAN) to detect
> out-of-bounds accesses to the 'regions' array.

I think you mean UBSan here (and CONFIG_FORTIFY_SOURCE)?

> ---
>  include/linux/memblock.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 221118b5a16e..ba7f7c999a45 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -91,7 +91,7 @@ struct memblock_type {
>  	unsigned long cnt;
>  	unsigned long max;
>  	phys_addr_t total_size;
> -	struct memblock_region *regions;
> +	struct memblock_region *regions __counted_by_ptr(max);
>  	char *name;
>  };

For the handful of places I spot checked, yeah, it looks like a nice
annotation.

-Kees

-- 
Kees Cook

Re: [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr

[Andrew Morton]

On Fri, 21 Nov 2025 16:30:43 -0800 Kees Cook <kees@kernel.org> wrote:

> On Fri, Nov 21, 2025 at 07:39:44PM +0000, Bill Wendling wrote:
> > Add the '__counted_by_ptr' attribute to the 'regions' field of 'struct
> > memblock_type'. The 'regions' field is an array of 'struct
> > memblock_region' and its size is tracked by the 'max' field, which
> > represents the total number of allocated regions.
> 
> As part of any counted_by annotation patch, there needs to be discussion
> in the commit log about how it's been shown to be a safe annotation
> to make. e.g. in this case, if all allocations of "regions" have a
> corresponding "max" assignment, etc. If just "git grep" can't find them
> all, using something like Coccinelle or CodeQL to search for struct
> memblock_type::regions assignments can work.

How is anyone to know these things?  I can't find anything about this
in include/ or Documentation/ or in the relevant commits.

There should be a comment at the __counted_by() definition site, please.

And possibly write a Documentation/ file then change checkpatch to
direct people to that file if they add a counted_by?

Re: [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr

[Kees Cook]

On Sat, Nov 22, 2025 at 02:16:14PM -0800, Andrew Morton wrote:
> On Fri, 21 Nov 2025 16:30:43 -0800 Kees Cook <kees@kernel.org> wrote:
> 
> > On Fri, Nov 21, 2025 at 07:39:44PM +0000, Bill Wendling wrote:
> > > Add the '__counted_by_ptr' attribute to the 'regions' field of 'struct
> > > memblock_type'. The 'regions' field is an array of 'struct
> > > memblock_region' and its size is tracked by the 'max' field, which
> > > represents the total number of allocated regions.
> > 
> > As part of any counted_by annotation patch, there needs to be discussion
> > in the commit log about how it's been shown to be a safe annotation
> > to make. e.g. in this case, if all allocations of "regions" have a
> > corresponding "max" assignment, etc. If just "git grep" can't find them
> > all, using something like Coccinelle or CodeQL to search for struct
> > memblock_type::regions assignments can work.
> 
> How is anyone to know these things?  I can't find anything about this
> in include/ or Documentation/ or in the relevant commits.
> 
> There should be a comment at the __counted_by() definition site, please.
> 
> And possibly write a Documentation/ file then change checkpatch to
> direct people to that file if they add a counted_by?

This is a fair point, yes. The documentation and discussions around
counted_by are very big in my mind (and for Bill), so it was mostly a
consolidation/reminder and some extra detail on prior solutions, but
for anyone new to that annotation, we should have collected common
guidance. I will write something up.

-- 
Kees Cook

Re: [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr

[Bill Wendling]

On Mon, Nov 24, 2025 at 11:19 AM Kees Cook <kees@kernel.org> wrote:
>
> On Sat, Nov 22, 2025 at 02:16:14PM -0800, Andrew Morton wrote:
> > On Fri, 21 Nov 2025 16:30:43 -0800 Kees Cook <kees@kernel.org> wrote:
> >
> > > On Fri, Nov 21, 2025 at 07:39:44PM +0000, Bill Wendling wrote:
> > > > Add the '__counted_by_ptr' attribute to the 'regions' field of 'struct
> > > > memblock_type'. The 'regions' field is an array of 'struct
> > > > memblock_region' and its size is tracked by the 'max' field, which
> > > > represents the total number of allocated regions.
> > >
> > > As part of any counted_by annotation patch, there needs to be discussion
> > > in the commit log about how it's been shown to be a safe annotation
> > > to make. e.g. in this case, if all allocations of "regions" have a
> > > corresponding "max" assignment, etc. If just "git grep" can't find them
> > > all, using something like Coccinelle or CodeQL to search for struct
> > > memblock_type::regions assignments can work.
> >
> > How is anyone to know these things?  I can't find anything about this
> > in include/ or Documentation/ or in the relevant commits.
> >
> > There should be a comment at the __counted_by() definition site, please.
> >
> > And possibly write a Documentation/ file then change checkpatch to
> > direct people to that file if they add a counted_by?
>
> This is a fair point, yes. The documentation and discussions around
> counted_by are very big in my mind (and for Bill), so it was mostly a
> consolidation/reminder and some extra detail on prior solutions, but
> for anyone new to that annotation, we should have collected common
> guidance. I will write something up.
>
Good point. I'll add documentation for these attributes both in
Documentation/ and at the macro site. The frustrating thing is that
we're likely to have at least one other macro flavor (something like
"__counted_by_expr"), though that's the only foreseeable one. All of
these macros are wrappers around the same attribute because of
compiler skew.

-bw

Re: [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr

[Mike Rapoport]

On Fri, Nov 21, 2025 at 07:39:44PM +0000, Bill Wendling wrote:
> Add the '__counted_by_ptr' attribute to the 'regions' field of 'struct
> memblock_type'. The 'regions' field is an array of 'struct
> memblock_region' and its size is tracked by the 'max' field, which
> represents the total number of allocated regions.
> 
> This annotation allows the Kernel Address Sanitizer (KASAN) to detect
> out-of-bounds accesses to the 'regions' array.
> 
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
>  include/linux/memblock.h | 2 +-

Please also update tools/testing/memblock

>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 221118b5a16e..ba7f7c999a45 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -91,7 +91,7 @@ struct memblock_type {
>  	unsigned long cnt;
>  	unsigned long max;
>  	phys_addr_t total_size;
> -	struct memblock_region *regions;
> +	struct memblock_region *regions __counted_by_ptr(max);
>  	char *name;
>  };
>  
> -- 
> 2.52.0.rc2.455.g230fcf2819-goog
> 

-- 
Sincerely yours,
Mike.