* [PATCH] elf: Remove the 1-page gap before the RELRO segment
@ 2022-01-11 2:12 H.J. Lu
2022-01-11 5:26 ` Fangrui Song
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: H.J. Lu @ 2022-01-11 2:12 UTC (permalink / raw)
To: binutils
The existing RELRO scheme may leave a 1-page gap before the RELRO segment
and align the end of the RELRO segment to the page size:
[18] .eh_frame PROGBITS 408fa0 008fa0 005e80 00 A 0 0 8
[19] .init_array INIT_ARRAY 410de0 00fde0 000008 08 WA 0 0 8
[20] .fini_array FINI_ARRAY 410de8 00fde8 000008 08 WA 0 0 8
[21] .dynamic DYNAMIC 410df0 00fdf0 000200 10 WA 7 0 8
[22] .got PROGBITS 410ff0 00fff0 000010 08 WA 0 0 8
[23] .got.plt PROGBITS 411000 010000 000048 08 WA 0 0 8
Instead, we can remove the 1-page gap if the maximum page size >= the
maximum section alignment:
[18] .eh_frame PROGBITS 408fa0 008fa0 005e80 00 A 0 0 8
[19] .init_array INIT_ARRAY 40fde0 00fde0 000008 08 WA 0 0 8
[20] .fini_array FINI_ARRAY 40fde8 00fde8 000008 08 WA 0 0 8
[21] .dynamic DYNAMIC 40fdf0 00fdf0 000200 10 WA 7 0 8
[22] .got PROGBITS 40fff0 00fff0 000010 08 WA 0 0 8
[23] .got.plt PROGBITS 410000 010000 000048 08 WA 0 0 8
Because the end of the RELRO segment is always aligned to the page size
and may not be moved, the RELRO segment size may be increased:
[ 3] .dynstr STRTAB 000148 000148 000001 00 A 0 0 1
[ 4] .eh_frame PROGBITS 000150 000150 000000 00 A 0 0 8
[ 5] .init_array INIT_ARRAY 200150 000150 000010 08 WA 0 0 1
[ 6] .fini_array FINI_ARRAY 200160 000160 000010 08 WA 0 0 1
[ 7] .jcr PROGBITS 200170 000170 000008 00 WA 0 0 1
[ 8] .data.rel.ro PROGBITS 200180 000180 000020 00 WA 0 0 16
[ 9] .dynamic DYNAMIC 2001a0 0001a0 0001c0 10 WA 3 0 8
[10] .got PROGBITS 200360 000360 0002a8 00 WA 0 0 8
[11] .bss NOBITS 201000 000608 000840 00 WA 0 0 1
vs the old section layout:
[ 3] .dynstr STRTAB 000148 000148 000001 00 A 0 0 1
[ 4] .eh_frame PROGBITS 000150 000150 000000 00 A 0 0 8
[ 5] .init_array INIT_ARRAY 200b48 000b48 000010 08 WA 0 0 1
[ 6] .fini_array FINI_ARRAY 200b58 000b58 000010 08 WA 0 0 1
[ 7] .jcr PROGBITS 200b68 000b68 000008 00 WA 0 0 1
[ 8] .data.rel.ro PROGBITS 200b70 000b70 000020 00 WA 0 0 16
[ 9] .dynamic DYNAMIC 200b90 000b90 0001c0 10 WA 3 0 8
[10] .got PROGBITS 200d50 000d50 0002a8 00 WA 0 0 8
[11] .bss NOBITS 201000 000ff8 000840 00 WA 0 0 1
But there is no 1-page gap.
PR ld/28743
* ldlang.c (lang_size_relro_segment_1): Remove the 1-page gap
before the RELRO segment if the maximum page size >= the maximum
section alignment.
* testsuite/ld-i386/pr20830.d: Adjusted.
* testsuite/ld-s390/gotreloc_64-relro-1.dd: Likewise.
* testsuite/ld-x86-64/pr14207.d: Likewise.
* testsuite/ld-x86-64/pr18176.d: Likewise.
* testsuite/ld-x86-64/pr20830a-now.d: Likewise.
* testsuite/ld-x86-64/pr20830a.d: Likewise.
* testsuite/ld-x86-64/pr20830b-now.d: Likewise.
* testsuite/ld-x86-64/pr20830b.d: Likewise.
* testsuite/ld-x86-64/pr21038a-now.d: Likewise.
* testsuite/ld-x86-64/pr21038a.d: Likewise.
* testsuite/ld-x86-64/pr21038b-now.d: Likewise.
* testsuite/ld-x86-64/pr21038c-now.d: Likewise.
* testsuite/ld-x86-64/pr21038c.d: Likewise.
---
ld/ldlang.c | 89 ++++++++++++++++-----
ld/testsuite/ld-i386/pr20830.d | 4 +-
ld/testsuite/ld-s390/gotreloc_64-relro-1.dd | 6 +-
ld/testsuite/ld-x86-64/pr14207.d | 6 +-
ld/testsuite/ld-x86-64/pr18176.d | 2 +-
ld/testsuite/ld-x86-64/pr20830a-now.d | 8 +-
ld/testsuite/ld-x86-64/pr20830a.d | 4 +-
ld/testsuite/ld-x86-64/pr20830b-now.d | 10 +--
ld/testsuite/ld-x86-64/pr20830b.d | 6 +-
ld/testsuite/ld-x86-64/pr21038a-now.d | 8 +-
ld/testsuite/ld-x86-64/pr21038a.d | 4 +-
ld/testsuite/ld-x86-64/pr21038b-now.d | 6 +-
ld/testsuite/ld-x86-64/pr21038c-now.d | 10 +--
ld/testsuite/ld-x86-64/pr21038c.d | 4 +-
14 files changed, 110 insertions(+), 57 deletions(-)
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 9dbc8752f87..4a1dc19e100 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6361,7 +6361,9 @@ static bfd_vma
lang_size_relro_segment_1 (seg_align_type *seg)
{
bfd_vma relro_end, desired_end;
- asection *sec;
+ asection *sec, *prev_sec = NULL;
+ bool remove_page_gap = false;
+ unsigned int max_alignment_power = 0;
/* Compute the expected PT_GNU_RELRO/PT_LOAD segment end. */
relro_end = ((seg->relro_end + seg->pagesize - 1)
@@ -6372,28 +6374,79 @@ lang_size_relro_segment_1 (seg_align_type *seg)
/* For sections in the relro segment.. */
for (sec = link_info.output_bfd->section_last; sec; sec = sec->prev)
- if ((sec->flags & SEC_ALLOC) != 0
- && sec->vma >= seg->base
- && sec->vma < seg->relro_end - seg->relro_offset)
+ if ((sec->flags & SEC_ALLOC) != 0)
{
- /* Where do we want to put this section so that it ends as
- desired? */
- bfd_vma start, end, bump;
-
- end = start = sec->vma;
- if (!IS_TBSS (sec))
- end += TO_ADDR (sec->size);
- bump = desired_end - 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;
+ 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? */
+ bfd_vma start, end, bump;
+
+ end = start = sec->vma;
+ if (!IS_TBSS (sec))
+ end += TO_ADDR (sec->size);
+ bump = desired_end - 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;
+ }
}
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)
+ {
+ /* The 1-page gap before the RELRO segment may be removed. */
+ remove_page_gap = ((prev_sec->vma + prev_sec->size
+ + seg->maxpagesize) < desired_end);
+
+ break;
+ }
+ }
+
+ if (remove_page_gap)
+ {
+ /* 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;
+
+ /* 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))
+ {
+ /* 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)
+ {
+ desired_end -= seg->maxpagesize;
+ relro_end -= seg->maxpagesize;
+ }
+ else
+ {
+ desired_end &= ~(seg->maxpagesize - 1);
+ relro_end &= ~(seg->maxpagesize - 1);
+ }
+ }
+ }
+
seg->base = desired_end;
return relro_end;
}
diff --git a/ld/testsuite/ld-i386/pr20830.d b/ld/testsuite/ld-i386/pr20830.d
index 8a14a6087a1..f1e37336733 100644
--- a/ld/testsuite/ld-i386/pr20830.d
+++ b/ld/testsuite/ld-i386/pr20830.d
@@ -49,12 +49,12 @@ Disassembly of section .plt:
Disassembly of section .plt.got:
0+120 <func@plt>:
- +[a-f0-9]+: ff a3 fc ff ff ff jmp \*-0x4\(%ebx\)
+ +[a-f0-9]+: ff a3 78 f0 ff ff jmp \*-0xf88\(%ebx\)
+[a-f0-9]+: 66 90 xchg %ax,%ax
Disassembly of section .text:
0+128 <foo>:
+[a-f0-9]+: e8 f3 ff ff ff call 120 <func@plt>
- +[a-f0-9]+: 8b 83 fc ff ff ff mov -0x4\(%ebx\),%eax
+ +[a-f0-9]+: 8b 83 78 f0 ff ff mov -0xf88\(%ebx\),%eax
#pass
diff --git a/ld/testsuite/ld-s390/gotreloc_64-relro-1.dd b/ld/testsuite/ld-s390/gotreloc_64-relro-1.dd
index 64151d10a7c..5a107465be2 100644
--- a/ld/testsuite/ld-s390/gotreloc_64-relro-1.dd
+++ b/ld/testsuite/ld-s390/gotreloc_64-relro-1.dd
@@ -5,8 +5,8 @@ Disassembly of section .text:
.* <foo>:
.*: c0 10 00 00 0f 0c [ ]*larl %r1,2000 <bar>
.*: c0 10 00 00 0f 09 [ ]*larl %r1,2000 <bar>
-.*: c4 1d 00 00 0f 02 [ ]*lrl %r1,1ff8 <_GLOBAL_OFFSET_TABLE_\+0x28>
+.*: c4 1d 00 00 07 8a [ ]*lrl %r1,1108 <_GLOBAL_OFFSET_TABLE_\+0x28>
.*: 58 10 c0 28 [ ]*l %r1,40\(%r12\)
.*: e3 10 c0 28 00 58 [ ]*ly %r1,40\(%r12\)
-.*: c4 18 00 00 0e f6 [ ]*lgrl %r1,1ff0 <_GLOBAL_OFFSET_TABLE_\+0x20>
-.*: c4 18 00 00 0e ef [ ]*lgrl %r1,1fe8 <_GLOBAL_OFFSET_TABLE_\+0x18>
+.*: c4 18 00 00 07 7e [ ]*lgrl %r1,1100 <_GLOBAL_OFFSET_TABLE_\+0x20>
+.*: c4 18 00 00 07 77 [ ]*lgrl %r1,10f8 <_GLOBAL_OFFSET_TABLE_\+0x18>
diff --git a/ld/testsuite/ld-x86-64/pr14207.d b/ld/testsuite/ld-x86-64/pr14207.d
index f330600b916..c06755f96d9 100644
--- a/ld/testsuite/ld-x86-64/pr14207.d
+++ b/ld/testsuite/ld-x86-64/pr14207.d
@@ -11,9 +11,9 @@ There are 4 program headers, starting at offset 64
Program Headers:
Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x000150 0x000150 R 0x200000
- LOAD 0x000b.8 0x0000000000200b.8 0x0000000000200b.8 0x0004.0 0x000c.8 RW 0x200000
- DYNAMIC 0x000b.0 0x0000000000200b.0 0x0000000000200b.0 0x0001.0 0x0001.0 RW 0x8
- GNU_RELRO 0x000b.8 0x0000000000200b.8 0x0000000000200b.8 0x0004.0 0x0004.8 R 0x1
+ LOAD 0x000150 0x0000000000200150 0x0000000000200150 0x0004b8 0x0016f0 RW 0x200000
+ DYNAMIC 0x0001a0 0x00000000002001a0 0x00000000002001a0 0x0001c0 0x0001c0 RW 0x8
+ GNU_RELRO 0x000150 0x0000000000200150 0x0000000000200150 0x0004b8 0x000eb0 R 0x1
Section to Segment mapping:
Segment Sections...
diff --git a/ld/testsuite/ld-x86-64/pr18176.d b/ld/testsuite/ld-x86-64/pr18176.d
index 4e3ad9ff08d..3ff34fad6cb 100644
--- a/ld/testsuite/ld-x86-64/pr18176.d
+++ b/ld/testsuite/ld-x86-64/pr18176.d
@@ -5,5 +5,5 @@
#target: x86_64-*-linux*
#...
- GNU_RELRO 0x04bd17 0x000000000024bd17 0x000000000024bd17 0x0022e9 0x0022e9 R 0x1
+ GNU_RELRO 0x04bcc7 0x000000000024bcc7 0x000000000024bcc7 0x002339 0x002339 R 0x1
#pass
diff --git a/ld/testsuite/ld-x86-64/pr20830a-now.d b/ld/testsuite/ld-x86-64/pr20830a-now.d
index 4f284c44a40..b8fa6acd1d4 100644
--- a/ld/testsuite/ld-x86-64/pr20830a-now.d
+++ b/ld/testsuite/ld-x86-64/pr20830a-now.d
@@ -50,19 +50,19 @@ Contents of the .eh_frame section:
Disassembly of section .plt:
0+1b0 <.plt>:
- +[a-f0-9]+: ff 35 32 0e 20 00 push 0x200e32\(%rip\) # 200fe8 <_GLOBAL_OFFSET_TABLE_\+0x8>
- +[a-f0-9]+: ff 25 34 0e 20 00 jmp \*0x200e34\(%rip\) # 200ff0 <_GLOBAL_OFFSET_TABLE_\+0x10>
+ +[a-f0-9]+: ff 35 aa 01 20 00 push 0x2001aa\(%rip\) # 200360 <_GLOBAL_OFFSET_TABLE_\+0x8>
+ +[a-f0-9]+: ff 25 ac 01 20 00 jmp \*0x2001ac\(%rip\) # 200368 <_GLOBAL_OFFSET_TABLE_\+0x10>
+[a-f0-9]+: 0f 1f 40 00 nopl 0x0\(%rax\)
Disassembly of section .plt.got:
0+1c0 <func@plt>:
- +[a-f0-9]+: ff 25 32 0e 20 00 jmp \*0x200e32\(%rip\) # 200ff8 <func>
+ +[a-f0-9]+: ff 25 aa 01 20 00 jmp \*0x2001aa\(%rip\) # 200370 <func>
+[a-f0-9]+: 66 90 xchg %ax,%ax
Disassembly of section .text:
0+1c8 <foo>:
+[a-f0-9]+: e8 f3 ff ff ff call 1c0 <func@plt>
- +[a-f0-9]+: 48 8b 05 24 0e 20 00 mov 0x200e24\(%rip\),%rax # 200ff8 <func>
+ +[a-f0-9]+: 48 8b 05 9c 01 20 00 mov 0x20019c\(%rip\),%rax # 200370 <func>
#pass
diff --git a/ld/testsuite/ld-x86-64/pr20830a.d b/ld/testsuite/ld-x86-64/pr20830a.d
index 615b59fd5de..5c16652c4a8 100644
--- a/ld/testsuite/ld-x86-64/pr20830a.d
+++ b/ld/testsuite/ld-x86-64/pr20830a.d
@@ -57,12 +57,12 @@ Disassembly of section .plt:
Disassembly of section .plt.got:
0+1c0 <func@plt>:
- +[a-f0-9]+: ff 25 32 0e 20 00 jmp \*0x200e32\(%rip\) # 200ff8 <func>
+ +[a-f0-9]+: ff 25 72 01 20 00 jmp \*0x200172\(%rip\) # 200338 <func>
+[a-f0-9]+: 66 90 xchg %ax,%ax
Disassembly of section .text:
0+1c8 <foo>:
+[a-f0-9]+: e8 f3 ff ff ff call 1c0 <func@plt>
- +[a-f0-9]+: 48 8b 05 24 0e 20 00 mov 0x200e24\(%rip\),%rax # 200ff8 <func>
+ +[a-f0-9]+: 48 8b 05 64 01 20 00 mov 0x200164\(%rip\),%rax # 200338 <func>
#pass
diff --git a/ld/testsuite/ld-x86-64/pr20830b-now.d b/ld/testsuite/ld-x86-64/pr20830b-now.d
index 7c7f2f928ac..0c4f2ada3dd 100644
--- a/ld/testsuite/ld-x86-64/pr20830b-now.d
+++ b/ld/testsuite/ld-x86-64/pr20830b-now.d
@@ -1,4 +1,4 @@
-#name: PR ld/20830 (.plt.got, -z now)
+#name: PR ld/20830 x32 (.plt.got, -z now)
#source: pr20830.s
#as: --x32
#ld: -z now -melf32_x86_64 -shared -z relro --ld-generated-unwind-info --hash-style=sysv -z max-page-size=0x200000 -z noseparate-code
@@ -42,19 +42,19 @@ Contents of the .eh_frame section:
Disassembly of section .plt:
0+120 <.plt>:
- +[a-f0-9]+: ff 35 c2 0e 20 00 push 0x200ec2\(%rip\) # 200fe8 <_GLOBAL_OFFSET_TABLE_\+0x8>
- +[a-f0-9]+: ff 25 c4 0e 20 00 jmp \*0x200ec4\(%rip\) # 200ff0 <_GLOBAL_OFFSET_TABLE_\+0x10>
+ +[a-f0-9]+: ff 35 12 01 20 00 push 0x200112\(%rip\) # 200238 <_GLOBAL_OFFSET_TABLE_\+0x8>
+ +[a-f0-9]+: ff 25 14 01 20 00 jmp \*0x200114\(%rip\) # 200240 <_GLOBAL_OFFSET_TABLE_\+0x10>
+[a-f0-9]+: 0f 1f 40 00 nopl 0x0\(%rax\)
Disassembly of section .plt.got:
0+130 <func@plt>:
- +[a-f0-9]+: ff 25 c2 0e 20 00 jmp \*0x200ec2\(%rip\) # 200ff8 <func>
+ +[a-f0-9]+: ff 25 12 01 20 00 jmp \*0x200112\(%rip\) # 200248 <func>
+[a-f0-9]+: 66 90 xchg %ax,%ax
Disassembly of section .text:
0+138 <foo>:
+[a-f0-9]+: e8 f3 ff ff ff call 130 <func@plt>
- +[a-f0-9]+: 48 8b 05 b4 0e 20 00 mov 0x200eb4\(%rip\),%rax # 200ff8 <func>
+ +[a-f0-9]+: 48 8b 05 04 01 20 00 mov 0x200104\(%rip\),%rax # 200248 <func>
#pass
diff --git a/ld/testsuite/ld-x86-64/pr20830b.d b/ld/testsuite/ld-x86-64/pr20830b.d
index 3c5d42b9c43..6f4f22ca960 100644
--- a/ld/testsuite/ld-x86-64/pr20830b.d
+++ b/ld/testsuite/ld-x86-64/pr20830b.d
@@ -1,4 +1,4 @@
-#name: PR ld/20830 (.plt.got)
+#name: PR ld/20830 x32 (.plt.got)
#source: pr20830.s
#as: --x32
#ld: -melf32_x86_64 -shared -z relro --ld-generated-unwind-info --hash-style=sysv -z max-page-size=0x200000 -z noseparate-code
@@ -49,12 +49,12 @@ Disassembly of section .plt:
Disassembly of section .plt.got:
0+130 <func@plt>:
- +[a-f0-9]+: ff 25 c2 0e 20 00 jmp \*0x200ec2\(%rip\) # 200ff8 <func>
+ +[a-f0-9]+: ff 25 ea 00 20 00 jmp \*0x2000ea\(%rip\) # 200220 <func>
+[a-f0-9]+: 66 90 xchg %ax,%ax
Disassembly of section .text:
0+138 <foo>:
+[a-f0-9]+: e8 f3 ff ff ff call 130 <func@plt>
- +[a-f0-9]+: 48 8b 05 b4 0e 20 00 mov 0x200eb4\(%rip\),%rax # 200ff8 <func>
+ +[a-f0-9]+: 48 8b 05 dc 00 20 00 mov 0x2000dc\(%rip\),%rax # 200220 <func>
#pass
diff --git a/ld/testsuite/ld-x86-64/pr21038a-now.d b/ld/testsuite/ld-x86-64/pr21038a-now.d
index 1653a68ff8a..c05800962be 100644
--- a/ld/testsuite/ld-x86-64/pr21038a-now.d
+++ b/ld/testsuite/ld-x86-64/pr21038a-now.d
@@ -50,19 +50,19 @@ Contents of the .eh_frame section:
Disassembly of section .plt:
0+1b0 <.plt>:
- +[a-f0-9]+: ff 35 32 0e 20 00 push 0x200e32\(%rip\) # 200fe8 <_GLOBAL_OFFSET_TABLE_\+0x8>
- +[a-f0-9]+: f2 ff 25 33 0e 20 00 bnd jmp \*0x200e33\(%rip\) # 200ff0 <_GLOBAL_OFFSET_TABLE_\+0x10>
+ +[a-f0-9]+: ff 35 aa 01 20 00 push 0x2001aa\(%rip\) # 200360 <_GLOBAL_OFFSET_TABLE_\+0x8>
+ +[a-f0-9]+: f2 ff 25 ab 01 20 00 bnd jmp \*0x2001ab\(%rip\) # 200368 <_GLOBAL_OFFSET_TABLE_\+0x10>
+[a-f0-9]+: 0f 1f 00 nopl \(%rax\)
Disassembly of section .plt.got:
0+1c0 <func@plt>:
- +[a-f0-9]+: f2 ff 25 31 0e 20 00 bnd jmp \*0x200e31\(%rip\) # 200ff8 <func>
+ +[a-f0-9]+: f2 ff 25 a9 01 20 00 bnd jmp \*0x2001a9\(%rip\) # 200370 <func>
+[a-f0-9]+: 90 nop
Disassembly of section .text:
0+1c8 <foo>:
+[a-f0-9]+: e8 f3 ff ff ff call 1c0 <func@plt>
- +[a-f0-9]+: 48 8b 05 24 0e 20 00 mov 0x200e24\(%rip\),%rax # 200ff8 <func>
+ +[a-f0-9]+: 48 8b 05 9c 01 20 00 mov 0x20019c\(%rip\),%rax # 200370 <func>
#pass
diff --git a/ld/testsuite/ld-x86-64/pr21038a.d b/ld/testsuite/ld-x86-64/pr21038a.d
index 6ef8db254e9..e0c3c7cf63f 100644
--- a/ld/testsuite/ld-x86-64/pr21038a.d
+++ b/ld/testsuite/ld-x86-64/pr21038a.d
@@ -56,12 +56,12 @@ Disassembly of section .plt:
Disassembly of section .plt.got:
0+1c0 <func@plt>:
- +[a-f0-9]+: f2 ff 25 31 0e 20 00 bnd jmp \*0x200e31\(%rip\) # 200ff8 <func>
+ +[a-f0-9]+: f2 ff 25 71 01 20 00 bnd jmp \*0x200171\(%rip\) # 200338 <func>
+[a-f0-9]+: 90 nop
Disassembly of section .text:
0+1c8 <foo>:
+[a-f0-9]+: e8 f3 ff ff ff call 1c0 <func@plt>
- +[a-f0-9]+: 48 8b 05 24 0e 20 00 mov 0x200e24\(%rip\),%rax # 200ff8 <func>
+ +[a-f0-9]+: 48 8b 05 64 01 20 00 mov 0x200164\(%rip\),%rax # 200338 <func>
#pass
diff --git a/ld/testsuite/ld-x86-64/pr21038b-now.d b/ld/testsuite/ld-x86-64/pr21038b-now.d
index c042b6cf702..519c5a3e957 100644
--- a/ld/testsuite/ld-x86-64/pr21038b-now.d
+++ b/ld/testsuite/ld-x86-64/pr21038b-now.d
@@ -50,8 +50,8 @@ Contents of the .eh_frame section:
Disassembly of section .plt:
0+1b0 <.plt>:
- +[a-f0-9]+: ff 35 32 0e 20 00 push 0x200e32\(%rip\) # 200fe8 <_GLOBAL_OFFSET_TABLE_\+0x8>
- +[a-f0-9]+: f2 ff 25 33 0e 20 00 bnd jmp \*0x200e33\(%rip\) # 200ff0 <_GLOBAL_OFFSET_TABLE_\+0x10>
+ +[a-f0-9]+: ff 35 b2 01 20 00 push 0x2001b2\(%rip\) # 200368 <_GLOBAL_OFFSET_TABLE_\+0x8>
+ +[a-f0-9]+: f2 ff 25 b3 01 20 00 bnd jmp \*0x2001b3\(%rip\) # 200370 <_GLOBAL_OFFSET_TABLE_\+0x10>
+[a-f0-9]+: 0f 1f 00 nopl \(%rax\)
+[a-f0-9]+: 68 00 00 00 00 push \$0x0
+[a-f0-9]+: f2 e9 e5 ff ff ff bnd jmp 1b0 <func@plt-0x20>
@@ -60,7 +60,7 @@ Disassembly of section .plt:
Disassembly of section .plt.sec:
0+1d0 <func@plt>:
- +[a-f0-9]+: f2 ff 25 21 0e 20 00 bnd jmp \*0x200e21\(%rip\) # 200ff8 <func>
+ +[a-f0-9]+: f2 ff 25 a1 01 20 00 bnd jmp \*0x2001a1\(%rip\) # 200378 <func>
+[a-f0-9]+: 90 nop
Disassembly of section .text:
diff --git a/ld/testsuite/ld-x86-64/pr21038c-now.d b/ld/testsuite/ld-x86-64/pr21038c-now.d
index 2058512b74e..6c947ea4c13 100644
--- a/ld/testsuite/ld-x86-64/pr21038c-now.d
+++ b/ld/testsuite/ld-x86-64/pr21038c-now.d
@@ -59,8 +59,8 @@ Contents of the .eh_frame section:
Disassembly of section .plt:
0+1f0 <.plt>:
- +[a-f0-9]+: ff 35 ea 0d 20 00 push 0x200dea\(%rip\) # 200fe0 <_GLOBAL_OFFSET_TABLE_\+0x8>
- +[a-f0-9]+: f2 ff 25 eb 0d 20 00 bnd jmp \*0x200deb\(%rip\) # 200fe8 <_GLOBAL_OFFSET_TABLE_\+0x10>
+ +[a-f0-9]+: ff 35 12 02 20 00 push 0x200212\(%rip\) # 200408 <_GLOBAL_OFFSET_TABLE_\+0x8>
+ +[a-f0-9]+: f2 ff 25 13 02 20 00 bnd jmp \*0x200213\(%rip\) # 200410 <_GLOBAL_OFFSET_TABLE_\+0x10>
+[a-f0-9]+: 0f 1f 00 nopl \(%rax\)
+[a-f0-9]+: 68 00 00 00 00 push \$0x0
+[a-f0-9]+: f2 e9 e5 ff ff ff bnd jmp 1f0 <func1@plt-0x20>
@@ -69,13 +69,13 @@ Disassembly of section .plt:
Disassembly of section .plt.got:
0+210 <func1@plt>:
- +[a-f0-9]+: f2 ff 25 e1 0d 20 00 bnd jmp \*0x200de1\(%rip\) # 200ff8 <func1>
+ +[a-f0-9]+: f2 ff 25 09 02 20 00 bnd jmp \*0x200209\(%rip\) # 200420 <func1>
+[a-f0-9]+: 90 nop
Disassembly of section .plt.sec:
0+218 <func2@plt>:
- +[a-f0-9]+: f2 ff 25 d1 0d 20 00 bnd jmp \*0x200dd1\(%rip\) # 200ff0 <func2>
+ +[a-f0-9]+: f2 ff 25 f9 01 20 00 bnd jmp \*0x2001f9\(%rip\) # 200418 <func2>
+[a-f0-9]+: 90 nop
Disassembly of section .text:
@@ -83,5 +83,5 @@ Disassembly of section .text:
0+220 <foo>:
+[a-f0-9]+: e8 eb ff ff ff call 210 <func1@plt>
+[a-f0-9]+: e8 ee ff ff ff call 218 <func2@plt>
- +[a-f0-9]+: 48 8b 05 c7 0d 20 00 mov 0x200dc7\(%rip\),%rax # 200ff8 <func1>
+ +[a-f0-9]+: 48 8b 05 ef 01 20 00 mov 0x2001ef\(%rip\),%rax # 200420 <func1>
#pass
diff --git a/ld/testsuite/ld-x86-64/pr21038c.d b/ld/testsuite/ld-x86-64/pr21038c.d
index a62d43a7bc0..40ecc97e517 100644
--- a/ld/testsuite/ld-x86-64/pr21038c.d
+++ b/ld/testsuite/ld-x86-64/pr21038c.d
@@ -68,7 +68,7 @@ Disassembly of section .plt:
Disassembly of section .plt.got:
0+210 <func1@plt>:
- +[a-f0-9]+: f2 ff 25 e1 0d 20 00 bnd jmp \*0x200de1\(%rip\) # 200ff8 <func1>
+ +[a-f0-9]+: f2 ff 25 c9 01 20 00 bnd jmp \*0x2001c9\(%rip\) # 2003e0 <func1>
+[a-f0-9]+: 90 nop
Disassembly of section .plt.sec:
@@ -82,5 +82,5 @@ Disassembly of section .text:
0+220 <foo>:
+[a-f0-9]+: e8 eb ff ff ff call 210 <func1@plt>
+[a-f0-9]+: e8 ee ff ff ff call 218 <func2@plt>
- +[a-f0-9]+: 48 8b 05 c7 0d 20 00 mov 0x200dc7\(%rip\),%rax # 200ff8 <func1>
+ +[a-f0-9]+: 48 8b 05 af 01 20 00 mov 0x2001af\(%rip\),%rax # 2003e0 <func1>
#pass
--
2.34.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] elf: Remove the 1-page gap before the RELRO segment
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
2 siblings, 0 replies; 20+ messages in thread
From: Fangrui Song @ 2022-01-11 5:26 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
On 2022-01-10, H.J. Lu via Binutils wrote:
>The existing RELRO scheme may leave a 1-page gap before the RELRO segment
>and align the end of the RELRO segment to the page size:
>
> [18] .eh_frame PROGBITS 408fa0 008fa0 005e80 00 A 0 0 8
> [19] .init_array INIT_ARRAY 410de0 00fde0 000008 08 WA 0 0 8
> [20] .fini_array FINI_ARRAY 410de8 00fde8 000008 08 WA 0 0 8
> [21] .dynamic DYNAMIC 410df0 00fdf0 000200 10 WA 7 0 8
> [22] .got PROGBITS 410ff0 00fff0 000010 08 WA 0 0 8
> [23] .got.plt PROGBITS 411000 010000 000048 08 WA 0 0 8
>
>Instead, we can remove the 1-page gap if the maximum page size >= the
>maximum section alignment:
>
> [18] .eh_frame PROGBITS 408fa0 008fa0 005e80 00 A 0 0 8
> [19] .init_array INIT_ARRAY 40fde0 00fde0 000008 08 WA 0 0 8
> [20] .fini_array FINI_ARRAY 40fde8 00fde8 000008 08 WA 0 0 8
> [21] .dynamic DYNAMIC 40fdf0 00fdf0 000200 10 WA 7 0 8
> [22] .got PROGBITS 40fff0 00fff0 000010 08 WA 0 0 8
> [23] .got.plt PROGBITS 410000 010000 000048 08 WA 0 0 8
>
>Because the end of the RELRO segment is always aligned to the page size
>and may not be moved, the RELRO segment size may be increased:
>
> [ 3] .dynstr STRTAB 000148 000148 000001 00 A 0 0 1
> [ 4] .eh_frame PROGBITS 000150 000150 000000 00 A 0 0 8
> [ 5] .init_array INIT_ARRAY 200150 000150 000010 08 WA 0 0 1
> [ 6] .fini_array FINI_ARRAY 200160 000160 000010 08 WA 0 0 1
> [ 7] .jcr PROGBITS 200170 000170 000008 00 WA 0 0 1
> [ 8] .data.rel.ro PROGBITS 200180 000180 000020 00 WA 0 0 16
> [ 9] .dynamic DYNAMIC 2001a0 0001a0 0001c0 10 WA 3 0 8
> [10] .got PROGBITS 200360 000360 0002a8 00 WA 0 0 8
> [11] .bss NOBITS 201000 000608 000840 00 WA 0 0 1
>
>vs the old section layout:
>
> [ 3] .dynstr STRTAB 000148 000148 000001 00 A 0 0 1
> [ 4] .eh_frame PROGBITS 000150 000150 000000 00 A 0 0 8
> [ 5] .init_array INIT_ARRAY 200b48 000b48 000010 08 WA 0 0 1
> [ 6] .fini_array FINI_ARRAY 200b58 000b58 000010 08 WA 0 0 1
> [ 7] .jcr PROGBITS 200b68 000b68 000008 00 WA 0 0 1
> [ 8] .data.rel.ro PROGBITS 200b70 000b70 000020 00 WA 0 0 16
> [ 9] .dynamic DYNAMIC 200b90 000b90 0001c0 10 WA 3 0 8
> [10] .got PROGBITS 200d50 000d50 0002a8 00 WA 0 0 8
> [11] .bss NOBITS 201000 000ff8 000840 00 WA 0 0 1
>
>But there is no 1-page gap.
> [...}
If you want to avoid a max-page-size alignment (at the end of PT_GNU_RELRO), you may adopt ld.lld's design
I implemented:
LOAD 0x000000 0x0000000000200000 0x0000000000200000 0xcaac4c 0xcaac4c R 0x1000
LOAD 0xcaac50 0x0000000000eabc50 0x0000000000eabc50 0x208a0c0 0x208a0c0 R E 0x1000
LOAD 0x2d34d10 0x0000000002f36d10 0x0000000002f36d10 0x1777e8 0x1777e8 RW 0x1000 match
LOAD 0x2eac500 0x00000000030af500 0x00000000030af500 0x008038 0x064664 RW 0x1000
TLS 0x2d34d10 0x0000000002f35d10 0x0000000002f35d10 0x000000 0x000018 R 0x8
DYNAMIC 0x2ea5570 0x00000000030a7570 0x00000000030a7570 0x000240 0x000240 RW 0x8
GNU_RELRO 0x2d34d10 0x0000000002f36d10 0x0000000002f36d10 0x1777e8 0x1782f0 R 0x1 match
GNU_EH_FRAME 0x89ce34 0x0000000000a9ce34 0x0000000000a9ce34 0x08586c 0x08586c R 0x4
GNU_STACK 0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW 0
NOTE 0x0002fc 0x00000000002002fc 0x00000000002002fc 0x000020 0x000020 R 0x4
The idea is to have 2 RW PT_LOAD with one exactly matching PT_GNU_RELRO.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] elf: Remove the 1-page gap before the RELRO segment
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
2 siblings, 0 replies; 20+ messages in thread
From: Nick Clifton @ 2022-01-13 12:44 UTC (permalink / raw)
To: H.J. Lu, binutils
Hi H.J.
> PR ld/28743
> * ldlang.c (lang_size_relro_segment_1): Remove the 1-page gap
> before the RELRO segment if the maximum page size >= the maximum
> section alignment.
> * testsuite/ld-i386/pr20830.d: Adjusted.
> * testsuite/ld-s390/gotreloc_64-relro-1.dd: Likewise.
> * testsuite/ld-x86-64/pr14207.d: Likewise.
> * testsuite/ld-x86-64/pr18176.d: Likewise.
> * testsuite/ld-x86-64/pr20830a-now.d: Likewise.
> * testsuite/ld-x86-64/pr20830a.d: Likewise.
> * testsuite/ld-x86-64/pr20830b-now.d: Likewise.
> * testsuite/ld-x86-64/pr20830b.d: Likewise.
> * testsuite/ld-x86-64/pr21038a-now.d: Likewise.
> * testsuite/ld-x86-64/pr21038a.d: Likewise.
> * testsuite/ld-x86-64/pr21038b-now.d: Likewise.
> * testsuite/ld-x86-64/pr21038c-now.d: Likewise.
> * testsuite/ld-x86-64/pr21038c.d: Likewise.
Approved - please apply.
Cheers
Nick
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] elf: Remove the 1-page gap before the RELRO segment
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
2 siblings, 1 reply; 20+ messages in thread
From: Alan Modra @ 2022-01-13 12:52 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
On Mon, Jan 10, 2022 at 06:12:41PM -0800, H.J. Lu via Binutils wrote:
> The existing RELRO scheme may leave a 1-page gap before the RELRO segment
> and align the end of the RELRO segment to the page size:
>
> [18] .eh_frame PROGBITS 408fa0 008fa0 005e80 00 A 0 0 8
> [19] .init_array INIT_ARRAY 410de0 00fde0 000008 08 WA 0 0 8
> [20] .fini_array FINI_ARRAY 410de8 00fde8 000008 08 WA 0 0 8
> [21] .dynamic DYNAMIC 410df0 00fdf0 000200 10 WA 7 0 8
> [22] .got PROGBITS 410ff0 00fff0 000010 08 WA 0 0 8
> [23] .got.plt PROGBITS 411000 010000 000048 08 WA 0 0 8
Do you know what is going wrong with the relro section layout for this
to occur?
In this particular case, the end of the read-only segment is at
0x408fa0 + 0x5e80 = 0x40ee20. My guess is that layout of the
following rw sections starts on the next page plus current offset
within page, the standard choice to minimise disk pages. ie. We start
at 0x40fe20. Then discover that this puts .got.plt at 0x40fe20 + 8 +
8 + 0x200 + 0x10 = 0x40f040. However, we want this to be on a page
boundary so that the relro segment ends on a page boundary. So we
bump 0x40f040 up to 0x411000 and calculate backwards from there to
arrive at .init_array with a vma of 0x410de0. Resulting in the
0x40f000 page being unused.
If instead we start relro layout on the next page, we'd start laying
out at 0x40f000 rather than 0x40fe20. I think that would be the
correct thing to do rather than fixing up afterwards as your patch
does.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] elf: Remove the 1-page gap before the RELRO segment
2022-01-13 12:52 ` Alan Modra
@ 2022-01-13 13:19 ` H.J. Lu
2022-01-14 8:12 ` Alan Modra
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2022-01-13 13:19 UTC (permalink / raw)
To: Alan Modra; +Cc: Binutils
On Thu, Jan 13, 2022 at 4:52 AM Alan Modra <amodra@gmail.com> wrote:
>
> On Mon, Jan 10, 2022 at 06:12:41PM -0800, H.J. Lu via Binutils wrote:
> > The existing RELRO scheme may leave a 1-page gap before the RELRO segment
> > and align the end of the RELRO segment to the page size:
> >
> > [18] .eh_frame PROGBITS 408fa0 008fa0 005e80 00 A 0 0 8
> > [19] .init_array INIT_ARRAY 410de0 00fde0 000008 08 WA 0 0 8
> > [20] .fini_array FINI_ARRAY 410de8 00fde8 000008 08 WA 0 0 8
> > [21] .dynamic DYNAMIC 410df0 00fdf0 000200 10 WA 7 0 8
> > [22] .got PROGBITS 410ff0 00fff0 000010 08 WA 0 0 8
> > [23] .got.plt PROGBITS 411000 010000 000048 08 WA 0 0 8
>
> Do you know what is going wrong with the relro section layout for this
> to occur?
>
> In this particular case, the end of the read-only segment is at
> 0x408fa0 + 0x5e80 = 0x40ee20. My guess is that layout of the
> following rw sections starts on the next page plus current offset
> within page, the standard choice to minimise disk pages. ie. We start
> at 0x40fe20. Then discover that this puts .got.plt at 0x40fe20 + 8 +
> 8 + 0x200 + 0x10 = 0x40f040. However, we want this to be on a page
> boundary so that the relro segment ends on a page boundary. So we
> bump 0x40f040 up to 0x411000 and calculate backwards from there to
> arrive at .init_array with a vma of 0x410de0. Resulting in the
> 0x40f000 page being unused.
>
> If instead we start relro layout on the next page, we'd start laying
> out at 0x40f000 rather than 0x40fe20. I think that would be the
But if the prior ro section size is greater than one page, we want
to subtract 1 page:
+ /* 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)
+ {
+ desired_end -= seg->maxpagesize;
+ relro_end -= seg->maxpagesize;
+ }
+ else
+ {
+ desired_end &= ~(seg->maxpagesize - 1);
+ relro_end &= ~(seg->maxpagesize - 1);
+ }
+ }
> correct thing to do rather than fixing up afterwards as your patch
> does.
>
I am checking in my patch as Nick has approved it. There is a
possibility that one 1-page gap may be needed to maintain
the section alignment. My patch checks it and includes many
testcase adjustments to remove one 1-page gap. Can you
update the RELRO segment algorithm to make my fixup
unnecessary?
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] elf: Remove the 1-page gap before the RELRO segment
2022-01-13 13:19 ` H.J. Lu
@ 2022-01-14 8:12 ` Alan Modra
2022-01-14 9:37 ` Fangrui Song
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Alan Modra @ 2022-01-14 8:12 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
On Thu, Jan 13, 2022 at 05:19:22AM -0800, H.J. Lu wrote:
> On Thu, Jan 13, 2022 at 4:52 AM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Mon, Jan 10, 2022 at 06:12:41PM -0800, H.J. Lu via Binutils wrote:
> > > The existing RELRO scheme may leave a 1-page gap before the RELRO segment
> > > and align the end of the RELRO segment to the page size:
> > >
> > > [18] .eh_frame PROGBITS 408fa0 008fa0 005e80 00 A 0 0 8
> > > [19] .init_array INIT_ARRAY 410de0 00fde0 000008 08 WA 0 0 8
> > > [20] .fini_array FINI_ARRAY 410de8 00fde8 000008 08 WA 0 0 8
> > > [21] .dynamic DYNAMIC 410df0 00fdf0 000200 10 WA 7 0 8
> > > [22] .got PROGBITS 410ff0 00fff0 000010 08 WA 0 0 8
> > > [23] .got.plt PROGBITS 411000 010000 000048 08 WA 0 0 8
> >
> > Do you know what is going wrong with the relro section layout for this
> > to occur?
> >
> > In this particular case, the end of the read-only segment is at
> > 0x408fa0 + 0x5e80 = 0x40ee20. My guess is that layout of the
> > following rw sections starts on the next page plus current offset
> > within page, the standard choice to minimise disk pages. ie. We start
> > at 0x40fe20. Then discover that this puts .got.plt at 0x40fe20 + 8 +
> > 8 + 0x200 + 0x10 = 0x40f040. However, we want this to be on a page
> > boundary so that the relro segment ends on a page boundary. So we
> > bump 0x40f040 up to 0x411000 and calculate backwards from there to
> > arrive at .init_array with a vma of 0x410de0. Resulting in the
> > 0x40f000 page being unused.
> >
> > If instead we start relro layout on the next page, we'd start laying
> > out at 0x40f000 rather than 0x40fe20. I think that would be the
>
> But if the prior ro section size is greater than one page, we want
> to subtract 1 page:
>
> + /* 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)
> + {
> + desired_end -= seg->maxpagesize;
> + relro_end -= seg->maxpagesize;
> + }
> + else
> + {
> + desired_end &= ~(seg->maxpagesize - 1);
> + relro_end &= ~(seg->maxpagesize - 1);
> + }
> + }
The above code is the major reason why I took a dislike to your
patch. I fully expected you would have rev 2 posted. Why does
anything depend on the size of a previous section?? That just doesn't
make sense. And please don't write comments that just say what the
code is doing. Any half competent programmer can see what the code is
doing. Write comments that say *why*.
> > correct thing to do rather than fixing up afterwards as your patch
> > does.
> >
>
> I am checking in my patch as Nick has approved it. There is a
> possibility that one 1-page gap may be needed to maintain
> the section alignment. My patch checks it and includes many
> testcase adjustments to remove one 1-page gap. Can you
> update the RELRO segment algorithm to make my fixup
> unnecessary?
Yes, I do think that is possible and desirable. My thinking last
night was that it ought to be easy too. A one-liner even. Silly me,
a little experimentation soon showed up a fail of the pr18176
testcase, demonstrating that there are cases we can save disk space
without causing gaps.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] elf: Remove the 1-page gap before the RELRO segment
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
2 siblings, 0 replies; 20+ messages in thread
From: Fangrui Song @ 2022-01-14 9:37 UTC (permalink / raw)
To: Alan Modra; +Cc: H.J. Lu, Binutils
On 2022-01-14, Alan Modra via Binutils wrote:
>On Thu, Jan 13, 2022 at 05:19:22AM -0800, H.J. Lu wrote:
>> On Thu, Jan 13, 2022 at 4:52 AM Alan Modra <amodra@gmail.com> wrote:
>> >
>> > On Mon, Jan 10, 2022 at 06:12:41PM -0800, H.J. Lu via Binutils wrote:
>> > > The existing RELRO scheme may leave a 1-page gap before the RELRO segment
>> > > and align the end of the RELRO segment to the page size:
>> > >
>> > > [18] .eh_frame PROGBITS 408fa0 008fa0 005e80 00 A 0 0 8
>> > > [19] .init_array INIT_ARRAY 410de0 00fde0 000008 08 WA 0 0 8
>> > > [20] .fini_array FINI_ARRAY 410de8 00fde8 000008 08 WA 0 0 8
>> > > [21] .dynamic DYNAMIC 410df0 00fdf0 000200 10 WA 7 0 8
>> > > [22] .got PROGBITS 410ff0 00fff0 000010 08 WA 0 0 8
>> > > [23] .got.plt PROGBITS 411000 010000 000048 08 WA 0 0 8
>> >
>> > Do you know what is going wrong with the relro section layout for this
>> > to occur?
>> >
>> > In this particular case, the end of the read-only segment is at
>> > 0x408fa0 + 0x5e80 = 0x40ee20. My guess is that layout of the
>> > following rw sections starts on the next page plus current offset
>> > within page, the standard choice to minimise disk pages. ie. We start
>> > at 0x40fe20. Then discover that this puts .got.plt at 0x40fe20 + 8 +
>> > 8 + 0x200 + 0x10 = 0x40f040. However, we want this to be on a page
>> > boundary so that the relro segment ends on a page boundary. So we
>> > bump 0x40f040 up to 0x411000 and calculate backwards from there to
>> > arrive at .init_array with a vma of 0x410de0. Resulting in the
>> > 0x40f000 page being unused.
>> >
>> > If instead we start relro layout on the next page, we'd start laying
>> > out at 0x40f000 rather than 0x40fe20. I think that would be the
>>
>> But if the prior ro section size is greater than one page, we want
>> to subtract 1 page:
>>
>> + /* 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)
>> + {
>> + desired_end -= seg->maxpagesize;
>> + relro_end -= seg->maxpagesize;
>> + }
>> + else
>> + {
>> + desired_end &= ~(seg->maxpagesize - 1);
>> + relro_end &= ~(seg->maxpagesize - 1);
>> + }
>> + }
>
>The above code is the major reason why I took a dislike to your
>patch. I fully expected you would have rev 2 posted. Why does
>anything depend on the size of a previous section?? That just doesn't
>make sense. And please don't write comments that just say what the
>code is doing. Any half competent programmer can see what the code is
>doing. Write comments that say *why*.
>
>> > correct thing to do rather than fixing up afterwards as your patch
>> > does.
>> >
>>
>> I am checking in my patch as Nick has approved it. There is a
>> possibility that one 1-page gap may be needed to maintain
>> the section alignment. My patch checks it and includes many
>> testcase adjustments to remove one 1-page gap. Can you
>> update the RELRO segment algorithm to make my fixup
>> unnecessary?
>
>Yes, I do think that is possible and desirable. My thinking last
>night was that it ought to be easy too. A one-liner even. Silly me,
>a little experimentation soon showed up a fail of the pr18176
>testcase, demonstrating that there are cases we can save disk space
>without causing gaps.
How about the 2-RW design? |RW(RELRO) |RW(non-RELRO)
The bar | indicates an alignment. Only alignment the start of a
segment, never the end of a segment.
It may avoid the unusual "otherwise DATA_SEGMENT_ALIGN is padded so that
exp + offset is aligned to the commonpagesize argument given to
DATA_SEGMENT_ALIGN" semantics on
https://sourceware.org/binutils/docs/ld/Builtin-Functions.html
I thought about the layout in the past and did not find cases where it
used more pages than the single-RW layout, but I did not think very
hard. (I haven't thought about max-page-size > common-page-size cases.)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] elf: Remove the 1-page gap before the RELRO segment
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
2 siblings, 0 replies; 20+ messages in thread
From: H.J. Lu @ 2022-01-14 14:58 UTC (permalink / raw)
To: Alan Modra; +Cc: Binutils
On Fri, Jan 14, 2022 at 12:12 AM Alan Modra <amodra@gmail.com> wrote:
>
> On Thu, Jan 13, 2022 at 05:19:22AM -0800, H.J. Lu wrote:
> > On Thu, Jan 13, 2022 at 4:52 AM Alan Modra <amodra@gmail.com> wrote:
> > >
> > > On Mon, Jan 10, 2022 at 06:12:41PM -0800, H.J. Lu via Binutils wrote:
> > > > The existing RELRO scheme may leave a 1-page gap before the RELRO segment
> > > > and align the end of the RELRO segment to the page size:
> > > >
> > > > [18] .eh_frame PROGBITS 408fa0 008fa0 005e80 00 A 0 0 8
> > > > [19] .init_array INIT_ARRAY 410de0 00fde0 000008 08 WA 0 0 8
> > > > [20] .fini_array FINI_ARRAY 410de8 00fde8 000008 08 WA 0 0 8
> > > > [21] .dynamic DYNAMIC 410df0 00fdf0 000200 10 WA 7 0 8
> > > > [22] .got PROGBITS 410ff0 00fff0 000010 08 WA 0 0 8
> > > > [23] .got.plt PROGBITS 411000 010000 000048 08 WA 0 0 8
> > >
> > > Do you know what is going wrong with the relro section layout for this
> > > to occur?
> > >
> > > In this particular case, the end of the read-only segment is at
> > > 0x408fa0 + 0x5e80 = 0x40ee20. My guess is that layout of the
> > > following rw sections starts on the next page plus current offset
> > > within page, the standard choice to minimise disk pages. ie. We start
> > > at 0x40fe20. Then discover that this puts .got.plt at 0x40fe20 + 8 +
> > > 8 + 0x200 + 0x10 = 0x40f040. However, we want this to be on a page
> > > boundary so that the relro segment ends on a page boundary. So we
> > > bump 0x40f040 up to 0x411000 and calculate backwards from there to
> > > arrive at .init_array with a vma of 0x410de0. Resulting in the
> > > 0x40f000 page being unused.
> > >
> > > If instead we start relro layout on the next page, we'd start laying
> > > out at 0x40f000 rather than 0x40fe20. I think that would be the
> >
> > But if the prior ro section size is greater than one page, we want
> > to subtract 1 page:
> >
> > + /* 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)
> > + {
> > + desired_end -= seg->maxpagesize;
> > + relro_end -= seg->maxpagesize;
> > + }
> > + else
> > + {
> > + desired_end &= ~(seg->maxpagesize - 1);
> > + relro_end &= ~(seg->maxpagesize - 1);
> > + }
> > + }
>
> The above code is the major reason why I took a dislike to your
> patch. I fully expected you would have rev 2 posted. Why does
> anything depend on the size of a previous section?? That just doesn't
If the size of the preceding section is less than one page, we need
to align the RELRO segment. If we subtract one page, we still
have a 1-page gap. I ran out of time before I could dig into the
complex logic.
> make sense. And please don't write comments that just say what the
> code is doing. Any half competent programmer can see what the code is
> doing. Write comments that say *why*.
>
> > > correct thing to do rather than fixing up afterwards as your patch
> > > does.
> > >
> >
> > I am checking in my patch as Nick has approved it. There is a
> > possibility that one 1-page gap may be needed to maintain
> > the section alignment. My patch checks it and includes many
> > testcase adjustments to remove one 1-page gap. Can you
> > update the RELRO segment algorithm to make my fixup
> > unnecessary?
>
> Yes, I do think that is possible and desirable. My thinking last
> night was that it ought to be easy too. A one-liner even. Silly me,
> a little experimentation soon showed up a fail of the pr18176
> testcase, demonstrating that there are cases we can save disk space
> without causing gaps.
>
> --
> Alan Modra
> Australia Development Lab, IBM
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ld: Rewrite lang_size_relro_segment_1
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 ` H.J. Lu
2022-01-17 4:08 ` Alan Modra
2 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2022-01-14 21:55 UTC (permalink / raw)
To: Alan Modra; +Cc: Binutils
On Fri, Jan 14, 2022 at 06:42:16PM +1030, Alan Modra wrote:
> On Thu, Jan 13, 2022 at 05:19:22AM -0800, H.J. Lu wrote:
> > On Thu, Jan 13, 2022 at 4:52 AM Alan Modra <amodra@gmail.com> wrote:
> > >
> > > On Mon, Jan 10, 2022 at 06:12:41PM -0800, H.J. Lu via Binutils wrote:
> > > > The existing RELRO scheme may leave a 1-page gap before the RELRO segment
> > > > and align the end of the RELRO segment to the page size:
> > > >
> > > > [18] .eh_frame PROGBITS 408fa0 008fa0 005e80 00 A 0 0 8
> > > > [19] .init_array INIT_ARRAY 410de0 00fde0 000008 08 WA 0 0 8
> > > > [20] .fini_array FINI_ARRAY 410de8 00fde8 000008 08 WA 0 0 8
> > > > [21] .dynamic DYNAMIC 410df0 00fdf0 000200 10 WA 7 0 8
> > > > [22] .got PROGBITS 410ff0 00fff0 000010 08 WA 0 0 8
> > > > [23] .got.plt PROGBITS 411000 010000 000048 08 WA 0 0 8
> > >
> > > Do you know what is going wrong with the relro section layout for this
> > > to occur?
> > >
> > > In this particular case, the end of the read-only segment is at
> > > 0x408fa0 + 0x5e80 = 0x40ee20. My guess is that layout of the
> > > following rw sections starts on the next page plus current offset
> > > within page, the standard choice to minimise disk pages. ie. We start
> > > at 0x40fe20. Then discover that this puts .got.plt at 0x40fe20 + 8 +
> > > 8 + 0x200 + 0x10 = 0x40f040. However, we want this to be on a page
> > > boundary so that the relro segment ends on a page boundary. So we
> > > bump 0x40f040 up to 0x411000 and calculate backwards from there to
> > > arrive at .init_array with a vma of 0x410de0. Resulting in the
> > > 0x40f000 page being unused.
> > >
> > > If instead we start relro layout on the next page, we'd start laying
> > > out at 0x40f000 rather than 0x40fe20. I think that would be the
> >
> > But if the prior ro section size is greater than one page, we want
> > to subtract 1 page:
> >
> > + /* 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)
> > + {
> > + desired_end -= seg->maxpagesize;
> > + relro_end -= seg->maxpagesize;
> > + }
> > + else
> > + {
> > + desired_end &= ~(seg->maxpagesize - 1);
> > + relro_end &= ~(seg->maxpagesize - 1);
> > + }
> > + }
>
> The above code is the major reason why I took a dislike to your
> patch. I fully expected you would have rev 2 posted. Why does
> anything depend on the size of a previous section?? That just doesn't
> make sense. And please don't write comments that just say what the
> code is doing. Any half competent programmer can see what the code is
> doing. Write comments that say *why*.
>
> > > correct thing to do rather than fixing up afterwards as your patch
> > > does.
> > >
> >
> > I am checking in my patch as Nick has approved it. There is a
> > possibility that one 1-page gap may be needed to maintain
> > the section alignment. My patch checks it and includes many
> > testcase adjustments to remove one 1-page gap. Can you
> > update the RELRO segment algorithm to make my fixup
> > unnecessary?
>
> Yes, I do think that is possible and desirable. My thinking last
> night was that it ought to be easy too. A one-liner even. Silly me,
> a little experimentation soon showed up a fail of the pr18176
> testcase, demonstrating that there are cases we can save disk space
> without causing gaps.
>
Now I have time to rewrite lang_size_relro_segment_1. Is it OK for
master?
Thanks.
H.J.
---
1. Find the first section in the relro segment.
2. Find the first non-empty preceding section.
3. Don't add the 1-page gap before the relro segment if the maximum page
size >= the maximum section alignment.
4. Align the relro segment if there is a 1-page gap before the relro
segment.
PR ld/28743
* ldlang.c (lang_size_relro_segment_1): Rewrite.
---
ld/ldlang.c | 103 ++++++++++++++++++++++++----------------------------
1 file changed, 48 insertions(+), 55 deletions(-)
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 48e40828634..90b4e868a89 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6370,9 +6370,8 @@ 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_end, aligned_start = 0;
+ asection *sec, *prev_sec = NULL, *relro_sec = NULL;
unsigned int max_alignment_power = 0;
/* Compute the expected PT_GNU_RELRO/PT_LOAD segment end. */
@@ -6382,7 +6381,7 @@ lang_size_relro_segment_1 (seg_align_type *seg)
/* Adjust by the offset arg of XXX_SEGMENT_RELRO_END. */
desired_end = relro_end - seg->relro_offset;
- /* For sections in the relro segment.. */
+ /* 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)
{
@@ -6392,71 +6391,65 @@ lang_size_relro_segment_1 (seg_align_type *seg)
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? */
- bfd_vma start, end, bump;
-
- end = start = sec->vma;
- if (!IS_TBSS (sec))
- end += TO_ADDR (sec->size);
- bump = desired_end - 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;
+ relro_sec = sec;
prev_sec = sec->prev;
}
}
- 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)
- {
- /* The 1-page gap before the RELRO segment may be removed. */
- remove_page_gap = ((prev_sec->vma + prev_sec->size
- + seg->maxpagesize) < desired_end);
-
- break;
- }
- }
-
- if (remove_page_gap)
+ if (relro_sec)
{
- /* 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;
+ /* Where do we want to put this section so that it ends as
+ desired? */
+ 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;
- /* Remove the 1-page gap before the RELRO segment only if the
- maximum page size >= the maximum section alignment. */
+ end = start = relro_sec->vma;
+ if (!IS_TBSS (relro_sec))
+ end += TO_ADDR (relro_sec->size);
+ bump = desired_end - end;
if (seg->maxpagesize >= (1U << max_alignment_power))
{
- /* 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)
+ /* Don't add the 1-page gap before the relro segment if the
+ maximum page size >= the maximum section alignment. */
+ if (bump > seg->maxpagesize)
{
- desired_end -= seg->maxpagesize;
+ bump -= seg->maxpagesize;
relro_end -= seg->maxpagesize;
}
- else
+ else if (prev_sec != NULL)
{
- desired_end &= ~(seg->maxpagesize - 1);
- relro_end &= ~(seg->maxpagesize - 1);
+ /* Align the relro segment if there is a 1-page gap
+ before the relro segment. */
+ aligned_start = start + bump;
+ aligned_start &= ~(((bfd_vma) 1 << relro_sec->alignment_power)
+ - 1);
+ if ((prev_sec->vma + prev_sec->size + seg->maxpagesize)
+ < aligned_start)
+ {
+ aligned_start &= ~(seg->maxpagesize - 1);
+ relro_end &= ~(seg->maxpagesize - 1);
+ }
}
- }
- }
+ }
+ /* We'd like to increase START by BUMP, but we must heed
+ alignment so the increase might be less than optimum. */
+ if (aligned_start != 0)
+ start = aligned_start;
+ else
+ {
+ start += bump;
+ start &= ~(((bfd_vma) 1 << relro_sec->alignment_power) - 1);
+ }
+ /* This is now the desired end for the previous section. */
+ desired_end = start;
+ }
+ seg->phase = exp_seg_relro_adjust;
+ ASSERT (aligned_start != 0 || desired_end >= seg->base);
seg->base = desired_end;
return relro_end;
}
--
2.34.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ld: Rewrite lang_size_relro_segment_1
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
0 siblings, 1 reply; 20+ messages in thread
From: Alan Modra @ 2022-01-17 4:08 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
On Fri, Jan 14, 2022 at 01:55:27PM -0800, H.J. Lu wrote:
> Now I have time to rewrite lang_size_relro_segment_1. Is it OK for
> master?
No, I don't believe you can calculate the adjustment for the first
relro section without paintakingly working backwards from the last
section taking into account each section's alignment. See commit
0e5fabeb2c4b and b39910205f54.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] ld: Rewrite lang_size_relro_segment_1
2022-01-17 4:08 ` Alan Modra
@ 2022-01-18 4:16 ` H.J. Lu
[not found] ` <CAMe9rOpdkYZDigz8r_oPbweLnaCJUjx3-L-v-vp-70c0MGOHQw@mail.gmail.com>
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2022-01-18 4:16 UTC (permalink / raw)
To: Alan Modra; +Cc: Binutils
[-- Attachment #1: Type: text/plain, Size: 670 bytes --]
On Sun, Jan 16, 2022 at 8:08 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Fri, Jan 14, 2022 at 01:55:27PM -0800, H.J. Lu wrote:
> > Now I have time to rewrite lang_size_relro_segment_1. Is it OK for
> > master?
>
> No, I don't believe you can calculate the adjustment for the first
> relro section without paintakingly working backwards from the last
> section taking into account each section's alignment. See commit
> 0e5fabeb2c4b and b39910205f54.
>
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.
--
H.J.
[-- Attachment #2: v2-0001-ld-Rewrite-lang_size_relro_segment_1.patch --]
[-- Type: text/x-patch, Size: 11043 bytes --]
From 6cef5bac1669defc772927d0dd93a66b0a32d027 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 14 Jan 2022 13:48:36 -0800
Subject: [PATCH v2] ld: Rewrite lang_size_relro_segment_1
Rewrite lang_size_relro_segment_1:
1. Find the first section in the relro segment.
2. Find the first non-empty preceding section.
3. Find the maximum section alignment in sections starting from the
PT_GNU_RELRO segment.
4. Don't add the 1-page gap before the relro segment if the maximum page
size >= the maximum section alignment. When the maximum page size >= the
maximum section alignment, align the PT_GNU_RELRO segment to the maximum
page size won't impact the maximum section alignment in sections starting
from the PT_GNU_RELRO segment.
5. Align the relro segment if there is a 1-page gap before the relro
segment.
PR ld/28743
* 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 | 115 +++++++++++++++--------------
ld/testsuite/ld-x86-64/pr28743-1.d | 52 +++++++++++++
ld/testsuite/ld-x86-64/pr28743-1.s | 16 ++++
ld/testsuite/ld-x86-64/x86-64.exp | 1 +
4 files changed, 128 insertions(+), 56 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..5403bfe237f 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6370,10 +6370,15 @@ 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_end, aligned_start = 0;
+ asection *sec, *prev_sec = NULL, *relro_sec = NULL;
unsigned int max_alignment_power = 0;
+ enum
+ {
+ relro_sec_after, /* After the relro segment. */
+ relro_sec_in, /* In the relro segment. */
+ relro_sec_before /* Before the relro segment. */
+ } last_sec_status = relro_sec_after;
/* Compute the expected PT_GNU_RELRO/PT_LOAD segment end. */
relro_end = ((seg->relro_end + seg->pagesize - 1)
@@ -6382,81 +6387,79 @@ lang_size_relro_segment_1 (seg_align_type *seg)
/* Adjust by the offset arg of XXX_SEGMENT_RELRO_END. */
desired_end = relro_end - seg->relro_offset;
- /* For sections in the relro segment.. */
+ /* 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 (sec->alignment_power > max_alignment_power)
+ 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)
{
- /* Where do we want to put this section so that it ends as
- desired? */
- bfd_vma start, end, bump;
-
- end = start = sec->vma;
- if (!IS_TBSS (sec))
- end += TO_ADDR (sec->size);
- bump = desired_end - 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;
+ relro_sec = sec;
prev_sec = sec->prev;
+ last_sec_status = relro_sec_in;
}
+ else
+ last_sec_status = relro_sec_before;
}
- 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)
- {
- /* The 1-page gap before the RELRO segment may be removed. */
- remove_page_gap = ((prev_sec->vma + prev_sec->size
- + seg->maxpagesize) < desired_end);
-
- break;
- }
- }
-
- if (remove_page_gap)
+ if (relro_sec)
{
- /* 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;
+ /* Where do we want to put this section so that it ends as
+ desired? */
+ 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;
- /* Remove the 1-page gap before the RELRO segment only if the
- maximum page size >= the maximum section alignment. */
+ end = start = relro_sec->vma;
+ if (!IS_TBSS (relro_sec))
+ end += TO_ADDR (relro_sec->size);
+ bump = desired_end - end;
if (seg->maxpagesize >= (1U << max_alignment_power))
{
- /* 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)
+ /* Don't add the 1-page gap before the relro segment if the
+ maximum page size >= the maximum section alignment. */
+ if (bump > seg->maxpagesize)
{
- desired_end -= seg->maxpagesize;
+ bump -= seg->maxpagesize;
relro_end -= seg->maxpagesize;
}
- else
+ else if (prev_sec != NULL)
{
- desired_end &= ~(seg->maxpagesize - 1);
- relro_end &= ~(seg->maxpagesize - 1);
+ /* Align the relro segment if there is a 1-page gap
+ before the relro segment. */
+ aligned_start = start + bump;
+ aligned_start &= ~(((bfd_vma) 1 << relro_sec->alignment_power)
+ - 1);
+ if ((prev_sec->vma + prev_sec->size + seg->maxpagesize)
+ < aligned_start)
+ {
+ aligned_start &= ~(seg->maxpagesize - 1);
+ relro_end &= ~(seg->maxpagesize - 1);
+ }
}
- }
- }
+ }
+ /* We'd like to increase START by BUMP, but we must heed
+ alignment so the increase might be less than optimum. */
+ if (aligned_start != 0)
+ start = aligned_start;
+ else
+ {
+ start += bump;
+ start &= ~(((bfd_vma) 1 << relro_sec->alignment_power) - 1);
+ }
+ /* This is now the desired end for the previous section. */
+ desired_end = start;
+ }
+ seg->phase = exp_seg_relro_adjust;
+ ASSERT (aligned_start != 0 || desired_end >= seg->base);
seg->base = desired_end;
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..36297d284e8
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr28743-1.d
@@ -0,0 +1,52 @@
+#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 0000000000000158 000158 000018 04 A 2 0 8
+ \[ 2\] .dynsym DYNSYM 0000000000000170 000170 000048 18 A 3 1 8
+ \[ 3\] .dynstr STRTAB 00000000000001b8 0001b8 00000a 00 A 0 0 1
+ \[ 4\] .rela.dyn RELA 00000000000001c8 0001c8 000018 18 A 2 0 8
+ \[ 5\] .plt PROGBITS 00000000000001e0 0001e0 000010 10 AX 0 0 16
+ \[ 6\] .plt.got PROGBITS 00000000000001f0 0001f0 000008 08 AX 0 0 8
+ \[ 7\] .text PROGBITS 00000000000001f8 0001f8 00000c 00 AX 0 0 1
+ \[ 8\] .eh_frame PROGBITS 0000000000000208 000208 00006c 00 A 0 0 8
+ \[ 9\] .init_array INIT_ARRAY 0000000000001278 000278 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 5 program headers, starting at offset 64
+
+Program Headers:
+ Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
+ LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x000274 0x000274 R E 0x1000
+ LOAD 0x000278 0x0000000000001278 0x0000000000001278 0x000004 0x000004 RW 0x1000
+ LOAD 0x100000 0x0000000000100000 0x0000000000100000 0x001100 0x001100 RW 0x100000
+ DYNAMIC 0x100100 0x0000000000100100 0x0000000000100100 0x000150 0x000150 RW 0x8
+ GNU_RELRO 0x000278 0x0000000000001278 0x0000000000001278 0x000004 0x0ffd88 R 0x1
+
+ Section to Segment mapping:
+ Segment Sections...
+ 00 .hash .dynsym .dynstr .rela.dyn .plt .plt.got .text .eh_frame
+ 01 .init_array
+ 02 .fini_array .dynamic .got .data
+ 03 .dynamic
+ 04 .init_array
+#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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Fwd: [PATCH v2] ld: Rewrite lang_size_relro_segment_1
[not found] ` <CAMe9rOpdkYZDigz8r_oPbweLnaCJUjx3-L-v-vp-70c0MGOHQw@mail.gmail.com>
@ 2022-01-24 16:24 ` Nick Clifton
2022-01-24 21:17 ` [PATCH v3] " H.J. Lu
0 siblings, 1 reply; 20+ messages in thread
From: Nick Clifton @ 2022-01-24 16:24 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
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 ?
Once last_sec_status has been set to relro_sec_before, is there
any point in continuing the scan ?
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 ?
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 ?
/* 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.
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.
Cheers
Nick
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3] ld: Rewrite lang_size_relro_segment_1
2022-01-24 16:24 ` Fwd: " Nick Clifton
@ 2022-01-24 21:17 ` H.J. Lu
2022-01-25 15:05 ` [PATCH v4] " H.J. Lu
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2022-01-24 21:17 UTC (permalink / raw)
To: Nick Clifton; +Cc: Binutils
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.
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. Subtract the maximum page size from the desired PT_GNU_RELRO
segment base only if the preceding load section size is > the maximum
page size to avoid moving the PT_GNU_RELRO segment base before the
preceding load section. Otherwise, simply align the desired PT_GNU_RELRO
segment base.
PR ld/28743
* 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 | 90 ++++++++++++++++--------------
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, 114 insertions(+), 43 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..25eac62a63a 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6370,94 +6370,98 @@ 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;
- /* 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;
+
+ if ((prev_sec->vma + prev_sec->size + seg->maxpagesize)
+ < 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. */
+ desired_relro_base_reduced = true;
+
+ /* Don't add the 1-page gap before the relro segment if the
+ maximum page size >= the maximum section alignment. */
if (prev_sec->size > seg->maxpagesize)
{
- desired_end -= seg->maxpagesize;
+ /* Subtract the maximum page size only if the preceding
+ load section size is > the maximum page size to avoid
+ moving the relro segment base before the preceding
+ load section. */
+ desired_relro_base -= seg->maxpagesize;
relro_end -= seg->maxpagesize;
}
else
{
- desired_end &= ~(seg->maxpagesize - 1);
+ /* Align the relro segment if there is a 1-page gap
+ before the relro segment. */
+ desired_relro_base &= ~(seg->maxpagesize - 1);
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4] ld: Rewrite lang_size_relro_segment_1
2022-01-24 21:17 ` [PATCH v3] " H.J. Lu
@ 2022-01-25 15:05 ` H.J. Lu
2022-01-26 10:55 ` Nick Clifton
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2022-01-25 15:05 UTC (permalink / raw)
To: Nick Clifton, Binutils
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] ld: Rewrite lang_size_relro_segment_1
2022-01-25 15:05 ` [PATCH v4] " H.J. Lu
@ 2022-01-26 10:55 ` Nick Clifton
2022-01-27 0:48 ` Alan Modra
0 siblings, 1 reply; 20+ messages in thread
From: Nick Clifton @ 2022-01-26 10:55 UTC (permalink / raw)
To: H.J. Lu, Binutils
Hi H.J.
> 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
Looks good to me. Please apply. Branch and mainline.
Cheers
Nick
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] ld: Rewrite lang_size_relro_segment_1
2022-01-26 10:55 ` Nick Clifton
@ 2022-01-27 0:48 ` Alan Modra
2022-01-27 2:10 ` H.J. Lu
0 siblings, 1 reply; 20+ messages in thread
From: Alan Modra @ 2022-01-27 0:48 UTC (permalink / raw)
To: Nick Clifton; +Cc: H.J. Lu, Binutils
On Wed, Jan 26, 2022 at 10:55:35AM +0000, Nick Clifton via Binutils wrote:
> Hi H.J.
>
> > 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
>
> Looks good to me. Please apply. Branch and mainline.
How was this patch tested? If it has not been tested on targets other
than x86 then it is not appropriate for the branch. Instead, I
believe commit 2f83249c13d ought to be reverted on the branch. A
wasted page in memory layout is not an important problem. We have
more serious relro issues on targets other than x86. See pr28824.
I'm speaking up because I raised concerns over the design of the
previous patch, not just the implementation, and had those concerns
overridden by Nick. Or possibly Nick didn't see my email before the
previous patch was approved. Regardless of how it happened, I fear
we might have even worse relro support in 2.38.
FWIW, I also think that any fix I might develop for pr28824 will
likely not be release branch quality until is has been tested for some
time on mainline.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] ld: Rewrite lang_size_relro_segment_1
2022-01-27 0:48 ` Alan Modra
@ 2022-01-27 2:10 ` H.J. Lu
2022-01-29 1:01 ` Alan Modra
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2022-01-27 2:10 UTC (permalink / raw)
To: Alan Modra; +Cc: Nick Clifton, Binutils
On Wed, Jan 26, 2022 at 4:48 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Wed, Jan 26, 2022 at 10:55:35AM +0000, Nick Clifton via Binutils wrote:
> > Hi H.J.
> >
> > > 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
> >
> > Looks good to me. Please apply. Branch and mainline.
>
> How was this patch tested? If it has not been tested on targets other
> than x86 then it is not appropriate for the branch. Instead, I
What kinds of tests are you looking for? Do you have a testcase to show
there is still an issue?
> believe commit 2f83249c13d ought to be reverted on the branch. A
> wasted page in memory layout is not an important problem. We have
> more serious relro issues on targets other than x86. See pr28824.
>
> I'm speaking up because I raised concerns over the design of the
> previous patch, not just the implementation, and had those concerns
> overridden by Nick. Or possibly Nick didn't see my email before the
> previous patch was approved. Regardless of how it happened, I fear
> we might have even worse relro support in 2.38.
>
> FWIW, I also think that any fix I might develop for pr28824 will
> likely not be release branch quality until is has been tested for some
> time on mainline.
>
> --
> Alan Modra
> Australia Development Lab, IBM
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] ld: Rewrite lang_size_relro_segment_1
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
0 siblings, 2 replies; 20+ messages in thread
From: Alan Modra @ 2022-01-29 1:01 UTC (permalink / raw)
To: H.J. Lu; +Cc: Nick Clifton, Binutils
On Wed, Jan 26, 2022 at 06:10:28PM -0800, H.J. Lu wrote:
> On Wed, Jan 26, 2022 at 4:48 PM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Wed, Jan 26, 2022 at 10:55:35AM +0000, Nick Clifton via Binutils wrote:
> > > Hi H.J.
> > >
> > > > 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
> > >
> > > Looks good to me. Please apply. Branch and mainline.
> >
> > How was this patch tested? If it has not been tested on targets other
> > than x86 then it is not appropriate for the branch. Instead, I
>
> What kinds of tests are you looking for? Do you have a testcase to show
> there is still an issue?
Can you answer my question?
> > believe commit 2f83249c13d ought to be reverted on the branch. A
> > wasted page in memory layout is not an important problem. We have
> > more serious relro issues on targets other than x86. See pr28824.
> >
> > I'm speaking up because I raised concerns over the design of the
> > previous patch, not just the implementation, and had those concerns
> > overridden by Nick. Or possibly Nick didn't see my email before the
> > previous patch was approved. Regardless of how it happened, I fear
> > we might have even worse relro support in 2.38.
> >
> > FWIW, I also think that any fix I might develop for pr28824 will
> > likely not be release branch quality until is has been tested for some
> > time on mainline.
This is the patch I'm playing with. It reverts your changes to
lang_size_relro_segment_1 and fixes PR2844.
The end of the PT_GNU_RELRO segment must be aligned to maxpagesize in
order for the last page in the segment to be write protected after
relocation. Unfortunately we currently only align to commonpagesize,
which works for x86 but no other architecture that actually uses
maxpagesize memory pages. I've kept the commonpage alignment for x86,
other targets get maxpagesize.
There are of course testsuite changes that are needed, multiple x86
and one s390x test. The only fail of any significance is pr18176, but
there is a fundamental tension between pr18176 and pr28743. You
either create holes or you waste disk space, both conditions can't be
met. Worse IMO, the pr18176 testcase not only has a hole but also a
gap in memory layout after the last section in the relro segment.
That's not ideal for powerpc where sections after .got (the last relro
section) are in some cases accessed via the got/toc pointer reg. On
x86 you'd see a gap between .got and .got.plt when SEPARATE_GOTPLT.
diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
index da8a488db36..c9b3e7c4d9f 100644
--- a/bfd/elfxx-x86.c
+++ b/bfd/elfxx-x86.c
@@ -4205,4 +4205,5 @@ _bfd_elf_linker_x86_set_options (struct bfd_link_info * info,
= elf_x86_hash_table (info, bed->target_id);
if (htab != NULL)
htab->params = params;
+ info->relro_use_commonpagesize = true;
}
diff --git a/include/bfdlink.h b/include/bfdlink.h
index 69fc9d33ff4..8d5561b0b28 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -413,6 +413,10 @@ struct bfd_link_info
/* TRUE if PT_GNU_RELRO segment should be created. */
unsigned int relro: 1;
+ /* TRUE if the relro segment should be aligned to commonpagesize
+ rather than maxpagesize. */
+ unsigned int relro_use_commonpagesize : 1;
+
/* TRUE if DT_RELR should be enabled for compact relative
relocations. */
unsigned int enable_dt_relr: 1;
diff --git a/ld/ldexp.c b/ld/ldexp.c
index 5f904aaf8ae..65618912043 100644
--- a/ld/ldexp.c
+++ b/ld/ldexp.c
@@ -469,7 +469,8 @@ fold_segment_align (seg_align_type *seg, etree_value_type *lhs)
}
else
{
- expld.result.value += expld.dot & (maxpage - 1);
+ if (!link_info.relro)
+ expld.result.value += expld.dot & (maxpage - 1);
if (seg->phase == exp_seg_done)
{
/* OK. */
@@ -480,6 +481,10 @@ fold_segment_align (seg_align_type *seg, etree_value_type *lhs)
seg->base = expld.result.value;
seg->pagesize = commonpage;
seg->maxpagesize = maxpage;
+ if (link_info.relro_use_commonpagesize)
+ seg->relropagesize = commonpage;
+ else
+ seg->relropagesize = maxpage;
seg->relro_end = 0;
}
else
@@ -508,10 +513,10 @@ fold_segment_relro_end (seg_align_type *seg, etree_value_type *lhs)
seg->relro_end = lhs->value + expld.result.value;
if (seg->phase == exp_seg_relro_adjust
- && (seg->relro_end & (seg->pagesize - 1)))
+ && (seg->relro_end & (seg->relropagesize - 1)))
{
- seg->relro_end += seg->pagesize - 1;
- seg->relro_end &= ~(seg->pagesize - 1);
+ seg->relro_end += seg->relropagesize - 1;
+ seg->relro_end &= ~(seg->relropagesize - 1);
expld.result.value = seg->relro_end - expld.result.value;
}
else
diff --git a/ld/ldexp.h b/ld/ldexp.h
index ac4fa7e82b0..f1701b66330 100644
--- a/ld/ldexp.h
+++ b/ld/ldexp.h
@@ -136,7 +136,8 @@ enum relro_enum {
typedef struct {
enum phase_enum phase;
- bfd_vma base, relro_offset, relro_end, end, pagesize, maxpagesize;
+ bfd_vma base, relro_offset, relro_end, end;
+ bfd_vma pagesize, maxpagesize, relropagesize;
enum relro_enum relro;
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 5dd3df12a0f..1a09b988fbe 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6370,101 +6370,40 @@ lang_size_segment (seg_align_type *seg)
static bfd_vma
lang_size_relro_segment_1 (seg_align_type *seg)
{
- 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;
+ bfd_vma relro_end, desired_end;
+ asection *sec;
/* Compute the expected PT_GNU_RELRO/PT_LOAD segment end. */
- relro_end = ((seg->relro_end + seg->pagesize - 1)
- & ~(seg->pagesize - 1));
+ relro_end = (seg->relro_end + seg->relropagesize - 1) & -seg->relropagesize;
/* Adjust by the offset arg of XXX_SEGMENT_RELRO_END. */
- desired_relro_base = relro_end - seg->relro_offset;
+ desired_end = 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)
+ if ((sec->flags & SEC_ALLOC) != 0
+ && sec->vma >= seg->base
+ && sec->vma < seg->relro_end - seg->relro_offset)
{
- /* 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 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_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_relro_base = start;
- relro_sec = sec;
- seen_reloc_section = true;
- }
- else if (seen_reloc_section)
- {
- /* Stop searching if we see a non-relro section after seeing
- relro sections. */
- break;
- }
+ /* Where do we want to put this section so that it ends as
+ desired? */
+ bfd_vma start, end, bump;
+
+ end = start = sec->vma;
+ if (!IS_TBSS (sec))
+ end += TO_ADDR (sec->size);
+ bump = desired_end - 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;
}
- if (relro_sec != NULL
- && seg->maxpagesize >= (1U << max_alignment_power))
- {
- asection *prev_sec;
- bfd_vma prev_sec_end_plus_1_page;
-
- /* 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)
- {
- 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)
- {
- /* Subtract the maximum page size if therer is still a
- 1-page gap. */
- desired_relro_base -= seg->maxpagesize;
- relro_end -= seg->maxpagesize;
- }
- else
- {
- /* Align the relro segment. */
- desired_relro_base = aligned_relro_base;
- relro_end &= ~(seg->maxpagesize - 1);
- }
- }
- }
-
seg->phase = exp_seg_relro_adjust;
- ASSERT (desired_relro_base_reduced
- || desired_relro_base >= seg->base);
- seg->base = desired_relro_base;
+ ASSERT (desired_end >= seg->base);
+ seg->base = desired_end;
return relro_end;
}
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] ld: Rewrite lang_size_relro_segment_1
2022-01-29 1:01 ` Alan Modra
@ 2022-01-29 9:06 ` Fangrui Song
2022-01-29 16:45 ` H.J. Lu
1 sibling, 0 replies; 20+ messages in thread
From: Fangrui Song @ 2022-01-29 9:06 UTC (permalink / raw)
To: Alan Modra; +Cc: H.J. Lu, Binutils
On 2022-01-29, Alan Modra via Binutils wrote:
>On Wed, Jan 26, 2022 at 06:10:28PM -0800, H.J. Lu wrote:
>> On Wed, Jan 26, 2022 at 4:48 PM Alan Modra <amodra@gmail.com> wrote:
>> >
>> > On Wed, Jan 26, 2022 at 10:55:35AM +0000, Nick Clifton via Binutils wrote:
>> > > Hi H.J.
>> > >
>> > > > 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
>> > >
>> > > Looks good to me. Please apply. Branch and mainline.
>> >
>> > How was this patch tested? If it has not been tested on targets other
>> > than x86 then it is not appropriate for the branch. Instead, I
>>
>> What kinds of tests are you looking for? Do you have a testcase to show
>> there is still an issue?
>
>Can you answer my question?
>
>> > believe commit 2f83249c13d ought to be reverted on the branch. A
>> > wasted page in memory layout is not an important problem. We have
>> > more serious relro issues on targets other than x86. See pr28824.
>> >
>> > I'm speaking up because I raised concerns over the design of the
>> > previous patch, not just the implementation, and had those concerns
>> > overridden by Nick. Or possibly Nick didn't see my email before the
>> > previous patch was approved. Regardless of how it happened, I fear
>> > we might have even worse relro support in 2.38.
>> >
>> > FWIW, I also think that any fix I might develop for pr28824 will
>> > likely not be release branch quality until is has been tested for some
>> > time on mainline.
>
>This is the patch I'm playing with. It reverts your changes to
>lang_size_relro_segment_1 and fixes PR2844.
>
>The end of the PT_GNU_RELRO segment must be aligned to maxpagesize in
>order for the last page in the segment to be write protected after
>relocation. Unfortunately we currently only align to commonpagesize,
>which works for x86 but no other architecture that actually uses
>maxpagesize memory pages. I've kept the commonpage alignment for x86,
>other targets get maxpagesize.
Wouldn't aligning to maxpagesize punish the architectures with 64KiB
maxpagesize? These binaries will incur size bloat due the PT_GNU_RELRO
end alignment.
I think my idea of two PF_R|PF_W PT_LOAD will be worth implementing at
some point :) There is no alignment.
The glibc strict check
https://sourceware.org/bugzilla/show_bug.cgi?id=28688 affects glibc<2.35
so I don't think ld making p_align smaller than maxpagesize is
reasonable in the near term, at least before objcopy/strip supporting
the small p_align becomes widespread.
(I need to confess I am not closely subscribed to the sudden p_align
movement in glibc)
>There are of course testsuite changes that are needed, multiple x86
>and one s390x test. The only fail of any significance is pr18176, but
>there is a fundamental tension between pr18176 and pr28743. You
>either create holes or you waste disk space, both conditions can't be
>met. Worse IMO, the pr18176 testcase not only has a hole but also a
>gap in memory layout after the last section in the relro segment.
>That's not ideal for powerpc where sections after .got (the last relro
>section) are in some cases accessed via the got/toc pointer reg. On
>x86 you'd see a gap between .got and .got.plt when SEPARATE_GOTPLT.
>
>diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
>index da8a488db36..c9b3e7c4d9f 100644
>--- a/bfd/elfxx-x86.c
>+++ b/bfd/elfxx-x86.c
>@@ -4205,4 +4205,5 @@ _bfd_elf_linker_x86_set_options (struct bfd_link_info * info,
> = elf_x86_hash_table (info, bed->target_id);
> if (htab != NULL)
> htab->params = params;
>+ info->relro_use_commonpagesize = true;
> }
>diff --git a/include/bfdlink.h b/include/bfdlink.h
>index 69fc9d33ff4..8d5561b0b28 100644
>--- a/include/bfdlink.h
>+++ b/include/bfdlink.h
>@@ -413,6 +413,10 @@ struct bfd_link_info
> /* TRUE if PT_GNU_RELRO segment should be created. */
> unsigned int relro: 1;
>
>+ /* TRUE if the relro segment should be aligned to commonpagesize
>+ rather than maxpagesize. */
>+ unsigned int relro_use_commonpagesize : 1;
>+
> /* TRUE if DT_RELR should be enabled for compact relative
> relocations. */
> unsigned int enable_dt_relr: 1;
>diff --git a/ld/ldexp.c b/ld/ldexp.c
>index 5f904aaf8ae..65618912043 100644
>--- a/ld/ldexp.c
>+++ b/ld/ldexp.c
>@@ -469,7 +469,8 @@ fold_segment_align (seg_align_type *seg, etree_value_type *lhs)
> }
> else
> {
>- expld.result.value += expld.dot & (maxpage - 1);
>+ if (!link_info.relro)
>+ expld.result.value += expld.dot & (maxpage - 1);
> if (seg->phase == exp_seg_done)
> {
> /* OK. */
>@@ -480,6 +481,10 @@ fold_segment_align (seg_align_type *seg, etree_value_type *lhs)
> seg->base = expld.result.value;
> seg->pagesize = commonpage;
> seg->maxpagesize = maxpage;
>+ if (link_info.relro_use_commonpagesize)
>+ seg->relropagesize = commonpage;
>+ else
>+ seg->relropagesize = maxpage;
> seg->relro_end = 0;
> }
> else
>@@ -508,10 +513,10 @@ fold_segment_relro_end (seg_align_type *seg, etree_value_type *lhs)
> seg->relro_end = lhs->value + expld.result.value;
>
> if (seg->phase == exp_seg_relro_adjust
>- && (seg->relro_end & (seg->pagesize - 1)))
>+ && (seg->relro_end & (seg->relropagesize - 1)))
> {
>- seg->relro_end += seg->pagesize - 1;
>- seg->relro_end &= ~(seg->pagesize - 1);
>+ seg->relro_end += seg->relropagesize - 1;
>+ seg->relro_end &= ~(seg->relropagesize - 1);
> expld.result.value = seg->relro_end - expld.result.value;
> }
> else
>diff --git a/ld/ldexp.h b/ld/ldexp.h
>index ac4fa7e82b0..f1701b66330 100644
>--- a/ld/ldexp.h
>+++ b/ld/ldexp.h
>@@ -136,7 +136,8 @@ enum relro_enum {
> typedef struct {
> enum phase_enum phase;
>
>- bfd_vma base, relro_offset, relro_end, end, pagesize, maxpagesize;
>+ bfd_vma base, relro_offset, relro_end, end;
>+ bfd_vma pagesize, maxpagesize, relropagesize;
>
> enum relro_enum relro;
>
>diff --git a/ld/ldlang.c b/ld/ldlang.c
>index 5dd3df12a0f..1a09b988fbe 100644
>--- a/ld/ldlang.c
>+++ b/ld/ldlang.c
>@@ -6370,101 +6370,40 @@ lang_size_segment (seg_align_type *seg)
> static bfd_vma
> lang_size_relro_segment_1 (seg_align_type *seg)
> {
>- 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;
>+ bfd_vma relro_end, desired_end;
>+ asection *sec;
>
> /* Compute the expected PT_GNU_RELRO/PT_LOAD segment end. */
>- relro_end = ((seg->relro_end + seg->pagesize - 1)
>- & ~(seg->pagesize - 1));
>+ relro_end = (seg->relro_end + seg->relropagesize - 1) & -seg->relropagesize;
>
> /* Adjust by the offset arg of XXX_SEGMENT_RELRO_END. */
>- desired_relro_base = relro_end - seg->relro_offset;
>+ desired_end = 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)
>+ if ((sec->flags & SEC_ALLOC) != 0
>+ && sec->vma >= seg->base
>+ && sec->vma < seg->relro_end - seg->relro_offset)
> {
>- /* 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 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_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_relro_base = start;
>- relro_sec = sec;
>- seen_reloc_section = true;
>- }
>- else if (seen_reloc_section)
>- {
>- /* Stop searching if we see a non-relro section after seeing
>- relro sections. */
>- break;
>- }
>+ /* Where do we want to put this section so that it ends as
>+ desired? */
>+ bfd_vma start, end, bump;
>+
>+ end = start = sec->vma;
>+ if (!IS_TBSS (sec))
>+ end += TO_ADDR (sec->size);
>+ bump = desired_end - 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;
> }
>
>- if (relro_sec != NULL
>- && seg->maxpagesize >= (1U << max_alignment_power))
>- {
>- asection *prev_sec;
>- bfd_vma prev_sec_end_plus_1_page;
>-
>- /* 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)
>- {
>- 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)
>- {
>- /* Subtract the maximum page size if therer is still a
>- 1-page gap. */
>- desired_relro_base -= seg->maxpagesize;
>- relro_end -= seg->maxpagesize;
>- }
>- else
>- {
>- /* Align the relro segment. */
>- desired_relro_base = aligned_relro_base;
>- relro_end &= ~(seg->maxpagesize - 1);
>- }
>- }
>- }
>-
> seg->phase = exp_seg_relro_adjust;
>- ASSERT (desired_relro_base_reduced
>- || desired_relro_base >= seg->base);
>- seg->base = desired_relro_base;
>+ ASSERT (desired_end >= seg->base);
>+ seg->base = desired_end;
> return relro_end;
> }
>
>
>--
>Alan Modra
>Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] ld: Rewrite lang_size_relro_segment_1
2022-01-29 1:01 ` Alan Modra
2022-01-29 9:06 ` Fangrui Song
@ 2022-01-29 16:45 ` H.J. Lu
1 sibling, 0 replies; 20+ messages in thread
From: H.J. Lu @ 2022-01-29 16:45 UTC (permalink / raw)
To: Alan Modra; +Cc: Nick Clifton, Binutils
On Fri, Jan 28, 2022 at 5:01 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Wed, Jan 26, 2022 at 06:10:28PM -0800, H.J. Lu wrote:
> > On Wed, Jan 26, 2022 at 4:48 PM Alan Modra <amodra@gmail.com> wrote:
> > >
> > > On Wed, Jan 26, 2022 at 10:55:35AM +0000, Nick Clifton via Binutils wrote:
> > > > Hi H.J.
> > > >
> > > > > 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
> > > >
> > > > Looks good to me. Please apply. Branch and mainline.
> > >
> > > How was this patch tested? If it has not been tested on targets other
> > > than x86 then it is not appropriate for the branch. Instead, I
> >
> > What kinds of tests are you looking for? Do you have a testcase to show
> > there is still an issue?
>
> Can you answer my question?
My initial patch passes all tests in binutils, including cross binutils. But
tests didn't catch all the cases. For the second patch, I built
glibc for Linux
targets. Only ia64 glibc doesn't have GNU_RELRO since it doesn't support
GNU_RELRO.
> > > believe commit 2f83249c13d ought to be reverted on the branch. A
> > > wasted page in memory layout is not an important problem. We have
> > > more serious relro issues on targets other than x86. See pr28824.
> > >
> > > I'm speaking up because I raised concerns over the design of the
> > > previous patch, not just the implementation, and had those concerns
> > > overridden by Nick. Or possibly Nick didn't see my email before the
> > > previous patch was approved. Regardless of how it happened, I fear
> > > we might have even worse relro support in 2.38.
> > >
> > > FWIW, I also think that any fix I might develop for pr28824 will
> > > likely not be release branch quality until is has been tested for some
> > > time on mainline.
>
> This is the patch I'm playing with. It reverts your changes to
> lang_size_relro_segment_1 and fixes PR2844.
>
> The end of the PT_GNU_RELRO segment must be aligned to maxpagesize in
> order for the last page in the segment to be write protected after
> relocation. Unfortunately we currently only align to commonpagesize,
> which works for x86 but no other architecture that actually uses
> maxpagesize memory pages. I've kept the commonpage alignment for x86,
> other targets get maxpagesize.
x86 should also get maxpagesize. commonpagesize should be only
used to align the beginning of the PT_GNU_RELRO segment, not the
end of it. Since the PT_LOAD segment is aligned to maxpagesize, it
should be OK to use commonpagesize to align the PT_GNU_RELRO
segment which is inside of the PT_LOAD segment. But there is no
reason to have a 1-page gap before the PT_GNU_RELRO segment.
> There are of course testsuite changes that are needed, multiple x86
> and one s390x test. The only fail of any significance is pr18176, but
> there is a fundamental tension between pr18176 and pr28743. You
> either create holes or you waste disk space, both conditions can't be
> met. Worse IMO, the pr18176 testcase not only has a hole but also a
> gap in memory layout after the last section in the relro segment.
> That's not ideal for powerpc where sections after .got (the last relro
> section) are in some cases accessed via the got/toc pointer reg. On
> x86 you'd see a gap between .got and .got.plt when SEPARATE_GOTPLT.
>
> diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
> index da8a488db36..c9b3e7c4d9f 100644
> --- a/bfd/elfxx-x86.c
> +++ b/bfd/elfxx-x86.c
> @@ -4205,4 +4205,5 @@ _bfd_elf_linker_x86_set_options (struct bfd_link_info * info,
> = elf_x86_hash_table (info, bed->target_id);
> if (htab != NULL)
> htab->params = params;
> + info->relro_use_commonpagesize = true;
> }
> diff --git a/include/bfdlink.h b/include/bfdlink.h
> index 69fc9d33ff4..8d5561b0b28 100644
> --- a/include/bfdlink.h
> +++ b/include/bfdlink.h
> @@ -413,6 +413,10 @@ struct bfd_link_info
> /* TRUE if PT_GNU_RELRO segment should be created. */
> unsigned int relro: 1;
>
> + /* TRUE if the relro segment should be aligned to commonpagesize
> + rather than maxpagesize. */
> + unsigned int relro_use_commonpagesize : 1;
> +
> /* TRUE if DT_RELR should be enabled for compact relative
> relocations. */
> unsigned int enable_dt_relr: 1;
> diff --git a/ld/ldexp.c b/ld/ldexp.c
> index 5f904aaf8ae..65618912043 100644
> --- a/ld/ldexp.c
> +++ b/ld/ldexp.c
> @@ -469,7 +469,8 @@ fold_segment_align (seg_align_type *seg, etree_value_type *lhs)
> }
> else
> {
> - expld.result.value += expld.dot & (maxpage - 1);
> + if (!link_info.relro)
> + expld.result.value += expld.dot & (maxpage - 1);
> if (seg->phase == exp_seg_done)
> {
> /* OK. */
> @@ -480,6 +481,10 @@ fold_segment_align (seg_align_type *seg, etree_value_type *lhs)
> seg->base = expld.result.value;
> seg->pagesize = commonpage;
> seg->maxpagesize = maxpage;
> + if (link_info.relro_use_commonpagesize)
> + seg->relropagesize = commonpage;
> + else
> + seg->relropagesize = maxpage;
> seg->relro_end = 0;
> }
> else
> @@ -508,10 +513,10 @@ fold_segment_relro_end (seg_align_type *seg, etree_value_type *lhs)
> seg->relro_end = lhs->value + expld.result.value;
>
> if (seg->phase == exp_seg_relro_adjust
> - && (seg->relro_end & (seg->pagesize - 1)))
> + && (seg->relro_end & (seg->relropagesize - 1)))
> {
> - seg->relro_end += seg->pagesize - 1;
> - seg->relro_end &= ~(seg->pagesize - 1);
> + seg->relro_end += seg->relropagesize - 1;
> + seg->relro_end &= ~(seg->relropagesize - 1);
> expld.result.value = seg->relro_end - expld.result.value;
> }
> else
> diff --git a/ld/ldexp.h b/ld/ldexp.h
> index ac4fa7e82b0..f1701b66330 100644
> --- a/ld/ldexp.h
> +++ b/ld/ldexp.h
> @@ -136,7 +136,8 @@ enum relro_enum {
> typedef struct {
> enum phase_enum phase;
>
> - bfd_vma base, relro_offset, relro_end, end, pagesize, maxpagesize;
> + bfd_vma base, relro_offset, relro_end, end;
> + bfd_vma pagesize, maxpagesize, relropagesize;
>
> enum relro_enum relro;
>
> diff --git a/ld/ldlang.c b/ld/ldlang.c
> index 5dd3df12a0f..1a09b988fbe 100644
> --- a/ld/ldlang.c
> +++ b/ld/ldlang.c
> @@ -6370,101 +6370,40 @@ lang_size_segment (seg_align_type *seg)
> static bfd_vma
> lang_size_relro_segment_1 (seg_align_type *seg)
> {
> - 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;
> + bfd_vma relro_end, desired_end;
> + asection *sec;
>
> /* Compute the expected PT_GNU_RELRO/PT_LOAD segment end. */
> - relro_end = ((seg->relro_end + seg->pagesize - 1)
> - & ~(seg->pagesize - 1));
> + relro_end = (seg->relro_end + seg->relropagesize - 1) & -seg->relropagesize;
>
> /* Adjust by the offset arg of XXX_SEGMENT_RELRO_END. */
> - desired_relro_base = relro_end - seg->relro_offset;
> + desired_end = 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)
> + if ((sec->flags & SEC_ALLOC) != 0
> + && sec->vma >= seg->base
> + && sec->vma < seg->relro_end - seg->relro_offset)
> {
> - /* 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 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_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_relro_base = start;
> - relro_sec = sec;
> - seen_reloc_section = true;
> - }
> - else if (seen_reloc_section)
> - {
> - /* Stop searching if we see a non-relro section after seeing
> - relro sections. */
> - break;
> - }
> + /* Where do we want to put this section so that it ends as
> + desired? */
> + bfd_vma start, end, bump;
> +
> + end = start = sec->vma;
> + if (!IS_TBSS (sec))
> + end += TO_ADDR (sec->size);
> + bump = desired_end - 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;
> }
>
> - if (relro_sec != NULL
> - && seg->maxpagesize >= (1U << max_alignment_power))
> - {
> - asection *prev_sec;
> - bfd_vma prev_sec_end_plus_1_page;
> -
> - /* 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)
> - {
> - 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)
> - {
> - /* Subtract the maximum page size if therer is still a
> - 1-page gap. */
> - desired_relro_base -= seg->maxpagesize;
> - relro_end -= seg->maxpagesize;
> - }
> - else
> - {
> - /* Align the relro segment. */
> - desired_relro_base = aligned_relro_base;
> - relro_end &= ~(seg->maxpagesize - 1);
> - }
> - }
> - }
> -
> seg->phase = exp_seg_relro_adjust;
> - ASSERT (desired_relro_base_reduced
> - || desired_relro_base >= seg->base);
> - seg->base = desired_relro_base;
> + ASSERT (desired_end >= seg->base);
> + seg->base = desired_end;
> return relro_end;
> }
>
>
> --
> Alan Modra
> Australia Development Lab, IBM
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2022-01-29 16:46 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH v4] " H.J. Lu
2022-01-26 10:55 ` 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
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).