Hi Kees,
I'm running into compilation warnings/errors due to fortify-string.h.
Environment:
- Commit 0fc810ae3ae110f9e2fcccce80fc8c8d62f97907 (current mainline master)
- gcc (GCC) 14.2.1 20240910
- Relevant config (from an Arch Linux distro config):
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_NR_CPUS=320
CONFIG_NR_CPUS_RANGE_BEGIN=2
CONFIG_NR_CPUS_RANGE_END=512
CONFIG_NR_CPUS_RANGE_DEFAULT=64
CONFIG_PADATA=y
Warning:
CC kernel/padata.o
In file included from ./include/linux/string.h:390,
from ./include/linux/bitmap.h:13,
from ./include/linux/cpumask.h:12,
from ./arch/x86/include/asm/paravirt.h:21,
from ./arch/x86/include/asm/irqflags.h:80,
from ./include/linux/irqflags.h:18,
from ./include/linux/spinlock.h:59,
from ./include/linux/swait.h:7,
from ./include/linux/completion.h:12,
from kernel/padata.c:14:
In function ‘bitmap_copy’,
inlined from ‘cpumask_copy’ at ./include/linux/cpumask.h:839:2,
inlined from ‘__padata_set_cpumasks’ at kernel/padata.c:730:2:
./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ reading between 41 and 536870904 bytes from a region of size 40 [-Werror=stringop-overread]
114 | #define __underlying_memcpy __builtin_memcpy
| ^
./include/linux/fortify-string.h:633:9: note: in expansion of macro ‘__underlying_memcpy’
633 | __underlying_##op(p, q, __fortify_size); \
| ^~~~~~~~~~~~~
./include/linux/fortify-string.h:678:26: note: in expansion of macro ‘__fortify_memcpy_chk’
678 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
| ^~~~~~~~~~~~~~~~~~~~
./include/linux/bitmap.h:259:17: note: in expansion of macro ‘memcpy’
259 | memcpy(dst, src, len);
| ^~~~~~
kernel/padata.c: In function ‘__padata_set_cpumasks’:
kernel/padata.c:713:48: note: source object ‘pcpumask’ of size [0, 40]
713 | cpumask_var_t pcpumask,
| ~~~~~~~~~~~~~~^~~~~~~~
In function ‘bitmap_copy’,
inlined from ‘cpumask_copy’ at ./include/linux/cpumask.h:839:2,
inlined from ‘__padata_set_cpumasks’ at kernel/padata.c:730:2:
./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ reading between 41 and 536870904 bytes from a region of size 40 [-Werror=stringop-overread]
114 | #define __underlying_memcpy __builtin_memcpy
| ^
./include/linux/fortify-string.h:633:9: note: in expansion of macro ‘__underlying_memcpy’
633 | __underlying_##op(p, q, __fortify_size); \
| ^~~~~~~~~~~~~
./include/linux/fortify-string.h:678:26: note: in expansion of macro ‘__fortify_memcpy_chk’
678 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
| ^~~~~~~~~~~~~~~~~~~~
./include/linux/bitmap.h:259:17: note: in expansion of macro ‘memcpy’
259 | memcpy(dst, src, len);
| ^~~~~~
kernel/padata.c: In function ‘__padata_set_cpumasks’:
kernel/padata.c:713:48: note: source object ‘pcpumask’ of size [0, 40]
713 | cpumask_var_t pcpumask,
| ~~~~~~~~~~~~~~^~~~~~~~
cc1: all warnings being treated as errors
Code:
712 static int __padata_set_cpumasks(struct padata_instance *pinst,
713 cpumask_var_t pcpumask,
714 cpumask_var_t cbcpumask)
715 {
716 int valid;
717 int err;
718
719 valid = padata_validate_cpumask(pinst, pcpumask);
720 if (!valid) {
721 __padata_stop(pinst);
722 goto out_replace;
723 }
724
725 valid = padata_validate_cpumask(pinst, cbcpumask);
726 if (!valid)
727 __padata_stop(pinst);
728
729 out_replace:
730 cpumask_copy(pinst->cpumask.pcpu, pcpumask);
731 cpumask_copy(pinst->cpumask.cbcpu, cbcpumask);
732
733 err = padata_setup_cpumasks(pinst) ?: padata_replace(pinst);
734
735 if (valid)
736 __padata_start(pinst);
737
738 return err;
739 }
The weird thing is, that only the cpumask_copy() in line 730 triggers
this warning. The one in line 731 doesn't. Also this is the only
instance of the warning I see in the whole build.
The warning goes away with the following change, but that introduces
runtime overhead and feels wrong. Also it doesn't explain why this
specific call is different from all others.
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 9278a50d514f..ded9d1bcef03 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -836,7 +838,7 @@ void cpumask_shift_left(struct cpumask *dstp, const struct cpumask *srcp, int n)
static __always_inline
void cpumask_copy(struct cpumask *dstp, const struct cpumask *srcp)
{
- bitmap_copy(cpumask_bits(dstp), cpumask_bits(srcp), large_cpumask_bits);
+ bitmap_copy(cpumask_bits(dstp), cpumask_bits(srcp), MIN(NR_CPUS, large_cpumask_bits));
}
/**
Any ideas?
Thomas
On Thu, Oct 31, 2024 at 08:40:33PM +0000, Thomas Weißschuh wrote:
> Hi Kees,
>
> I'm running into compilation warnings/errors due to fortify-string.h.
>
> Environment:
> - Commit 0fc810ae3ae110f9e2fcccce80fc8c8d62f97907 (current mainline master)
> - gcc (GCC) 14.2.1 20240910
> - Relevant config (from an Arch Linux distro config):
> CONFIG_64BIT=y
> CONFIG_X86_64=y
> CONFIG_NR_CPUS=320
> CONFIG_NR_CPUS_RANGE_BEGIN=2
> CONFIG_NR_CPUS_RANGE_END=512
> CONFIG_NR_CPUS_RANGE_DEFAULT=64
> CONFIG_PADATA=y
>
> Warning:
>
> CC kernel/padata.o
> In file included from ./include/linux/string.h:390,
> from ./include/linux/bitmap.h:13,
> from ./include/linux/cpumask.h:12,
> from ./arch/x86/include/asm/paravirt.h:21,
> from ./arch/x86/include/asm/irqflags.h:80,
> from ./include/linux/irqflags.h:18,
> from ./include/linux/spinlock.h:59,
> from ./include/linux/swait.h:7,
> from ./include/linux/completion.h:12,
> from kernel/padata.c:14:
> In function ‘bitmap_copy’,
> inlined from ‘cpumask_copy’ at ./include/linux/cpumask.h:839:2,
> inlined from ‘__padata_set_cpumasks’ at kernel/padata.c:730:2:
> ./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ reading between 41 and 536870904 bytes from a region of size 40 [-Werror=stringop-overread]
> 114 | #define __underlying_memcpy __builtin_memcpy
> | ^
> ./include/linux/fortify-string.h:633:9: note: in expansion of macro ‘__underlying_memcpy’
> 633 | __underlying_##op(p, q, __fortify_size); \
> | ^~~~~~~~~~~~~
> ./include/linux/fortify-string.h:678:26: note: in expansion of macro ‘__fortify_memcpy_chk’
> 678 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
> | ^~~~~~~~~~~~~~~~~~~~
> ./include/linux/bitmap.h:259:17: note: in expansion of macro ‘memcpy’
> 259 | memcpy(dst, src, len);
> | ^~~~~~
> kernel/padata.c: In function ‘__padata_set_cpumasks’:
> kernel/padata.c:713:48: note: source object ‘pcpumask’ of size [0, 40]
> 713 | cpumask_var_t pcpumask,
> | ~~~~~~~~~~~~~~^~~~~~~~
> In function ‘bitmap_copy’,
> inlined from ‘cpumask_copy’ at ./include/linux/cpumask.h:839:2,
> inlined from ‘__padata_set_cpumasks’ at kernel/padata.c:730:2:
> ./include/linux/fortify-string.h:114:33: error: ‘__builtin_memcpy’ reading between 41 and 536870904 bytes from a region of size 40 [-Werror=stringop-overread]
> 114 | #define __underlying_memcpy __builtin_memcpy
> | ^
> ./include/linux/fortify-string.h:633:9: note: in expansion of macro ‘__underlying_memcpy’
> 633 | __underlying_##op(p, q, __fortify_size); \
> | ^~~~~~~~~~~~~
> ./include/linux/fortify-string.h:678:26: note: in expansion of macro ‘__fortify_memcpy_chk’
> 678 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
> | ^~~~~~~~~~~~~~~~~~~~
> ./include/linux/bitmap.h:259:17: note: in expansion of macro ‘memcpy’
> 259 | memcpy(dst, src, len);
> | ^~~~~~
> kernel/padata.c: In function ‘__padata_set_cpumasks’:
> kernel/padata.c:713:48: note: source object ‘pcpumask’ of size [0, 40]
> 713 | cpumask_var_t pcpumask,
> | ~~~~~~~~~~~~~~^~~~~~~~
> cc1: all warnings being treated as errors
>
> Code:
>
> 712 static int __padata_set_cpumasks(struct padata_instance *pinst,
> 713 cpumask_var_t pcpumask,
> 714 cpumask_var_t cbcpumask)
> 715 {
> 716 int valid;
> 717 int err;
> 718
> 719 valid = padata_validate_cpumask(pinst, pcpumask);
> 720 if (!valid) {
> 721 __padata_stop(pinst);
> 722 goto out_replace;
> 723 }
> 724
> 725 valid = padata_validate_cpumask(pinst, cbcpumask);
> 726 if (!valid)
> 727 __padata_stop(pinst);
> 728
> 729 out_replace:
> 730 cpumask_copy(pinst->cpumask.pcpu, pcpumask);
> 731 cpumask_copy(pinst->cpumask.cbcpu, cbcpumask);
> 732
> 733 err = padata_setup_cpumasks(pinst) ?: padata_replace(pinst);
> 734
> 735 if (valid)
> 736 __padata_start(pinst);
> 737
> 738 return err;
> 739 }
>
>
> The weird thing is, that only the cpumask_copy() in line 730 triggers
> this warning. The one in line 731 doesn't. Also this is the only
> instance of the warning I see in the whole build.
>
> The warning goes away with the following change, but that introduces
> runtime overhead and feels wrong. Also it doesn't explain why this
> specific call is different from all others.
>
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index 9278a50d514f..ded9d1bcef03 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -836,7 +838,7 @@ void cpumask_shift_left(struct cpumask *dstp, const struct cpumask *srcp, int n)
> static __always_inline
> void cpumask_copy(struct cpumask *dstp, const struct cpumask *srcp)
> {
> - bitmap_copy(cpumask_bits(dstp), cpumask_bits(srcp), large_cpumask_bits);
> + bitmap_copy(cpumask_bits(dstp), cpumask_bits(srcp), MIN(NR_CPUS, large_cpumask_bits));
> }
>
> /**
>
> Any ideas?
My notes on looking at this:
#define DECLARE_BITMAP(name,bits) \
unsigned long name[BITS_TO_LONGS(bits)]
...
typedef struct cpumask { DECLARE_BITMAP(bits, NR_CPUS); } cpumask_t;
#ifdef CONFIG_CPUMASK_OFFSTACK
typedef struct cpumask *cpumask_var_t;
#else
typedef struct cpumask cpumask_var_t[1];
#endif /* CONFIG_CPUMASK_OFFSTACK */
...
#define cpumask_bits(maskp) ((maskp)->bits)
...
int padata_set_cpumask(struct padata_instance *pinst, int cpumask_type,
cpumask_var_t cpumask)
...
struct cpumask *serial_mask, *parallel_mask;
...
parallel_mask = cpumask;
...or...
parallel_mask = pinst->cpumask.pcpu;
...
err = __padata_set_cpumasks(pinst, parallel_mask, serial_mask);
...
static int __padata_set_cpumasks(struct padata_instance *pinst,
cpumask_var_t pcpumask,
cpumask_var_t cbcpumask)
...
cpumask_copy(pinst->cpumask.cbcpu, cbcpumask);
...
static __always_inline
void cpumask_copy(struct cpumask *dstp, const struct cpumask *srcp)
{
bitmap_copy(cpumask_bits(dstp), cpumask_bits(srcp),
large_cpumask_bits);
}
...
extern unsigned int nr_cpu_ids;
...
#if NR_CPUS <= BITS_PER_LONG // false: 320 <= 64
#define small_cpumask_bits ((unsigned int)NR_CPUS)
#define large_cpumask_bits ((unsigned int)NR_CPUS)
#elif NR_CPUS <= 4*BITS_PER_LONG // false: 320 <= 256
#define small_cpumask_bits nr_cpu_ids
#define large_cpumask_bits ((unsigned int)NR_CPUS)
#else
#define small_cpumask_bits nr_cpu_ids
#define large_cpumask_bits nr_cpu_ids // not a compile-time constant
#endif
...
static __always_inline
void bitmap_copy(unsigned long *dst, const unsigned long *src, unsigned
int nbits)
{
unsigned int len = bitmap_size(nbits);
if (small_const_nbits(nbits))
*dst = *src;
else
memcpy(dst, src, len);
}
So the result is:
memcpy(pcpumask->bits, cbcpumask->bits, nr_cpu_ids)
And the error is that the compiler has determined that nc_cpu_ids got
limited to possibly have a value between 41 and 536870904. This is an
odd limit, though: 0x1FFFFFF8, but does feel constructable -- it's close
to some magic numbers.
There are some new diagnostics being added to GCC that might help track
this down[1]. I'm waiting for a v4 before I take it for a spin.
-Kees
[1] https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666870.html
--
Kees Cook
© 2016 - 2024 Red Hat, Inc.