Messages in this thread Patch in this message | | | Date | Fri, 28 Apr 2023 18:58:14 +0000 | From | Fangrui Song <> | Subject | Re: [PATCH] arm64: lds: move .got section out of .text |
| |
On 2023-04-28, Ard Biesheuvel wrote: >Hello Fangrui,
Hello Ard, thank you for the rapid response.
>On Fri, 28 Apr 2023 at 06:05, Fangrui Song <maskray@google.com> wrote: >> >> Currently, the .got section is placed within the output section .text. >> However, when .got is non-empty, the SHF_WRITE flag is set when linked >> by lld. GNU ld recognizes .text as a special section and ignores the >> SHF_WRITE flag. By renaming .text, we can also get the SHF_WRITE flag. >> >> Conventionally, the .got section is placed just before .got.plt (which >> should be empty and omitted in the kernel). Therefore, we move the .got >> section to a conventional location (between .text and .data) and remove >> the unneeded `. = ALIGN(16)`. >> >> Signed-off-by: Fangrui Song <maskray@google.com> >> --- >> arch/arm64/kernel/vmlinux.lds.S | 20 ++++++++++---------- >> 1 file changed, 10 insertions(+), 10 deletions(-) >> >> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S >> index b9202c2ee18e..2bcb3b30db41 100644 >> --- a/arch/arm64/kernel/vmlinux.lds.S >> +++ b/arch/arm64/kernel/vmlinux.lds.S >> @@ -181,18 +181,8 @@ SECTIONS >> KPROBES_TEXT >> HYPERVISOR_TEXT >> *(.gnu.warning) >> - . = ALIGN(16); >> - *(.got) /* Global offset table */ >> } >> >> - /* >> - * Make sure that the .got.plt is either completely empty or it >> - * contains only the lazy dispatch entries. >> - */ >> - .got.plt : { *(.got.plt) } >> - ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, >> - "Unexpected GOT/PLT entries detected!") >> - >> . = ALIGN(SEGMENT_ALIGN); >> _etext = .; /* End of text section */ >> >> @@ -247,6 +237,16 @@ SECTIONS >> >> . = ALIGN(SEGMENT_ALIGN); >> __inittext_end = .; >> + >> + .got : { *(.got) } > >This is the .init region, which gets freed and unmapped after boot. If >the GOT is non-empty, it needs to remain mapped, so we cannot place it >here.
Thanks. I did not know the constraint.
>We have the same issue with the .rodata section, which incorporates >variables marked as __ro_after_init, which are not const qualified. So >given that .rodata is already emitted as WA, and we cannot do anything >about that, let's move the GOT in there.
Yes, writable .data..ro_after_init and __jump_table sections in include/asm-generic/vmlinux.lds.h (#define RO_DATA(align)) makes the output section .rodata writable. Perhaps this is very difficult to fix, and we will have writable .rodata for a long time.
What do you think of moving .got/.got.plt immediately before .data? I want to place .got/.got.plt before the guaranteed-writable sections, not some sections which are "unfortunately" writable (.rodata, __modver, .hyp.rodata, .rodata.text, etc).
For userspace programs, either linked with GNU ld or lld, .got/.got.plt are usually immediately before .data .
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S index b9202c2ee18e..48bd7c25b6ab 100644 --- a/arch/arm64/kernel/vmlinux.lds.S +++ b/arch/arm64/kernel/vmlinux.lds.S @@ -181,18 +181,8 @@ SECTIONS KPROBES_TEXT HYPERVISOR_TEXT *(.gnu.warning) - . = ALIGN(16); - *(.got) /* Global offset table */ } - /* - * Make sure that the .got.plt is either completely empty or it - * contains only the lazy dispatch entries. - */ - .got.plt : { *(.got.plt) } - ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, - "Unexpected GOT/PLT entries detected!") - . = ALIGN(SEGMENT_ALIGN); _etext = .; /* End of text section */ @@ -286,6 +276,15 @@ SECTIONS __initdata_end = .; __init_end = .; + .got : { *(.got) } + /* + * Make sure that the .got.plt is either completely empty or it + * contains only the lazy dispatch entries. + */ + .got.plt : { *(.got.plt) } + ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, + "Unexpected GOT/PLT entries detected!") + _data = .; _sdata = .; RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_ALIGN) -- 2.40.1.495.gc816e09b53d-goog
>> + /* >> + * Make sure that the .got.plt is either completely empty or it >> + * contains only the lazy dispatch entries. >> + */ >> + .got.plt : { *(.got.plt) } >> + ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, >> + "Unexpected GOT/PLT entries detected!") >> + >> __initdata_begin = .; >> >> init_idmap_pg_dir = .; >> -- >> 2.40.1.495.gc816e09b53d-goog >>
| |