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, 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 <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
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 <[email protected]>
Hi Nick,
On Fri, 28 Apr 2023 at 19:09, <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> 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 <[email protected]>
>
On Fri, Apr 28, 2023 at 11:17 AM Ard Biesheuvel <[email protected]> wrote:
>
> Hi Nick,
>
> On Fri, 28 Apr 2023 at 19:09, <[email protected]> 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.
Ah, sorry, I see IDMAP_TEXT is being placed in .rodata.text in
arch/arm64/kernel/vmlinux.lds.S, so it's just the synthetic .got then
that's affecting .text. So Fangrui's patch is meant to address that.
https://lore.kernel.org/linux-arm-kernel/[email protected]/
In that case, I'll drop this paragraph starting with "Because"; I
misspelled symbolizing anyways.
>
> This doesn't impact the correctness of the patch, so with this
> paragraph clarified:
>
> Reviewed-by: Ard Biesheuvel <[email protected]>
>
>
> > 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 <[email protected]>
> > Signed-off-by: Nick Desaulniers <[email protected]>
> > ---
> > 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 <[email protected]>
> >
--
Thanks,
~Nick Desaulniers