From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x62f.google.com (mail-pl1-x62f.google.com [IPv6:2607:f8b0:4864:20::62f]) by sourceware.org (Postfix) with ESMTPS id 2275A3857C7E for ; Tue, 25 Jan 2022 15:05:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2275A3857C7E Received: by mail-pl1-x62f.google.com with SMTP id x11so13780394plg.6 for ; Tue, 25 Jan 2022 07:05:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=SJOvzdieTsE2ZnDUVoDnR1R4GELmJ6iJXOIJ7udEAgA=; b=dt1jYYTWt7g7NNKLJ4IAorrssAUc2YuZiZRqY6JuNgI1esbwx7VBJKypMz80ankkzn SzuX5cS1dGh/BDwqaUbm9SPn2sYoyccMnO+pYmbmO3q60zpl0e9AfjRdFHaz1qVHQyd8 kY5C6IAMYiVPpl2TzsfB6MPEj46V6zxOU9yWldgLzHDWtINWbW32Tu+CgL9oJT41Qi2x cfd7yXu/ajdMeJ4o8U9JPTiWTDkwhU6xyC+ofbxLVzgRTGPIxTcXeonv3HYX7ReO3Qk1 nI15QRFSJErZSENfiSkFZcBQ7sDvY2y7pMugmAyZ2svO2n8Z7p7CLKRYsXmGfyKatZ7h gGSA== X-Gm-Message-State: AOAM530NV+MTsE7I+SvM0pScPXvsy584yedAS0gqdU12J3pS7BiDzOu9 u6oXLNYxs7d9mmOGRar4omxBeqHKpOY= X-Google-Smtp-Source: ABdhPJzaQa1KTV/alnJrwzq3G4Eh/LEjG2hG1DqEWHSHJWQU0vq5tEdAFGyV8Ghlwm4wkQbRNjAxYQ== X-Received: by 2002:a17:90b:38c1:: with SMTP id nn1mr3965767pjb.56.1643123127832; Tue, 25 Jan 2022 07:05:27 -0800 (PST) Received: from gnu-tgl-3.localdomain ([172.58.35.133]) by smtp.gmail.com with ESMTPSA id d70sm800715pga.48.2022.01.25.07.05.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Jan 2022 07:05:27 -0800 (PST) Received: by gnu-tgl-3.localdomain (Postfix, from userid 1000) id 551D4C0730; Tue, 25 Jan 2022 07:05:25 -0800 (PST) Date: Tue, 25 Jan 2022 07:05:25 -0800 From: "H.J. Lu" To: Nick Clifton , Binutils Subject: [PATCH v4] ld: Rewrite lang_size_relro_segment_1 Message-ID: References: <20220111021241.1937265-1-hjl.tools@gmail.com> <5448901b-167d-4423-c99e-557d8178e56d@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Spam-Status: No, score=-3028.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_NUMSUBJECT, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Jan 2022 15:05:33 -0000 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