Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 16 Apr 2021 23:18:55 +0200
From:   Borislav Petkov <bp@alien8.de>
To:     Sami Tolvanen <samitolvanen@google.com>
Cc:     x86@kernel.org, Kees Cook <keescook@chromium.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Sedat Dilek <sedat.dilek@gmail.com>,
        linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org,
        clang-built-linux@googlegroups.com
Subject: Re: [PATCH 05/15] x86: Implement function_nocfi
Message-ID: <20210416211855.GD22348@zn.tnic>
References: <20210416203844.3803177-1-samitolvanen@google.com>
 <20210416203844.3803177-6-samitolvanen@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20210416203844.3803177-6-samitolvanen@google.com>
Precedence: bulk

On Fri, Apr 16, 2021 at 01:38:34PM -0700, Sami Tolvanen wrote:
> With CONFIG_CFI_CLANG, the compiler replaces function addresses in
> instrumented C code with jump table addresses. This change implements
> the function_nocfi() macro, which returns the actual function address
> instead.
> 
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> ---
>  arch/x86/include/asm/page.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
> index 7555b48803a8..5499a05c44fc 100644
> --- a/arch/x86/include/asm/page.h
> +++ b/arch/x86/include/asm/page.h
> @@ -71,6 +71,20 @@ static inline void copy_user_page(void *to, void *from, unsigned long vaddr,
>  extern bool __virt_addr_valid(unsigned long kaddr);
>  #define virt_addr_valid(kaddr)	__virt_addr_valid((unsigned long) (kaddr))
>  
> +#ifdef CONFIG_CFI_CLANG

Almost every patch is talking about this magical config symbol but it
is nowhere to be found. How do I disable it, is there a Kconfig entry
somewhere, etc, etc?

> +/*
> + * With CONFIG_CFI_CLANG, the compiler replaces function address
> + * references with the address of the function's CFI jump table
> + * entry. The function_nocfi macro always returns the address of the
> + * actual function instead.
> + */
> +#define function_nocfi(x) ({						\
> +	void *addr;							\
> +	asm("leaq " __stringify(x) "(%%rip), %0\n\t" : "=r" (addr));	\
> +	addr;								\
> +})

Also, eww.

It seems all these newfangled things we get, mean obfuscating code even
more. What's wrong with using __nocfi instead? That's nicely out of the
way so slap that in front of functions.

Also, did you even build this on, say, 32-bit allmodconfig?

Oh well.

In file included from ./include/linux/ftrace.h:22:0,
                 from ./include/linux/init_task.h:9,
                 from init/init_task.c:2:
./include/linux/ftrace.h: In function ‘ftrace_init_nop’:
./arch/x86/include/asm/ftrace.h:9:40: error: implicit declaration of function ‘function_nocfi’ [-Werror=implicit-function-declaration]
 # define MCOUNT_ADDR  ((unsigned long)(function_nocfi(__fentry__)))
                                        ^
./include/linux/ftrace.h:671:35: note: in expansion of macro ‘MCOUNT_ADDR’
  return ftrace_make_nop(mod, rec, MCOUNT_ADDR);
                                   ^~~~~~~~~~~
In file included from ./include/linux/ftrace.h:22:0,
                 from ./include/linux/perf_event.h:49,
                 from ./include/linux/trace_events.h:10,
                 from ./include/trace/syscall.h:7,
                 from ./include/linux/syscalls.h:85,
                 from init/main.c:21:
./include/linux/ftrace.h: In function ‘ftrace_init_nop’:
./arch/x86/include/asm/ftrace.h:9:40: error: implicit declaration of function ‘function_nocfi’ [-Werror=implicit-function-declaration]
 # define MCOUNT_ADDR  ((unsigned long)(function_nocfi(__fentry__)))
                                        ^
./include/linux/ftrace.h:671:35: note: in expansion of macro ‘MCOUNT_ADDR’
  return ftrace_make_nop(mod, rec, MCOUNT_ADDR);
                                   ^~~~~~~~~~~
In file included from ./include/linux/ftrace.h:22:0,
                 from ./include/linux/perf_event.h:49,
                 from ./include/linux/trace_events.h:10,
                 from ./include/trace/syscall.h:7,
                 from ./include/linux/syscalls.h:85,
                 from init/initramfs.c:10:
./include/linux/ftrace.h: In function ‘ftrace_init_nop’:
./arch/x86/include/asm/ftrace.h:9:40: error: implicit declaration of function ‘function_nocfi’ [-Werror=implicit-function-declaration]
 # define MCOUNT_ADDR  ((unsigned long)(function_nocfi(__fentry__)))
                                        ^
./include/linux/ftrace.h:671:35: note: in expansion of macro ‘MCOUNT_ADDR’
  return ftrace_make_nop(mod, rec, MCOUNT_ADDR);
                                   ^~~~~~~~~~~
In file included from ./include/linux/ftrace.h:22:0,
                 from ./include/linux/perf_event.h:49,
                 from arch/x86/events/core.c:15:
./include/linux/ftrace.h: In function ‘ftrace_init_nop’:
./arch/x86/include/asm/ftrace.h:9:40: error: implicit declaration of function ‘function_nocfi’ [-Werror=implicit-function-declaration]
 # define MCOUNT_ADDR  ((unsigned long)(function_nocfi(__fentry__)))
...


-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette