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  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]
Date:   Tue, 6 Apr 2021 12:36:57 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Sami Tolvanen <samitolvanen@...gle.com>
Cc:     Kees Cook <keescook@...omium.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Will Deacon <will@...nel.org>, Jessica Yu <jeyu@...nel.org>,
        Arnd Bergmann <arnd@...db.de>, Tejun Heo <tj@...nel.org>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Christoph Hellwig <hch@...radead.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Sedat Dilek <sedat.dilek@...il.com>,
        Catalin Marinas <catalin.marinas@....com>, bpf@...r.kernel.org,
        linux-hardening@...r.kernel.org, linux-arch@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kbuild@...r.kernel.org,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        clang-built-linux@...glegroups.com
Subject: Re: [PATCH v5 12/18] arm64: implement function_nocfi

On Thu, Apr 01, 2021 at 04:32:10PM -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>
> Reviewed-by: Kees Cook <keescook@...omium.org>

I think that it's unfortunate that we have to drop to assembly here, but
given this is infrequent I agree it's not the end of the world, so:

Acked-by: Mark Rutland <mark.rutland@....com>

> ---
>  arch/arm64/include/asm/memory.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index 0aabc3be9a75..b55410afd3d1 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -321,6 +321,21 @@ static inline void *phys_to_virt(phys_addr_t x)
>  #define virt_to_pfn(x)		__phys_to_pfn(__virt_to_phys((unsigned long)(x)))
>  #define sym_to_pfn(x)		__phys_to_pfn(__pa_symbol(x))
>  
> +#ifdef CONFIG_CFI_CLANG
> +/*
> + * 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("adrp %0, " __stringify(x) "\n\t"				\
> +	    "add  %0, %0, :lo12:" __stringify(x) : "=r" (addr));	\

If it's not too painful, could we please move the asm constrain onto its
own line? That makes it slightly easier to read, and aligns with what
we've (mostly) done elsewhere in arm64.

Not a big deal either way, and the ack stands regardless.

Thanks,
Mark.

> +	addr;								\
> +})
> +#endif
> +
>  /*
>   *  virt_to_page(x)	convert a _valid_ virtual address to struct page *
>   *  virt_addr_valid(x)	indicates whether a virtual address is valid
> -- 
> 2.31.0.208.g409f899ff0-goog
> 

Powered by blists - more mailing lists