Re: [PATCH] arch_numa: Avoid false positive fortify warning in setup_node_to_cpumask_map()
Andrew Morton <[email protected]>
| Newsgroups | org.kernel.vger.stable,dev.linux.lists.llvm,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On Tue, 11 Aug 2026 22:22:42 -0700 Nathan Chancellor <[email protected]> wrote: > When building ARCH=riscv using clang with CONFIG_FORTIFY_SOURCE and > CONFIG_UBSAN_BOUNDS enabled, CONFIG_NR_CPUS > 64, and the default value > of 2 for CONFIG_NODES_SHIFT, there is a compiletime warning from the > fortify routines. (cc Kees) > In file included from mm/arch_numa.c:11: > In file included from include/linux/acpi.h:14: > In file included from include/linux/resource_ext.h:11: > In file included from include/linux/slab.h:17: > In file included from include/linux/gfp.h:7: > In file included from include/linux/mmzone.h:8: > In file included from include/linux/spinlock.h:60: > In file included from include/linux/interrupt_rc.h:17: > In file included from include/linux/smp.h:13: > In file included from include/linux/cpumask.h:11: > In file included from include/linux/bitmap.h:13: > In file included from include/linux/string.h:383: > include/linux/fortify-string.h:430:4: warning: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribue-warning] > 430 | __write_overflow_field(p_size_field, size); > | ^ > include/linux/fortify-string.h:430:4: note: called by function 'fortify_memset_chk(unsigned long, unsigned long, unsigned long)' > include/linux/bitmap.h:248:3: note: inlined by function 'setup_node_to_cpumask_map' > 248 | memset(dst, 0, len); > | ^ > include/linux/fortify-string.h:462:25: note: expanded from macro 'memset' > 462 | #define memset(p, c, s) __fortify_memset_chk(p, c, s, \ > | ^ > include/linux/fortify-string.h:453:2: note: expanded from macro '__fortify_memset_chk' > 453 | fortify_memset_chk(__fortify_size, p_size, p_size_field), \ > | ^ > include/linux/fortify-string.h:430:4: note: use '-gline-directives-only' (implied by '-g1') or higher for more accurate inlining chain locations > 430 | __write_overflow_field(p_size_field, size); > | ^ > 1 warning generated. > > In this configuration, MAX_NUMNODES is 4. clang unrolls the for loop in > setup_node_to_cpumask_map() past this, which triggers the fortify check > when accessing node_to_cpumask_map on the theoretical fifth loop > iteration because it would be an out of bounds write. > > Make it clear to clang that nr_node_ids is bounded by MAX_NUMNODES due > to the logic in setup_nr_node_ids() by early returning in > setup_node_to_cpumask_map() should that condition be violated. > > Cc: [email protected] # all applicable > Closes: https://github.com/ClangBuiltLinux/linux/issues/2174 > Signed-off-by: Nathan Chancellor <[email protected]> > --- > This is based on mm-unstable due to the move of arch_numa.c from mm/ to > drivers/base/ living there. I have CC'd stable because this warning > appears in my testing back to at least 6.1 but I see no reason why it > should not apply to all trees. No fixes tag since this is a layered > problem that just happens to appear under certain conditions. > > Another alternative would be using the __assume macro to say something > like > > __assume(nr_node_ids <= MAX_NUMNODES); > > but that seems a little more fragile. > --- > mm/arch_numa.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/mm/arch_numa.c b/mm/arch_numa.c > index 442ea239bba7..bd7772b05bcf 100644 > --- a/mm/arch_numa.c > +++ b/mm/arch_numa.c > @@ -105,6 +105,18 @@ static void __init setup_node_to_cpumask_map(void) > if (nr_node_ids == MAX_NUMNODES) > setup_nr_node_ids(); > > + /* > + * This check should never be true but it makes it clear to compilers > + * that node_to_cpumask_map is bound by nr_node_ids, avoiding false > + * positive fortify warnings when accessing node_to_cpumask_map in the > + * for loop below. > + */ > + if (unlikely(nr_node_ids > MAX_NUMNODES)) { > + pr_err("nr_node_ids (%u) is larger than MAX_NUMNODES (%d)", Sashiko wants a \n there. > + nr_node_ids, MAX_NUMNODES); Lord only knows why nr_node_ids is unsigned but MAX_NUMNODES is signed. lgtm otherwise. > + return; > + } > +