public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Alan Modra <amodra@gmail.com>
To: binutils@sourceware.org
Subject: [PATCH 1/4] PR28824, relro security issues
Date: Tue,  8 Feb 2022 11:38:30 +1030	[thread overview]
Message-ID: <20220208010833.2103874-2-amodra@gmail.com> (raw)
In-Reply-To: <20220208010833.2103874-1-amodra@gmail.com>

Background
==========
There are constraints on layout of binaries to meet demand paging and
memory protection requirements.  Demand paged binaries must have file
offset mod pagesize equal to vma mod pagesize.  Memory protection
(executable, read, write status) can only change at page boundaries.
The linker's MAXPAGESIZE variable gives the page size for these layout
constraints.

In a typical basic executable with two memory segments, text (RE) and
data (RW), the data segment must start on a different page to the
last text segment page.  For example, with 64k pages and a small
executable of 48k text and 1k data, the text segment might start at
address 0x10000 and data at 0x20000 for a total of two 64k memory
pages.  Demand paging would require the image on disk to be 64k+1k
in size.  We can do better than that.  If the data segment instead
starts at 0x2c000 (the end of the text segment plus one 64k page) then
there are still only two memory pages, but the disk image is now
smaller, 48k+1k in size.  This is why the linker normally starts the
data segment at the end of the text segment plus one page.  That
simple heuristic isn't ideal in all cases.  Changing our simple
example to one with 64k-1 text size, following that heuristic would
result in data starting at 0x2ffff.  Now we have two 64k memory data
pages for a data segment of 1k!  If the data segment instead started
at 0x30000 we'd get a single data segment page at the cost of 1 byte
extra in the disk image, which is likely a good trade-off.  So the
linker does adjust the simple heuristic.  Just how much disk image
size increase is allowed is controlled by the linker's COMMONPAGESIZE
variable.

A PT_GNU_RELRO segment overlays the initial part of the data segment,
saying that those pages should be made read-only after relocation by
the dynamic loader.  Page granularity for memory protection means that
the end of the relro segment must be at a page boundary.

The problem
===========
Unfortunately most targets currently only align the end of the relro
segment to COMMONPAGESIZE.  That results in only partial relro
protection if an executable is running with MAXPAGESIZE pages, since
any part of the relro segment past the last MAXPAGESIZE boundary can't
be made read-only without also affecting sections past the end of the
relro segment.  I believe this problem arose because x86 always runs
with 4k (COMMPAGESIZE) memory pages, and therefore using a larger
MAXPAGESIZE on x86 is for reasons other than the demand paging and
memory page protection boundary requirements.

The solution
============
Always end the relro segment on a MAXPAGESIZE boundary, except for
x86.  Note that the relro segment, comprising of sections at the start
of the data segment, is sized according to how those sections are laid
out.  That means the start of the relro segment is fixed relative to
its end.  Which also means the start of the data segment must be at a
fixed address mod MAXPAGESIZE.  So for relro the linker can't play
games with the start of the data segment to save disk space.  At
least, not without introducing gaps between the relro sections.  In
fact, because the linker was starting layout using its simple
heuristic of starting the data segment at the end of the text segment
plus one page, it was sometimes introducing page gaps for no reason.
See pr28743.

	PR 28824
	PR 28734
	* ldexp.c (fold_segment_align): When relro, don't adjust up by
	offset within page.  Set relropagesize.
	(fold_segment_relro_end): Align to relropagesize.
	* ldexp.h (seg_align_type): Rename pagesize to commonpagesize.
	Add relropagesize.  Comment.
	* ldlang.c (lang_size_segment): Adjust to suit field renaming.
	(lang_size_relro_segment_1): Align relro_end using relropagesize.

diff --git a/ld/ldexp.c b/ld/ldexp.c
index 5f904aaf8ae..a38cec7829d 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.  */
@@ -478,8 +479,9 @@ fold_segment_align (seg_align_type *seg, etree_value_type *lhs)
 	    {
 	      seg->phase = exp_seg_align_seen;
 	      seg->base = expld.result.value;
-	      seg->pagesize = commonpage;
+	      seg->commonpagesize = commonpage;
 	      seg->maxpagesize = maxpage;
+	      seg->relropagesize = maxpage;
 	      seg->relro_end = 0;
 	    }
 	  else
@@ -508,10 +510,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..ed6fb8be715 100644
--- a/ld/ldexp.h
+++ b/ld/ldexp.h
@@ -136,7 +136,10 @@ 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;
+  /* MAXPAGESIZE and COMMMONPAGESIZE as passed to DATA_SEGMENT_ALIGN.
+     relropagesize sets the alignment of the end of the relro segment.  */
+  bfd_vma maxpagesize, commonpagesize, relropagesize;
 
   enum relro_enum relro;
 
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 84511c4d615..f481586a7ba 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6352,12 +6352,12 @@ lang_size_segment (seg_align_type *seg)
      a page could be saved in the data segment.  */
   bfd_vma first, last;
 
-  first = -seg->base & (seg->pagesize - 1);
-  last = seg->end & (seg->pagesize - 1);
+  first = -seg->base & (seg->commonpagesize - 1);
+  last = seg->end & (seg->commonpagesize - 1);
   if (first && last
-      && ((seg->base & ~(seg->pagesize - 1))
-	  != (seg->end & ~(seg->pagesize - 1)))
-      && first + last <= seg->pagesize)
+      && ((seg->base & ~(seg->commonpagesize - 1))
+	  != (seg->end & ~(seg->commonpagesize - 1)))
+      && first + last <= seg->commonpagesize)
     {
       seg->phase = exp_seg_adjust;
       return true;
@@ -6374,8 +6374,7 @@ lang_size_relro_segment_1 (seg_align_type *seg)
   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_end = relro_end - seg->relro_offset;

  reply	other threads:[~2022-02-08  1:09 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-08  1:08 [PATCH 0/4] " Alan Modra
2022-02-08  1:08 ` Alan Modra [this message]
2022-02-08  1:08 ` [PATCH 2/4] PR28824, relro security issues, x86 keep COMMONPAGESIZE relro Alan Modra
2022-02-14  2:18   ` Alan Modra
2022-02-14  3:13     ` H.J. Lu
2022-02-08  1:08 ` [PATCH 3/4] Remove bfd ELF_RELROPAGESIZE Alan Modra
2022-02-08  1:08 ` [PATCH 4/4] Don't pass around expld.dataseg pointer Alan Modra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220208010833.2103874-2-amodra@gmail.com \
    --to=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).