linux-kernel - Re: [PATCH 4.4 072/101] arm64: assembler: make adr

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: <CAKv+Gu85oZufQHX5sz7TXceObQo9hc4c7_q9PzkEWRqxcFPB_A@mail.gmail.com>
Date:   Tue, 4 Jul 2017 10:24:37 +0100
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Mark Rutland <mark.rutland@....com>,
        Will Deacon <will.deacon@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Sasha Levin <alexander.levin@...izon.com>
Subject: Re: [PATCH 4.4 072/101] arm64: assembler: make adr_l work in modules
 under KASLR

On 3 July 2017 at 14:35, Greg Kroah-Hartman <gregkh@...uxfoundation.org> wrote:
> 4.4-stable review patch.  If anyone has any objections, please let me know.
>
> ------------------
>
> From: Ard Biesheuvel <ard.biesheuvel@...aro.org>
>
>
> [ Upstream commit 41c066f2c4d436c535616fe182331766c57838f0 ]
>
> When CONFIG_RANDOMIZE_MODULE_REGION_FULL=y, the offset between loaded
> modules and the core kernel may exceed 4 GB, putting symbols exported
> by the core kernel out of the reach of the ordinary adrp/add instruction
> pairs used to generate relative symbol references. So make the adr_l
> macro emit a movz/movk sequence instead when executing in module context.
>
> While at it, remove the pointless special case for the stack pointer.
>

As it turns out, this 'pointless special case' was not so pointless in
v4.4, and removing it breaks the build.

Given that the only in-module user of adr_l is the new scalar AES
code, which is not backported to v4.4, there is really no point in
backporting this to v4.4.

-- 
Ard.


> Acked-by: Mark Rutland <mark.rutland@....com>
> Acked-by: Will Deacon <will.deacon@....com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...aro.org>
> Signed-off-by: Catalin Marinas <catalin.marinas@....com>
> Signed-off-by: Sasha Levin <alexander.levin@...izon.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> ---
>  arch/arm64/include/asm/assembler.h |   36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
>
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -147,22 +147,25 @@ lr        .req    x30             // link register
>
>  /*
>   * Pseudo-ops for PC-relative adr/ldr/str <reg>, <symbol> where
> - * <symbol> is within the range +/- 4 GB of the PC.
> + * <symbol> is within the range +/- 4 GB of the PC when running
> + * in core kernel context. In module context, a movz/movk sequence
> + * is used, since modules may be loaded far away from the kernel
> + * when KASLR is in effect.
>   */
>         /*
>          * @dst: destination register (64 bit wide)
>          * @sym: name of the symbol
> -        * @tmp: optional scratch register to be used if <dst> == sp, which
> -        *       is not allowed in an adrp instruction
>          */
> -       .macro  adr_l, dst, sym, tmp=
> -       .ifb    \tmp
> +       .macro  adr_l, dst, sym
> +#ifndef MODULE
>         adrp    \dst, \sym
>         add     \dst, \dst, :lo12:\sym
> -       .else
> -       adrp    \tmp, \sym
> -       add     \dst, \tmp, :lo12:\sym
> -       .endif
> +#else
> +       movz    \dst, #:abs_g3:\sym
> +       movk    \dst, #:abs_g2_nc:\sym
> +       movk    \dst, #:abs_g1_nc:\sym
> +       movk    \dst, #:abs_g0_nc:\sym
> +#endif
>         .endm
>
>         /*
> @@ -173,6 +176,7 @@ lr  .req    x30             // link register
>          *       the address
>          */
>         .macro  ldr_l, dst, sym, tmp=
> +#ifndef MODULE
>         .ifb    \tmp
>         adrp    \dst, \sym
>         ldr     \dst, [\dst, :lo12:\sym]
> @@ -180,6 +184,15 @@ lr .req    x30             // link register
>         adrp    \tmp, \sym
>         ldr     \dst, [\tmp, :lo12:\sym]
>         .endif
> +#else
> +       .ifb    \tmp
> +       adr_l   \dst, \sym
> +       ldr     \dst, [\dst]
> +       .else
> +       adr_l   \tmp, \sym
> +       ldr     \dst, [\tmp]
> +       .endif
> +#endif
>         .endm
>
>         /*
> @@ -189,8 +202,13 @@ lr .req    x30             // link register
>          *       while <src> needs to be preserved.
>          */
>         .macro  str_l, src, sym, tmp
> +#ifndef MODULE
>         adrp    \tmp, \sym
>         str     \src, [\tmp, :lo12:\sym]
> +#else
> +       adr_l   \tmp, \sym
> +       str     \src, [\tmp]
> +#endif
>         .endm
>
>  /*
>
>