Currently, whether the alloc/free stack traces collection is enabled by
default for hardware tag-based KASAN depends on CONFIG_DEBUG_KERNEL.
The intention for this dependency was to only enable collection on slow
debug kernels due to a significant perf and memory impact.
As it turns out, CONFIG_DEBUG_KERNEL is not considered a debug option
and is enabled on many productions kernels including Android and Ubuntu.
As the result, this dependency is pointless and only complicates the code
and documentation.
Having stack traces collection disabled by default would make the hardware
mode work differently to to the software ones, which is confusing.
This change removes the dependency and enables stack traces collection
by default.
Looking into the future, this default might makes sense for production
kernels, assuming we implement a fast stack trace collection approach.
Signed-off-by: Andrey Konovalov <[email protected]>
---
Documentation/dev-tools/kasan.rst | 3 +--
mm/kasan/hw_tags.c | 8 ++------
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
index 1651d961f06a..a248ac3941be 100644
--- a/Documentation/dev-tools/kasan.rst
+++ b/Documentation/dev-tools/kasan.rst
@@ -163,8 +163,7 @@ particular KASAN features.
- ``kasan=off`` or ``=on`` controls whether KASAN is enabled (default: ``on``).
- ``kasan.stacktrace=off`` or ``=on`` disables or enables alloc and free stack
- traces collection (default: ``on`` for ``CONFIG_DEBUG_KERNEL=y``, otherwise
- ``off``).
+ traces collection (default: ``on``).
- ``kasan.fault=report`` or ``=panic`` controls whether to only print a KASAN
report or also panic the kernel (default: ``report``).
diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
index e529428e7a11..d558799b25b3 100644
--- a/mm/kasan/hw_tags.c
+++ b/mm/kasan/hw_tags.c
@@ -134,12 +134,8 @@ void __init kasan_init_hw_tags(void)
switch (kasan_arg_stacktrace) {
case KASAN_ARG_STACKTRACE_DEFAULT:
- /*
- * Default to enabling stack trace collection for
- * debug kernels.
- */
- if (IS_ENABLED(CONFIG_DEBUG_KERNEL))
- static_branch_enable(&kasan_flag_stacktrace);
+ /* Default to enabling stack trace collection. */
+ static_branch_enable(&kasan_flag_stacktrace);
break;
case KASAN_ARG_STACKTRACE_OFF:
/* Do nothing, kasan_flag_stacktrace keeps its default value. */
--
2.30.0.478.g8a0d178c01-goog
On Mon, 8 Feb 2021 at 19:40, Andrey Konovalov <[email protected]> wrote:
>
> Currently, whether the alloc/free stack traces collection is enabled by
> default for hardware tag-based KASAN depends on CONFIG_DEBUG_KERNEL.
> The intention for this dependency was to only enable collection on slow
> debug kernels due to a significant perf and memory impact.
>
> As it turns out, CONFIG_DEBUG_KERNEL is not considered a debug option
> and is enabled on many productions kernels including Android and Ubuntu.
> As the result, this dependency is pointless and only complicates the code
> and documentation.
>
> Having stack traces collection disabled by default would make the hardware
> mode work differently to to the software ones, which is confusing.
>
> This change removes the dependency and enables stack traces collection
> by default.
>
> Looking into the future, this default might makes sense for production
> kernels, assuming we implement a fast stack trace collection approach.
>
> Signed-off-by: Andrey Konovalov <[email protected]>
Reviewed-by: Marco Elver <[email protected]>
I'm in favor of this simplification.
The fact that CONFIG_DEBUG_KERNEL cannot be relied upon to determine
if we're running a debug kernel or not is a bit unfortunate though.
Thanks!
> ---
> Documentation/dev-tools/kasan.rst | 3 +--
> mm/kasan/hw_tags.c | 8 ++------
> 2 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
> index 1651d961f06a..a248ac3941be 100644
> --- a/Documentation/dev-tools/kasan.rst
> +++ b/Documentation/dev-tools/kasan.rst
> @@ -163,8 +163,7 @@ particular KASAN features.
> - ``kasan=off`` or ``=on`` controls whether KASAN is enabled (default: ``on``).
>
> - ``kasan.stacktrace=off`` or ``=on`` disables or enables alloc and free stack
> - traces collection (default: ``on`` for ``CONFIG_DEBUG_KERNEL=y``, otherwise
> - ``off``).
> + traces collection (default: ``on``).
>
> - ``kasan.fault=report`` or ``=panic`` controls whether to only print a KASAN
> report or also panic the kernel (default: ``report``).
> diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
> index e529428e7a11..d558799b25b3 100644
> --- a/mm/kasan/hw_tags.c
> +++ b/mm/kasan/hw_tags.c
> @@ -134,12 +134,8 @@ void __init kasan_init_hw_tags(void)
>
> switch (kasan_arg_stacktrace) {
> case KASAN_ARG_STACKTRACE_DEFAULT:
> - /*
> - * Default to enabling stack trace collection for
> - * debug kernels.
> - */
> - if (IS_ENABLED(CONFIG_DEBUG_KERNEL))
> - static_branch_enable(&kasan_flag_stacktrace);
> + /* Default to enabling stack trace collection. */
> + static_branch_enable(&kasan_flag_stacktrace);
> break;
> case KASAN_ARG_STACKTRACE_OFF:
> /* Do nothing, kasan_flag_stacktrace keeps its default value. */
> --
> 2.30.0.478.g8a0d178c01-goog
>