* [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr [not found] <20251121193957.1655580-1-morbo@google.com> @ 2025-11-21 19:39 ` Bill Wendling 2025-11-22 0:30 ` Kees Cook 2025-11-25 12:08 ` Mike Rapoport 0 siblings, 2 replies; 6+ messages in thread From: Bill Wendling @ 2025-11-21 19:39 UTC (permalink / raw) To: linux-kernel Cc: Bill Wendling, Kees Cook, Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Justin Stitt, Miguel Ojeda, Peter Zijlstra, Andrew Morton, Heiko Carstens, Marc Herbert, Uros Bizjak, Tejun Heo, Jeff Xu, Michal Koutný, Shakeel Butt, Thomas Weißschuh, John Stultz, Christian Brauner, Randy Dunlap, Brian Gerst, Masahiro Yamada, Mike Rapoport, linux-mm, linux-hardening, llvm 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr 2025-11-21 19:39 ` [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr Bill Wendling @ 2025-11-22 0:30 ` Kees Cook 2025-11-22 22:16 ` Andrew Morton 2025-11-25 12:08 ` Mike Rapoport 1 sibling, 1 reply; 6+ messages in thread From: Kees Cook @ 2025-11-22 0:30 UTC (permalink / raw) To: Bill Wendling Cc: linux-kernel, Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Justin Stitt, Miguel Ojeda, Peter Zijlstra, Andrew Morton, Heiko Carstens, Marc Herbert, Uros Bizjak, Tejun Heo, Jeff Xu, Michal Koutný, Shakeel Butt, Thomas Weißschuh, John Stultz, Christian Brauner, Randy Dunlap, Brian Gerst, Masahiro Yamada, Mike Rapoport, linux-mm, linux-hardening, llvm 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr 2025-11-22 0:30 ` Kees Cook @ 2025-11-22 22:16 ` Andrew Morton 2025-11-24 19:19 ` Kees Cook 0 siblings, 1 reply; 6+ messages in thread From: Andrew Morton @ 2025-11-22 22:16 UTC (permalink / raw) To: Kees Cook Cc: Bill Wendling, linux-kernel, Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Justin Stitt, Miguel Ojeda, Peter Zijlstra, Heiko Carstens, Marc Herbert, Uros Bizjak, Tejun Heo, Jeff Xu, Michal Koutný, Shakeel Butt, Thomas Weißschuh, John Stultz, Christian Brauner, Randy Dunlap, Brian Gerst, Masahiro Yamada, Mike Rapoport, linux-mm, linux-hardening, llvm 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? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr 2025-11-22 22:16 ` Andrew Morton @ 2025-11-24 19:19 ` Kees Cook 2025-11-24 20:15 ` Bill Wendling 0 siblings, 1 reply; 6+ messages in thread From: Kees Cook @ 2025-11-24 19:19 UTC (permalink / raw) To: Andrew Morton Cc: Bill Wendling, linux-kernel, Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Justin Stitt, Miguel Ojeda, Peter Zijlstra, Heiko Carstens, Marc Herbert, Uros Bizjak, Tejun Heo, Jeff Xu, Michal Koutný, Shakeel Butt, Thomas Weißschuh, John Stultz, Christian Brauner, Randy Dunlap, Brian Gerst, Masahiro Yamada, Mike Rapoport, linux-mm, linux-hardening, llvm 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr 2025-11-24 19:19 ` Kees Cook @ 2025-11-24 20:15 ` Bill Wendling 0 siblings, 0 replies; 6+ messages in thread From: Bill Wendling @ 2025-11-24 20:15 UTC (permalink / raw) To: Kees Cook Cc: Andrew Morton, linux-kernel, Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Justin Stitt, Miguel Ojeda, Peter Zijlstra, Heiko Carstens, Marc Herbert, Uros Bizjak, Tejun Heo, Jeff Xu, Michal Koutný, Shakeel Butt, Thomas Weißschuh, John Stultz, Christian Brauner, Randy Dunlap, Brian Gerst, Masahiro Yamada, Mike Rapoport, linux-mm, linux-hardening, llvm 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr 2025-11-21 19:39 ` [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr Bill Wendling 2025-11-22 0:30 ` Kees Cook @ 2025-11-25 12:08 ` Mike Rapoport 1 sibling, 0 replies; 6+ messages in thread From: Mike Rapoport @ 2025-11-25 12:08 UTC (permalink / raw) To: Bill Wendling Cc: linux-kernel, Kees Cook, Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Justin Stitt, Miguel Ojeda, Peter Zijlstra, Andrew Morton, Heiko Carstens, Marc Herbert, Uros Bizjak, Tejun Heo, Jeff Xu, Michal Koutný, Shakeel Butt, Thomas Weißschuh, John Stultz, Christian Brauner, Randy Dunlap, Brian Gerst, Masahiro Yamada, linux-mm, linux-hardening, llvm 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-11-25 12:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20251121193957.1655580-1-morbo@google.com>
2025-11-21 19:39 ` [PATCH 2/2] memblock: annotate struct memblock_type with __counted_by_ptr Bill Wendling
2025-11-22 0:30 ` Kees Cook
2025-11-22 22:16 ` Andrew Morton
2025-11-24 19:19 ` Kees Cook
2025-11-24 20:15 ` Bill Wendling
2025-11-25 12:08 ` Mike Rapoport
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox