| Message ID | 20250507180852.work.231-kees@kernel.org |
|---|---|
| Headers | show |
| Series | stackleak: Support Clang stack depth tracking | expand |
On Wed, May 07, 2025 at 11:16:07AM -0700, Kees Cook wrote: > Force it to be __always_inline to make sure it is always available for > use with BUILD_BUG_ON(). Reviewed-by: Keith Busch <kbusch@kernel.org>
On Wed, May 07, 2025 at 08:45:15PM +0200, Ingo Molnar wrote: > > * Kees Cook <kees@kernel.org> wrote: > > > - The STACKLEAK gcc plugin instruments the kernel code for tracking > > + The STACKLEAK options instruments the kernel code for tracking > > speling. Thanks! > Also, any chance to fix this terrible name? Should be something like > KSTACKZERO or KSTACKCLEAR, to tell people that it doesn't leak the > stack but prevents leaks on the stack by clearing it, and that it's > about the kernel stack, not any other stack. Yeah, better to name it for what it does rather than want to protects against. The internal naming for what it does is "stack erase", so perhaps KSTACK_ERASE ? -Kees
+Cc KCOV maintainers On Wed, 7 May 2025 at 20:16, Kees Cook <kees@kernel.org> wrote: > > While __noinstr already contained __no_sanitize_coverage, it needs to > be added to __init and __head section markings to support the Clang > implementation of CONFIG_STACKLEAK. This is to make sure the stack depth > tracking callback is not executed in unsupported contexts. > > The other sanitizer coverage options (trace-pc and trace-cmp) aren't > needed in __head nor __init either ("We are interested in code coverage > as a function of a syscall inputs"[1]), so this appears safe to disable > for them as well. @ Dmitry, Aleksandr - Will this produce some unwanted side-effects for syzbot? I also think it's safe, but just double checking. > Link: https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/kcov.c?h=v6.14#n179 [1] > Signed-off-by: Kees Cook <kees@kernel.org> Acked-by: Marco Elver <elver@google.com> > --- > Cc: Marco Elver <elver@google.com> > Cc: Andrey Konovalov <andreyknvl@gmail.com> > Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: <x86@kernel.org> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Ard Biesheuvel <ardb@kernel.org> > Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > Cc: Hou Wenlong <houwenlong.hwl@antgroup.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Masahiro Yamada <masahiroy@kernel.org> > Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org> > Cc: Luis Chamberlain <mcgrof@kernel.org> > Cc: Sami Tolvanen <samitolvanen@google.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Christophe Leroy <christophe.leroy@csgroup.eu> > Cc: <kasan-dev@googlegroups.com> > --- > arch/x86/include/asm/init.h | 2 +- > include/linux/init.h | 4 +++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/init.h b/arch/x86/include/asm/init.h > index 8b1b1abcef15..6bfdaeddbae8 100644 > --- a/arch/x86/include/asm/init.h > +++ b/arch/x86/include/asm/init.h > @@ -5,7 +5,7 @@ > #if defined(CONFIG_CC_IS_CLANG) && CONFIG_CLANG_VERSION < 170000 > #define __head __section(".head.text") __no_sanitize_undefined __no_stack_protector > #else > -#define __head __section(".head.text") __no_sanitize_undefined > +#define __head __section(".head.text") __no_sanitize_undefined __no_sanitize_coverage > #endif > > struct x86_mapping_info { > diff --git a/include/linux/init.h b/include/linux/init.h > index ee1309473bc6..c65a050d52a7 100644 > --- a/include/linux/init.h > +++ b/include/linux/init.h > @@ -49,7 +49,9 @@ > > /* These are for everybody (although not all archs will actually > discard it in modules) */ > -#define __init __section(".init.text") __cold __latent_entropy __noinitretpoline > +#define __init __section(".init.text") __cold __latent_entropy \ > + __noinitretpoline \ > + __no_sanitize_coverage > #define __initdata __section(".init.data") > #define __initconst __section(".init.rodata") > #define __exitdata __section(".exit.data") > -- > 2.34.1 >
On Thu, 8 May 2025 at 14:23, Marco Elver <elver@google.com> wrote: > > +Cc KCOV maintainers > > On Wed, 7 May 2025 at 20:16, Kees Cook <kees@kernel.org> wrote: > > > > While __noinstr already contained __no_sanitize_coverage, it needs to > > be added to __init and __head section markings to support the Clang > > implementation of CONFIG_STACKLEAK. This is to make sure the stack depth > > tracking callback is not executed in unsupported contexts. > > > > The other sanitizer coverage options (trace-pc and trace-cmp) aren't > > needed in __head nor __init either ("We are interested in code coverage > > as a function of a syscall inputs"[1]), so this appears safe to disable > > for them as well. > > @ Dmitry, Aleksandr - Will this produce some unwanted side-effects for > syzbot? I also think it's safe, but just double checking. I do not see any problems with this. > > Link: https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/kcov.c?h=v6.14#n179 [1] > > Signed-off-by: Kees Cook <kees@kernel.org> > > Acked-by: Marco Elver <elver@google.com> > > > --- > > Cc: Marco Elver <elver@google.com> > > Cc: Andrey Konovalov <andreyknvl@gmail.com> > > Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com> > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Cc: Ingo Molnar <mingo@redhat.com> > > Cc: Borislav Petkov <bp@alien8.de> > > Cc: Dave Hansen <dave.hansen@linux.intel.com> > > Cc: <x86@kernel.org> > > Cc: "H. Peter Anvin" <hpa@zytor.com> > > Cc: Ard Biesheuvel <ardb@kernel.org> > > Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > > Cc: Hou Wenlong <houwenlong.hwl@antgroup.com> > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: Masahiro Yamada <masahiroy@kernel.org> > > Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org> > > Cc: Luis Chamberlain <mcgrof@kernel.org> > > Cc: Sami Tolvanen <samitolvanen@google.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Christophe Leroy <christophe.leroy@csgroup.eu> > > Cc: <kasan-dev@googlegroups.com> > > --- > > arch/x86/include/asm/init.h | 2 +- > > include/linux/init.h | 4 +++- > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/include/asm/init.h b/arch/x86/include/asm/init.h > > index 8b1b1abcef15..6bfdaeddbae8 100644 > > --- a/arch/x86/include/asm/init.h > > +++ b/arch/x86/include/asm/init.h > > @@ -5,7 +5,7 @@ > > #if defined(CONFIG_CC_IS_CLANG) && CONFIG_CLANG_VERSION < 170000 > > #define __head __section(".head.text") __no_sanitize_undefined __no_stack_protector > > #else > > -#define __head __section(".head.text") __no_sanitize_undefined > > +#define __head __section(".head.text") __no_sanitize_undefined __no_sanitize_coverage > > #endif > > > > struct x86_mapping_info { > > diff --git a/include/linux/init.h b/include/linux/init.h > > index ee1309473bc6..c65a050d52a7 100644 > > --- a/include/linux/init.h > > +++ b/include/linux/init.h > > @@ -49,7 +49,9 @@ > > > > /* These are for everybody (although not all archs will actually > > discard it in modules) */ > > -#define __init __section(".init.text") __cold __latent_entropy __noinitretpoline > > +#define __init __section(".init.text") __cold __latent_entropy \ > > + __noinitretpoline \ > > + __no_sanitize_coverage > > #define __initdata __section(".init.data") > > #define __initconst __section(".init.rodata") > > #define __exitdata __section(".exit.data") > > -- > > 2.34.1 > >