On Mon, 5 Nov 2018 14:15:35 +0100
Martin Schwidefsky <schwidefsky@xxxxxxxxxx> wrote:
>
> Follow-up question: the __no_sanitize_address_or_inline define has the 'notrace'
> option that is missing for __no_kasan_or_inline. We need that option for
> __load_psw_mask, if we do the replacement all users of __no_kasan_or_inline
> would get the option as well. This affects __read_once_size_nocheck and
> read_word_at_a_time. Do these function have to be traceable ?
Some history on why I added notrace to inline. It was to make things
more consistent. Since gcc wont add a fentry/mcount call to inlined
functions, having those functions show up in the trace depending on
whether or not gcc honored the "inline" tag made things confusing. And
it even once caused a crash when local_irq_save() stopped being
inlined.
I added notrace to inline so that if you mark something as inline, it
would not show up in the trace regardless of gcc deciding to inline the
function or not.
>
> This patch would work for me:
> --
> >From 4aaa09fe4b54e930edabac86606dee979b12647c Mon Sep 17 00:00:00 2001
> From: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
> Date: Mon, 5 Nov 2018 07:36:28 +0100
> Subject: [PATCH] compiler: remove __no_sanitize_address_or_inline again
>
> The __no_sanitize_address_or_inline and __no_kasan_or_inline defines
> are almost identical. The only difference is that __no_kasan_or_inline
> does not have the 'notrace' attribute.
>
> To be able to replace __no_sanitize_address_or_inline with the older
> definition, add 'notrace' to __no_kasan_or_inline and change to two
> users of __no_sanitize_address_or_inline in the s390 code.
>
> The 'notrace' option is necessary for e.g. the __load_psw_mask function
> in arch/s390/include/asm/processor.h. Without the option it is possible
> to trace __load_psw_mask which leads to kernel stack overflow.
Acked-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>
-- Steve
>
> Signed-off-by: Martin Schwidefsky <schwidefsky@xxxxxxxxxx>
> ---
> arch/s390/include/asm/processor.h | 4 ++--
> include/linux/compiler-gcc.h | 12 ------------
> include/linux/compiler.h | 2 +-
> 3 files changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/arch/s390/include/asm/processor.h b/arch/s390/include/asm/processor.h
> index 302795c47c06..81038ab357ce 100644
> --- a/arch/s390/include/asm/processor.h
> +++ b/arch/s390/include/asm/processor.h
> @@ -236,7 +236,7 @@ static inline unsigned long current_stack_pointer(void)
> return sp;
> }
>
> -static __no_sanitize_address_or_inline unsigned short stap(void)
> +static __no_kasan_or_inline unsigned short stap(void)
> {
> unsigned short cpu_address;
>
> @@ -330,7 +330,7 @@ static inline void __load_psw(psw_t psw)
> * Set PSW mask to specified value, while leaving the
> * PSW addr pointing to the next instruction.
> */
> -static __no_sanitize_address_or_inline void __load_psw_mask(unsigned long mask)
> +static __no_kasan_or_inline void __load_psw_mask(unsigned long mask)
> {
> unsigned long addr;
> psw_t psw;
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index c0f5db3a9621..2010493e1040 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -143,18 +143,6 @@
> #define KASAN_ABI_VERSION 3
> #endif
>
> -/*
> - * Because __no_sanitize_address conflicts with inlining:
> - * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
> - * we do one or the other.
> - */
> -#ifdef CONFIG_KASAN
> -#define __no_sanitize_address_or_inline \
> - __no_sanitize_address __maybe_unused notrace
> -#else
> -#define __no_sanitize_address_or_inline inline
> -#endif
> -
> #if GCC_VERSION >= 50100
> #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
> #endif
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 18c80cfa4fc4..06396c1cf127 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -189,7 +189,7 @@ void __read_once_size(const volatile void *p, void *res, int size)
> * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
> * '__maybe_unused' allows us to avoid defined-but-not-used warnings.
> */
> -# define __no_kasan_or_inline __no_sanitize_address __maybe_unused
> +# define __no_kasan_or_inline __no_sanitize_address notrace __maybe_unused
> #else
> # define __no_kasan_or_inline __always_inline
> #endif
