linux-kernel - Re: [PATCH v7 07/28] x86/asm/crypto: annotate local functions

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]

Date:   Mon, 25 Feb 2019 11:13:07 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Jiri Slaby <jslaby@...e.cz>
Cc:     mingo@...hat.com, linux-arch@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        linux-crypto@...r.kernel.org
Subject: Re: [PATCH v7 07/28] x86/asm/crypto: annotate local functions

On Wed, Jan 30, 2019 at 01:46:50PM +0100, Jiri Slaby wrote:
> Use the newly added SYM_FUNC_START_LOCAL to annotate starts of all
> functions which do not have ".globl" annotation, but their ends are
> annotated by ENDPROC. This is needed to balance ENDPROC for tools that
> generate debuginfo.
> 
> To be symmetric, we also convert their ENDPROCs to the new SYM_FUNC_END.
> 
> Signed-off-by: Jiri Slaby <jslaby@...e.cz>
> Cc: Herbert Xu <herbert@...dor.apana.org.au>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: "H. Peter Anvin" <hpa@...or.com>
> Cc: <x86@...nel.org>
> Cc: <linux-crypto@...r.kernel.org>
> ---
>  arch/x86/crypto/aegis128-aesni-asm.S         |  8 ++--
>  arch/x86/crypto/aegis128l-aesni-asm.S        |  8 ++--
>  arch/x86/crypto/aegis256-aesni-asm.S         |  8 ++--
>  arch/x86/crypto/aesni-intel_asm.S            | 49 ++++++++------------
>  arch/x86/crypto/camellia-aesni-avx-asm_64.S  | 20 ++++----
>  arch/x86/crypto/camellia-aesni-avx2-asm_64.S | 20 ++++----
>  arch/x86/crypto/cast5-avx-x86_64-asm_64.S    |  8 ++--
>  arch/x86/crypto/cast6-avx-x86_64-asm_64.S    |  8 ++--
>  arch/x86/crypto/chacha-ssse3-x86_64.S        |  4 +-
>  arch/x86/crypto/ghash-clmulni-intel_asm.S    |  4 +-
>  arch/x86/crypto/morus1280-avx2-asm.S         | 16 +++----
>  arch/x86/crypto/morus1280-sse2-asm.S         | 16 +++----
>  arch/x86/crypto/morus640-sse2-asm.S          | 16 +++----
>  arch/x86/crypto/serpent-avx-x86_64-asm_64.S  |  8 ++--
>  arch/x86/crypto/serpent-avx2-asm_64.S        |  8 ++--
>  arch/x86/crypto/twofish-avx-x86_64-asm_64.S  |  8 ++--
>  16 files changed, 100 insertions(+), 109 deletions(-)
> 
> diff --git a/arch/x86/crypto/aegis128-aesni-asm.S b/arch/x86/crypto/aegis128-aesni-asm.S
> index 5f7e43d4f64a..87b94664296a 100644
> --- a/arch/x86/crypto/aegis128-aesni-asm.S
> +++ b/arch/x86/crypto/aegis128-aesni-asm.S
> @@ -74,7 +74,7 @@
>   *   %r8
>   *   %r9
>   */
> -__load_partial:
> +SYM_FUNC_START_LOCAL(__load_partial)

Btw, here's what I mean with it being problematic when the global symbol
names are not distinctive enough:

arch/x86/crypto/aegis128-aesni-asm.S:77:SYM_FUNC_START_LOCAL(__load_partial)
arch/x86/crypto/aegis128l-aesni-asm.S:68:SYM_FUNC_START_LOCAL(__load_partial)
arch/x86/crypto/aegis256-aesni-asm.S:61:SYM_FUNC_START_LOCAL(__load_partial)
arch/x86/crypto/morus1280-avx2-asm.S:115:SYM_FUNC_START_LOCAL(__load_partial)
arch/x86/crypto/morus1280-sse2-asm.S:237:SYM_FUNC_START_LOCAL(__load_partial)
arch/x86/crypto/morus640-sse2-asm.S:115:SYM_FUNC_START_LOCAL(__load_partial)

There are a bunch of those __{load,store}_partial labels and staring at
traces with them in it does not really help, unless you know which one
is it.

Should not be addressed by your patchset but those should be local
labels...

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.