Re: [PATCH] arm64: kernel: remove SHF_WRITE|SHF_EXECINSTR from .idmap.text

[Ard Biesheuvel]

Hi Nick,

On Fri, 28 Apr 2023 at 19:09, <ndesaulniers@xxxxxxxxxx> wrote:
>
> commit d54170812ef1 ("arm64: fix .idmap.text assertion for large kernels")
> modified some of the section assembler directives that declare
> .idmap.text to be SHF_ALLOC instead of
> SHF_ALLOC|SHF_WRITE|SHF_EXECINSTR.
>
> This patch fixes up the remaining stragglers that were left behind.
>
> Because .idmap.text is merged into .text,

Nit: this is no longer the case: the ID map code is never executed via
the kernel mapping, so we moved it into a special .rodata.text section
that contains all generated code that should not have an executable
mapping by default, but only when it gets copied and/or mapped into a
different executable region.

This doesn't impact the correctness of the patch, so with this
paragraph clarified:

Reviewed-by: Ard Biesheuvel <ardb@xxxxxxxxxx>


> LLD will retain the
> SHF_EXECINSTR on .text, in addition to the synthetic .got.  This doesn't
> matter to the kernel loader, but syzkaller is having trouble symboling
> such sections. Clean this up while we additionally fix up syzkaller. Add
> Fixes tag so that this doesn't precede related change in stable.
>
> Fixes: d54170812ef1 ("arm64: fix .idmap.text assertion for large kernels")
> Reported-by: Greg Thelen <gthelen@xxxxxxxxxx>
> Signed-off-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> ---
>  arch/arm64/kernel/cpu-reset.S | 2 +-
>  arch/arm64/kernel/sleep.S     | 2 +-
>  arch/arm64/mm/proc.S          | 6 +++---
>  3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/kernel/cpu-reset.S b/arch/arm64/kernel/cpu-reset.S
> index 6b752fe89745..c87445dde674 100644
> --- a/arch/arm64/kernel/cpu-reset.S
> +++ b/arch/arm64/kernel/cpu-reset.S
> @@ -14,7 +14,7 @@
>  #include <asm/virt.h>
>
>  .text
> -.pushsection    .idmap.text, "awx"
> +.pushsection    .idmap.text, "a"
>
>  /*
>   * cpu_soft_restart(el2_switch, entry, arg0, arg1, arg2)
> diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S
> index 2ae7cff1953a..2aa5129d8253 100644
> --- a/arch/arm64/kernel/sleep.S
> +++ b/arch/arm64/kernel/sleep.S
> @@ -97,7 +97,7 @@ SYM_FUNC_START(__cpu_suspend_enter)
>         ret
>  SYM_FUNC_END(__cpu_suspend_enter)
>
> -       .pushsection ".idmap.text", "awx"
> +       .pushsection ".idmap.text", "a"
>  SYM_CODE_START(cpu_resume)
>         mov     x0, xzr
>         bl      init_kernel_el
> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index 91410f488090..c2cb437821ca 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -167,7 +167,7 @@ alternative_else_nop_endif
>  SYM_FUNC_END(cpu_do_resume)
>  #endif
>
> -       .pushsection ".idmap.text", "awx"
> +       .pushsection ".idmap.text", "a"
>
>  .macro __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
>         adrp    \tmp1, reserved_pg_dir
> @@ -201,7 +201,7 @@ SYM_FUNC_END(idmap_cpu_replace_ttbr1)
>
>  #define KPTI_NG_PTE_FLAGS      (PTE_ATTRINDX(MT_NORMAL) | SWAPPER_PTE_FLAGS)
>
> -       .pushsection ".idmap.text", "awx"
> +       .pushsection ".idmap.text", "a"
>
>         .macro  kpti_mk_tbl_ng, type, num_entries
>         add     end_\type\()p, cur_\type\()p, #\num_entries * 8
> @@ -400,7 +400,7 @@ SYM_FUNC_END(idmap_kpti_install_ng_mappings)
>   * Output:
>   *     Return in x0 the value of the SCTLR_EL1 register.
>   */
> -       .pushsection ".idmap.text", "awx"
> +       .pushsection ".idmap.text", "a"
>  SYM_FUNC_START(__cpu_setup)
>         tlbi    vmalle1                         // Invalidate local TLB
>         dsb     nsh
>
> ---
> base-commit: 22b8cc3e78f5448b4c5df00303817a9137cd663f
> change-id: 20230428-awx-c73f4bde79c4
>
> Best regards,
> --
> Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
>