From: "H.J. Lu" <hjl.tools@gmail.com>
To: Nick Clifton <nickc@redhat.com>, Binutils <binutils@sourceware.org>
Subject: [PATCH v4] ld: Rewrite lang_size_relro_segment_1
Date: Tue, 25 Jan 2022 07:05:25 -0800 [thread overview]
Message-ID: <YfARtdab4sdRAMSo@gmail.com> (raw)
In-Reply-To: <Ye8XY0LeslW4lkAR@gmail.com>
On Mon, Jan 24, 2022 at 01:17:23PM -0800, H.J. Lu wrote:
> On Mon, Jan 24, 2022 at 04:24:33PM +0000, Nick Clifton wrote:
> > Hi H.J.
> >
> > > Here is the v2 patch to track the maximum section alignment from
> > > the RELRO segment. If the maximum page size >= the maximum
> > > section alignment, we can align the RELRO segment to the maximum
> > > page size.
> >
> > I am having some trouble getting my head around this patch, so please
> > bare with me if I ask some silly questions:
> >
> > /* Find the first section in the relro segment. */
> > for (sec = link_info.output_bfd->section_last; sec; sec = sec->prev)
> > if ((sec->flags & SEC_ALLOC) != 0)
> > {
> > if (last_sec_status != relro_sec_before
> > && sec->alignment_power > max_alignment_power)
> > max_alignment_power = sec->alignment_power;
> >
> > if (sec->vma >= seg->base
> > && sec->vma < seg->relro_end - seg->relro_offset)
> > {
> > relro_sec = sec;
> > prev_sec = sec->prev;
> > last_sec_status = relro_sec_in;
> > }
> > else
> > last_sec_status = relro_sec_before;
> > }
> >
> > Questions:
> > Should a zero-sized section (with SHF_ALLOC) be treated as a
> > viable candidate ?
>
> Fixed.
>
> >
> > Once last_sec_status has been set to relro_sec_before, is there
> > any point in continuing the scan ?
>
> Fixed.
>
> >
> > If the section list contains a section which starts beyond the
> > end of the segment (ie sec->vma >= seg->relro_end - seg->relro_offset)
> > then a) is this an error ? and b) should its alignment power be
> > considered against max_alignment_power ?
>
> Since sections are scanned backwards from the highest address and
> sections whose vma >= seg->relro_end - seg->relro_offset are placed
> before the PT_GNU_RELRO segment, these sections are scanned for
> max_alignment_power.
>
> >
> >
> > if (relro_sec)
> > {
> > /* Where do we want to put this section so that it ends as
> > desired? */
> >
> > By "desired" I assume you mean "with the desired alignment, but
> > without any unnecessary padding". Is that correct ?
>
> I changed it to
>
> /* Where do we want to put the relro section so that the
> relro segment ends on the page bounary? */
> bfd_vma start, end, bump;
>
> >
> >
> > /* Find the first non-empty preceding section. */
> > for (; prev_sec; prev_sec = prev_sec->prev)
> > if (prev_sec->size != 0 && (prev_sec->flags & SEC_ALLOC) != 0)
> > break;
> >
> > Given that prev_sec is only needed later on, inside an if-statement,
> > you could move this loop - potentially saving a bit of time in some
> > cases.
> >
>
> Fixed.
>
> >
> > Overall I think that the code could use some more comments, explaining
> > what is going on with some examples of how each part of the algorithm
> > operates.
> >
>
> I added more comments. Here is the v3 patch.
>
> Thanks.
>
Here is the v4 patch to align the PT_GNU_RELRO segment first and subtract
the maximum page size if therer is still a 1-page gap. This fixes:
https://sourceware.org/bugzilla/show_bug.cgi?id=28819
H.J.
---
1. Compute the desired PT_GNU_RELRO segment base and find the maximum
section alignment of sections starting from the PT_GNU_RELRO segment.
2. Find the first preceding load section.
3. Don't add the 1-page gap between the first preceding load section and
the relro segment if the maximum page size >= the maximum section
alignment. Align the PT_GNU_RELRO segment first. Subtract the maximum
page size if therer is still a 1-page gap.
PR ld/28743
PR ld/28819
* ldlang.c (lang_size_relro_segment_1): Rewrite.
* testsuite/ld-x86-64/pr28743-1.d: New file.
* testsuite/ld-x86-64/pr28743-1.s: Likewise.
* testsuite/ld-x86-64/x86-64.exp: Run pr28743-1.
---
ld/ldlang.c | 95 ++++++++++++++++--------------
ld/testsuite/ld-x86-64/pr28743-1.d | 50 ++++++++++++++++
ld/testsuite/ld-x86-64/pr28743-1.s | 16 +++++
ld/testsuite/ld-x86-64/x86-64.exp | 1 +
4 files changed, 118 insertions(+), 44 deletions(-)
create mode 100644 ld/testsuite/ld-x86-64/pr28743-1.d
create mode 100644 ld/testsuite/ld-x86-64/pr28743-1.s
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 93fcfc4cbc7..4e6511ba4ff 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6370,94 +6370,101 @@ lang_size_segment (seg_align_type *seg)
static bfd_vma
lang_size_relro_segment_1 (seg_align_type *seg)
{
- bfd_vma relro_end, desired_end;
- asection *sec, *prev_sec = NULL;
- bool remove_page_gap = false;
+ bfd_vma relro_end, desired_relro_base;
+ asection *sec, *relro_sec = NULL;
unsigned int max_alignment_power = 0;
+ bool seen_reloc_section = false;
+ bool desired_relro_base_reduced = false;
/* Compute the expected PT_GNU_RELRO/PT_LOAD segment end. */
relro_end = ((seg->relro_end + seg->pagesize - 1)
& ~(seg->pagesize - 1));
/* Adjust by the offset arg of XXX_SEGMENT_RELRO_END. */
- desired_end = relro_end - seg->relro_offset;
+ desired_relro_base = relro_end - seg->relro_offset;
- /* For sections in the relro segment.. */
+ /* For sections in the relro segment. */
for (sec = link_info.output_bfd->section_last; sec; sec = sec->prev)
if ((sec->flags & SEC_ALLOC) != 0)
{
+ /* Record the maximum alignment for all sections starting from
+ the relro segment. */
if (sec->alignment_power > max_alignment_power)
max_alignment_power = sec->alignment_power;
if (sec->vma >= seg->base
&& sec->vma < seg->relro_end - seg->relro_offset)
{
- /* Where do we want to put this section so that it ends as
- desired? */
+ /* Where do we want to put the relro section so that the
+ relro segment ends on the page bounary? */
bfd_vma start, end, bump;
end = start = sec->vma;
if (!IS_TBSS (sec))
end += TO_ADDR (sec->size);
- bump = desired_end - end;
+ bump = desired_relro_base - end;
/* We'd like to increase START by BUMP, but we must heed
alignment so the increase might be less than optimum. */
start += bump;
start &= ~(((bfd_vma) 1 << sec->alignment_power) - 1);
/* This is now the desired end for the previous section. */
- desired_end = start;
- prev_sec = sec->prev;
+ desired_relro_base = start;
+ relro_sec = sec;
+ seen_reloc_section = true;
}
- }
-
- seg->phase = exp_seg_relro_adjust;
- ASSERT (desired_end >= seg->base);
-
- for (; prev_sec; prev_sec = prev_sec->prev)
- if ((prev_sec->flags & SEC_ALLOC) != 0)
- {
- if (prev_sec->alignment_power > max_alignment_power)
- max_alignment_power = prev_sec->alignment_power;
-
- if (prev_sec->size != 0)
+ else if (seen_reloc_section)
{
- /* The 1-page gap before the RELRO segment may be removed. */
- remove_page_gap = ((prev_sec->vma + prev_sec->size
- + seg->maxpagesize) < desired_end);
-
+ /* Stop searching if we see a non-relro section after seeing
+ relro sections. */
break;
}
}
- if (remove_page_gap)
+ if (relro_sec != NULL
+ && seg->maxpagesize >= (1U << max_alignment_power))
{
- /* Find the maximum section alignment. */
- for (sec = prev_sec; sec; sec = sec->prev)
- if ((sec->flags & SEC_ALLOC) != 0
- && sec->alignment_power > max_alignment_power)
- max_alignment_power = sec->alignment_power;
+ asection *prev_sec;
+ bfd_vma prev_sec_end_plus_1_page;
- /* Remove the 1-page gap before the RELRO segment only if the
- maximum page size >= the maximum section alignment. */
- if (seg->maxpagesize >= (1U << max_alignment_power))
+ /* Find the first preceding load section. */
+ for (prev_sec = relro_sec->prev;
+ prev_sec != NULL;
+ prev_sec = prev_sec->prev)
+ if ((prev_sec->flags & SEC_ALLOC) != 0)
+ break;
+
+ prev_sec_end_plus_1_page = (prev_sec->vma + prev_sec->size
+ + seg->maxpagesize);
+ if (prev_sec_end_plus_1_page < desired_relro_base)
{
- /* If the preceding section size is greater than the maximum
- page size, subtract the maximum page size. Otherwise,
- align the RELRO segment to the maximum page size. */
- if (prev_sec->size > seg->maxpagesize)
+ bfd_vma aligned_relro_base;
+
+ desired_relro_base_reduced = true;
+
+ /* Don't add the 1-page gap before the relro segment. Align
+ the relro segment first. */
+ aligned_relro_base = (desired_relro_base
+ & ~(seg->maxpagesize - 1));
+ if (prev_sec_end_plus_1_page < aligned_relro_base)
{
- desired_end -= seg->maxpagesize;
+ /* Subtract the maximum page size if therer is still a
+ 1-page gap. */
+ desired_relro_base -= seg->maxpagesize;
relro_end -= seg->maxpagesize;
}
else
{
- desired_end &= ~(seg->maxpagesize - 1);
+ /* Align the relro segment. */
+ desired_relro_base = aligned_relro_base;
relro_end &= ~(seg->maxpagesize - 1);
}
- }
- }
+ }
+ }
- seg->base = desired_end;
+ seg->phase = exp_seg_relro_adjust;
+ ASSERT (desired_relro_base_reduced
+ || desired_relro_base >= seg->base);
+ seg->base = desired_relro_base;
return relro_end;
}
diff --git a/ld/testsuite/ld-x86-64/pr28743-1.d b/ld/testsuite/ld-x86-64/pr28743-1.d
new file mode 100644
index 00000000000..4a20f8e3900
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr28743-1.d
@@ -0,0 +1,50 @@
+#as: --64
+#ld: -melf_x86_64 -shared -z relro -z now --hash-style=sysv -z max-page-size=0x1000 -z noseparate-code $NO_DT_RELR_LDFLAGS
+#readelf: -S -l --wide
+#target: x86_64-*-linux*
+
+There are 17 section headers, starting at offset 0x101228:
+
+Section Headers:
+ \[Nr\] Name Type Address Off Size ES Flg Lk Inf Al
+ \[ 0\] NULL 0000000000000000 000000 000000 00 0 0 0
+ \[ 1\] .hash HASH 0000000000000120 000120 000018 04 A 2 0 8
+ \[ 2\] .dynsym DYNSYM 0000000000000138 000138 000048 18 A 3 1 8
+ \[ 3\] .dynstr STRTAB 0000000000000180 000180 00000a 00 A 0 0 1
+ \[ 4\] .rela.dyn RELA 0000000000000190 000190 000018 18 A 2 0 8
+ \[ 5\] .plt PROGBITS 00000000000001b0 0001b0 000010 10 AX 0 0 16
+ \[ 6\] .plt.got PROGBITS 00000000000001c0 0001c0 000008 08 AX 0 0 8
+ \[ 7\] .text PROGBITS 00000000000001c8 0001c8 00000c 00 AX 0 0 1
+ \[ 8\] .eh_frame PROGBITS 00000000000001d8 0001d8 00006c 00 A 0 0 8
+ \[ 9\] .init_array INIT_ARRAY 00000000000ffff8 0ffff8 000004 08 WA 0 0 8
+ \[10\] .fini_array FINI_ARRAY 0000000000100000 100000 000100 08 WA 0 0 1048576
+ \[11\] .dynamic DYNAMIC 0000000000100100 100100 000150 10 WA 3 0 8
+ \[12\] .got PROGBITS 0000000000100250 100250 000020 08 WA 0 0 8
+ \[13\] .data PROGBITS 0000000000101000 101000 000100 00 WA 0 0 1
+ \[14\] .symtab SYMTAB 0000000000000000 101100 000078 18 15 3 8
+ \[15\] .strtab STRTAB 0000000000000000 101178 000029 00 0 0 1
+ \[16\] .shstrtab STRTAB 0000000000000000 1011a1 000080 00 0 0 1
+Key to Flags:
+ W \(write\), A \(alloc\), X \(execute\), M \(merge\), S \(strings\), I \(info\),
+ L \(link order\), O \(extra OS processing required\), G \(group\), T \(TLS\),
+ C \(compressed\), x \(unknown\), o \(OS specific\), E \(exclude\),
+ D \(mbind\), l \(large\), p \(processor specific\)
+
+Elf file type is DYN \(Shared object file\)
+Entry point 0x0
+There are 4 program headers, starting at offset 64
+
+Program Headers:
+ Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
+ LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x000244 0x000244 R E 0x1000
+ LOAD 0x0ffff8 0x00000000000ffff8 0x00000000000ffff8 0x001108 0x001108 RW 0x100000
+ DYNAMIC 0x100100 0x0000000000100100 0x0000000000100100 0x000150 0x000150 RW 0x8
+ GNU_RELRO 0x0ffff8 0x00000000000ffff8 0x00000000000ffff8 0x001008 0x001008 R 0x1
+
+ Section to Segment mapping:
+ Segment Sections...
+ 00 .hash .dynsym .dynstr .rela.dyn .plt .plt.got .text .eh_frame
+ 01 .init_array .fini_array .dynamic .got .data
+ 02 .dynamic
+ 03 .init_array .fini_array .dynamic .got
+#pass
diff --git a/ld/testsuite/ld-x86-64/pr28743-1.s b/ld/testsuite/ld-x86-64/pr28743-1.s
new file mode 100644
index 00000000000..2e9a1503741
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr28743-1.s
@@ -0,0 +1,16 @@
+ .text
+ .globl foo
+ .type foo, @function
+foo:
+ .cfi_startproc
+ call func@plt
+ movq func@GOTPCREL(%rip), %rax
+ .cfi_endproc
+ .section .init_array,"aw",@init_array
+ .p2align 3
+ .zero 0x4
+ .section .fini_array,"aw",@fini_array
+ .p2align 20
+ .zero 0x100
+ .data
+ .zero 0x100
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index 3bfc5e6e467..5ef66a237ef 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -511,6 +511,7 @@ run_dump_test "dt-relr-1a"
run_dump_test "dt-relr-1a-x32"
run_dump_test "dt-relr-1b"
run_dump_test "dt-relr-1b-x32"
+run_dump_test "pr28743-1"
if ![istarget "x86_64-*-linux*"] {
return
--
2.34.1
next prev parent reply other threads:[~2022-01-25 15:05 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-11 2:12 [PATCH] elf: Remove the 1-page gap before the RELRO segment H.J. Lu
2022-01-11 5:26 ` Fangrui Song
2022-01-13 12:44 ` Nick Clifton
2022-01-13 12:52 ` Alan Modra
2022-01-13 13:19 ` H.J. Lu
2022-01-14 8:12 ` Alan Modra
2022-01-14 9:37 ` Fangrui Song
2022-01-14 14:58 ` H.J. Lu
2022-01-14 21:55 ` [PATCH] ld: Rewrite lang_size_relro_segment_1 H.J. Lu
2022-01-17 4:08 ` Alan Modra
2022-01-18 4:16 ` [PATCH v2] " H.J. Lu
[not found] ` <CAMe9rOpdkYZDigz8r_oPbweLnaCJUjx3-L-v-vp-70c0MGOHQw@mail.gmail.com>
2022-01-24 16:24 ` Fwd: " Nick Clifton
2022-01-24 21:17 ` [PATCH v3] " H.J. Lu
2022-01-25 15:05 ` H.J. Lu [this message]
2022-01-26 10:55 ` [PATCH v4] " Nick Clifton
2022-01-27 0:48 ` Alan Modra
2022-01-27 2:10 ` H.J. Lu
2022-01-29 1:01 ` Alan Modra
2022-01-29 9:06 ` Fangrui Song
2022-01-29 16:45 ` H.J. Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YfARtdab4sdRAMSo@gmail.com \
--to=hjl.tools@gmail.com \
--cc=binutils@sourceware.org \
--cc=nickc@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).