linux-kernel - Re: [PATCH 05/15] x86: Implement function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210416211855.GD22348@zn.tnic>
Date:   Fri, 16 Apr 2021 23:18:55 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Sami Tolvanen <samitolvanen@...gle.com>
Cc:     x86@...nel.org, Kees Cook <keescook@...omium.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Sedat Dilek <sedat.dilek@...il.com>,
        linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org,
        clang-built-linux@...glegroups.com
Subject: Re: [PATCH 05/15] x86: Implement function_nocfi

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@...gle.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