lkml.org 
[lkml]   [2023]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] arm64: lds: move .got section out of .text
On 2023-04-28, Ard Biesheuvel wrote:
>On Fri, 28 Apr 2023 at 19:58, Fangrui Song <maskray@google.com> wrote:
>>
>> 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 .
>>
>
>I don't think that would be the right choice.
>
>We have five pseudo-segments in the kernel
>
>text (RX)
>rodata (R)
>inittext (RX)
>initdata (RW)
>data (RW)
>
>where the init ones disappear entirely when the boot completes.
>
>The GOT should not be modifiable, so it should not be in .data. So the
>only appropriate 'segment' for the GOT is rodata
>
>Note that we don't use PIC codegen for the kernel, so all const
>qualified data structures containing statically initialized global
>pointer variables are emitted into .rodata as well, and relocated at
>boot. So having the GOT in rodata too makes sense imho.

arm64's vmlinux is linked with -shared -Bsymbolic and .got has many
entries that need to be relocated. .got is initially writable, and
becomes read-only after relocation resolving.
I think this property is significant enough to make it outside of any rodata
segment.

(In the userspace .got is typically placed in PT_GNU_RELRO by using -z relro.)

If we remove inittext and initdata pseudo-segments which disappear after
the boot, we have

text (RX)
rodata (R)
/// where is .got (RELRO)
data (RW)

If we consider .got distinct from rodata, I think placing .got immediately
after rodata or immediately before data is both fine?


>
>
>
>> 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
>> >>

\
 
 \ /
  Last update: 2023-04-28 23:06    [W:0.040 / U:0.284 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site