linux-kernel - Re: [PATCH v5 1/2] ARM: kprobes: fix UNPREDICTABLE warnings

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:   Thu, 11 Feb 2021 09:15:00 +0100
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
Cc:     Russell King <linux@...linux.org.uk>,
        Arnd Bergmann <arnd@...nel.org>,
        clang-built-linux <clang-built-linux@...glegroups.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Peter Smith <peter.smith@....com>,
        Renato Golin <rengolin@...temcall.eu>,
        David Spickett <david.spickett@...aro.org>,
        "kernelci . org bot" <bot@...nelci.org>
Subject: Re: [PATCH v5 1/2] ARM: kprobes: fix UNPREDICTABLE warnings

On Thu, 11 Feb 2021 at 03:52, Nick Desaulniers <ndesaulniers@...gle.com> wrote:
>
> GNU as warns twice for this file:
> Warning: using r15 results in unpredictable behaviour
>
> via the Arm ARM:
> K1.1.1 Overview of the constraints on Armv7 UNPREDICTABLE behaviors
>
>   The term UNPREDICTABLE describes a number of cases where the
>   architecture has a feature that software must not use.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1271
> Link: https://reviews.llvm.org/D95586
> Reported-by: kernelci.org bot <bot@...nelci.org>
> Suggested-by: Peter Smith <peter.smith@....com>
> Suggested-by: Renato Golin <rengolin@...temcall.eu>
> Suggested-by: David Spickett <david.spickett@...aro.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@...gle.com>

Acked-by: Ard Biesheuvel <ardb@...nel.org>

But can we add a bit more context to the commit log, please? It is not
obvious to the casual reader why it is OK to emit UNPREDICTABLE
opcodes, i.e., that these are selftests that aim to ensure that kprobe
never attempts to replace the opcodes in question with a probe, but
that they are not actually executed, or expected to occur in real
code.

> ---
>  arch/arm/probes/kprobes/test-arm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/probes/kprobes/test-arm.c b/arch/arm/probes/kprobes/test-arm.c
> index 977369f1aa48..2543106a203e 100644
> --- a/arch/arm/probes/kprobes/test-arm.c
> +++ b/arch/arm/probes/kprobes/test-arm.c
> @@ -166,10 +166,10 @@ void kprobe_arm_test_cases(void)
>
>         /* Data-processing with PC as a target and status registers updated */
>         TEST_UNSUPPORTED("movs  pc, r1")
> -       TEST_UNSUPPORTED("movs  pc, r1, lsl r2")
> +       TEST_UNSUPPORTED(__inst_arm(0xe1b0f211) "       @movs   pc, r1, lsl r2")
>         TEST_UNSUPPORTED("movs  pc, #0x10000")
>         TEST_UNSUPPORTED("adds  pc, lr, r1")
> -       TEST_UNSUPPORTED("adds  pc, lr, r1, lsl r2")
> +       TEST_UNSUPPORTED(__inst_arm(0xe09ef211) "       @adds   pc, lr, r1, lsl r2")
>         TEST_UNSUPPORTED("adds  pc, lr, #4")
>
>         /* Data-processing with SP as target */
> --
> 2.30.0.478.g8a0d178c01-goog
>