2022-06-17 15:38:48

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH] [RFC] Kbuild: change CONFIG_FRAME_WARN for 32-bit

From: Arnd Bergmann <[email protected]>

The introduction of CONFIG_GCC_PLUGIN_LATENT_ENTROPY raised the
warning limit for 32-bit architectures to a much higher value in
2016. Initially this had no effect for Arm machines as their kernels
tend to be cross-compiled, and the feature detection for the plugin did
not work with common cross compilers.

I could not find the original regression report that led to the warning
limit getting raised, but I have been unable to reproduce this with gcc-12
and linux-5.18 -- all frame sizes appear to be be below the normal 1024
byte limit on at least arm32 and i386.

However, the KASAN feature introduced later on does raise the frame size
of a number of functions above the warning limit, in almost all cases to
somewhere below 1280 bytes. The effect is that an arm allmodconfig build
produces no warnings because of the check for the latent entropy plugin,
but a normal KASAN based build with gcc does produce build warnings or
failures when CONFIG_WERROR is set.

The 1280 byte limit is an arbitrary choice here, as it disables almost
all warnings but leaves a few outliers that are probably better addressed
with code changes. There are a number of functions that are just below the
normal 1024 byte limit at the moment, and an extra 256 bytes for those
is within the expectation, but more than that is probably an indication
of a bad design or a real bug.

Cc: Uwe Kleine-König <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: [email protected]
Signed-off-by: Arnd Bergmann <[email protected]>
---
lib/Kconfig.debug | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 3da5f9acb966..8a3afd837e99 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -387,9 +387,9 @@ endif # DEBUG_INFO
config FRAME_WARN
int "Warn for stack frames larger than"
range 0 8192
- default 2048 if GCC_PLUGIN_LATENT_ENTROPY
default 2048 if PARISC
default 1536 if (!64BIT && XTENSA)
+ default 1280 if (!64BIT && KASAN_STACK)
default 1024 if !64BIT
default 2048 if 64BIT
help
--
2.29.2


2022-06-17 17:28:28

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH] [RFC] Kbuild: change CONFIG_FRAME_WARN for 32-bit

Hello Arnd,

your To: was empty, I wonder if that was on purpose?!

On Fri, Jun 17, 2022 at 05:08:53PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The introduction of CONFIG_GCC_PLUGIN_LATENT_ENTROPY raised the
> warning limit for 32-bit architectures to a much higher value in
> 2016. Initially this had no effect for Arm machines as their kernels
> tend to be cross-compiled, and the feature detection for the plugin did
> not work with common cross compilers.
>
> I could not find the original regression report that led to the warning
> limit getting raised, but I have been unable to reproduce this with gcc-12
> and linux-5.18 -- all frame sizes appear to be be below the normal 1024
> byte limit on at least arm32 and i386.
>
> However, the KASAN feature introduced later on does raise the frame size
> of a number of functions above the warning limit, in almost all cases to
> somewhere below 1280 bytes. The effect is that an arm allmodconfig build
> produces no warnings because of the check for the latent entropy plugin,
> but a normal KASAN based build with gcc does produce build warnings or
> failures when CONFIG_WERROR is set.
>
> The 1280 byte limit is an arbitrary choice here, as it disables almost
> all warnings but leaves a few outliers that are probably better addressed
> with code changes. There are a number of functions that are just below the
> normal 1024 byte limit at the moment, and an extra 256 bytes for those
> is within the expectation, but more than that is probably an indication
> of a bad design or a real bug.
>
> Cc: Uwe Kleine-K?nig <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: [email protected]
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> lib/Kconfig.debug | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 3da5f9acb966..8a3afd837e99 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -387,9 +387,9 @@ endif # DEBUG_INFO
> config FRAME_WARN
> int "Warn for stack frames larger than"
> range 0 8192
> - default 2048 if GCC_PLUGIN_LATENT_ENTROPY
> default 2048 if PARISC
> default 1536 if (!64BIT && XTENSA)
> + default 1280 if (!64BIT && KASAN_STACK)
> default 1024 if !64BIT
> default 2048 if 64BIT
> help

I wasn't aware this was configurable with a Kconfig symbol.

Thanks for addressing that issue, the reasoning looks good and right.
I just now noticed you explained your findings earlier in irc, only
found them because I digged my irc logs for my original report to you.

Acked-by: Uwe Kleine-K?nig <[email protected]>

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |


Attachments:
(No filename) (2.89 kB)
signature.asc (499.00 B)
Download all attachments

2022-06-17 21:54:44

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] [RFC] Kbuild: change CONFIG_FRAME_WARN for 32-bit

Hi Arnd,

On Fri, Jun 17, 2022 at 05:08:53PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The introduction of CONFIG_GCC_PLUGIN_LATENT_ENTROPY raised the
> warning limit for 32-bit architectures to a much higher value in
> 2016. Initially this had no effect for Arm machines as their kernels
> tend to be cross-compiled, and the feature detection for the plugin did
> not work with common cross compilers.
>
> I could not find the original regression report that led to the warning
> limit getting raised, but I have been unable to reproduce this with gcc-12
> and linux-5.18 -- all frame sizes appear to be be below the normal 1024
> byte limit on at least arm32 and i386.
>
> However, the KASAN feature introduced later on does raise the frame size
> of a number of functions above the warning limit, in almost all cases to
> somewhere below 1280 bytes. The effect is that an arm allmodconfig build
> produces no warnings because of the check for the latent entropy plugin,
> but a normal KASAN based build with gcc does produce build warnings or
> failures when CONFIG_WERROR is set.
>
> The 1280 byte limit is an arbitrary choice here, as it disables almost
> all warnings but leaves a few outliers that are probably better addressed
> with code changes. There are a number of functions that are just below the
> normal 1024 byte limit at the moment, and an extra 256 bytes for those
> is within the expectation, but more than that is probably an indication
> of a bad design or a real bug.
>
> Cc: Uwe Kleine-K?nig <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: [email protected]
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> lib/Kconfig.debug | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 3da5f9acb966..8a3afd837e99 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -387,9 +387,9 @@ endif # DEBUG_INFO
> config FRAME_WARN
> int "Warn for stack frames larger than"
> range 0 8192
> - default 2048 if GCC_PLUGIN_LATENT_ENTROPY
> default 2048 if PARISC
> default 1536 if (!64BIT && XTENSA)
> + default 1280 if (!64BIT && KASAN_STACK)

For what it's worth, if this was changed to just KASAN, this would
improve the situation for clang and ARCH=arm allmodconfig as well. It
leaves just two -Wframe-larger-than warnings in
drivers/gpu/drm/amd/display, which can be worked around with a kernel
side change:

https://github.com/ClangBuiltLinux/linux/issues/1455#issuecomment-995337569

Cheers,
Nathan