linux-kernel - Re: [PATCH v4 09/29] arm64: disable recordmcount with DYNAMIC_FTRACE_WITH

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: <20200930095850.GA68612@C02TD0UTHF1T.local>
Date:   Wed, 30 Sep 2020 10:58:50 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Sami Tolvanen <samitolvanen@...gle.com>
Cc:     Masahiro Yamada <masahiroy@...nel.org>,
        Will Deacon <will@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        clang-built-linux@...glegroups.com,
        kernel-hardening@...ts.openwall.com, linux-arch@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kbuild@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
        x86@...nel.org
Subject: Re: [PATCH v4 09/29] arm64: disable recordmcount with
 DYNAMIC_FTRACE_WITH_REGS

Hi Sami,

On Tue, Sep 29, 2020 at 02:46:11PM -0700, Sami Tolvanen wrote:
> Select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY to disable
> recordmcount when DYNAMIC_FTRACE_WITH_REGS is selected.

Could you please add an explanation as to /why/ this is necessary in the
commit message? I couldn't figure this out form the commit message
alone, and reading the cover letter also didn't help.

If the minimum required GCC version supports patchable-function-entry
I'd be happy to make that a requirement for dynamic ftrace on arm64, as
then we'd only need to support one mechanism, and can get rid of some
redundant code. We already default to it when present anyhow.

> 
> Signed-off-by: Sami Tolvanen <samitolvanen@...gle.com>
> ---
>  arch/arm64/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 6d232837cbee..ad522b021f35 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -155,6 +155,8 @@ config ARM64
>  	select HAVE_DYNAMIC_FTRACE
>  	select HAVE_DYNAMIC_FTRACE_WITH_REGS \
>  		if $(cc-option,-fpatchable-function-entry=2)
> +	select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY \
> +		if DYNAMIC_FTRACE_WITH_REGS

This doesn't look quite right to me. Presumably we shouldn't allow
DYNAMIC_FTRACE_WITH_REGS to be selected if HAVE_DYNAMIC_FTRACE_WITH_REGS
isn't.

Thanks,
Mark.