[PATCH 0/4] PR28824, relro security issues

[PATCH 0/4] PR28824, relro security issues

[Alan Modra]

The first two patches in this series fix relro protection for all
targets that might be running with MAXPAGESIZE larger than
COMMONPAGESIZE memory pages.  They also fix pr28734, the extra page
gap between text and data segments when relro.

See the first patch for lots of detail on what is going on.  I'll
leave this series a few days before committing in case someone spots a
bug or has constructive comments.

Alan Modra (4):
  PR28824, relro security issues
  PR28824, relro security issues, x86 keep COMMONPAGESIZE relro
  Remove bfd ELF_RELROPAGESIZE
  Don't pass around expld.dataseg pointer

 bfd/bfd-in2.h                    |  2 +-
 bfd/bfd.c                        |  9 +++----
 bfd/elf-bfd.h                    |  3 ---
 bfd/elf32-ppc.c                  |  1 -
 bfd/elf64-ppc.c                  |  1 -
 bfd/elfxx-target.h               | 11 --------
 ld/emultempl/elf-x86.em          |  1 +
 ld/ld.h                          |  4 +++
 ld/ldemul.c                      |  3 +--
 ld/ldexp.c                       | 33 ++++++++++++++++--------
 ld/ldexp.h                       |  5 +++-
 ld/ldlang.c                      | 43 ++++++++++++++++----------------
 ld/testsuite/ld-x86-64/pr18176.d |  1 +
 13 files changed, 59 insertions(+), 58 deletions(-)

[PATCH 1/4] PR28824, relro security issues

[Alan Modra]

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;

[PATCH 2/4] PR28824, relro security issues, x86 keep COMMONPAGESIZE relro

[Alan Modra]

x86 treats MAXPAGESIZE as a memory optimisation parameter, actual
hardware paging is always COMMPAGESIZE of 4k.  Use COMMONPAGESIZE for
the end of the relro segment alignment.

The previous patch regresses pr18176, increasing the testcase file
size from 322208 to 2099872 bytes.  Fixing this on x86 will require
introducing a gap after the end of the relro segment (of up to
relropagesize-1 bytes).

	PR 28824
	PR 18176
	* ld.h (ld_config_type): Add relro_use_commonpagesize field.
	* ldexp.c (fold_segment_align): Set relropagesize depending on
	relro_use_commonpagesize.
	* emultempl/elf-x86.em (elf_x86_create_output_section_statements):
	Set relro_use_commonpagesize.
	* testsuite/ld-x86-64/pr18176.d: xfail.

diff --git a/ld/emultempl/elf-x86.em b/ld/emultempl/elf-x86.em
index f75521cecea..134e4e1b616 100644
--- a/ld/emultempl/elf-x86.em
+++ b/ld/emultempl/elf-x86.em
@@ -33,6 +33,7 @@ static struct elf_linker_x86_params params;
 static void
 elf_x86_create_output_section_statements (void)
 {
+  config.relro_use_commonpagesize = true;
   _bfd_elf_linker_x86_set_options (&link_info, &params);
 }
 
diff --git a/ld/ld.h b/ld/ld.h
index f3086bf30de..c7e4ca3d334 100644
--- a/ld/ld.h
+++ b/ld/ld.h
@@ -276,6 +276,10 @@ typedef struct
   /* If set, code and non-code sections should never be in one segment.  */
   bool separate_code;
 
+  /* TRUE if the end of the relro segment should be aligned to
+     COMMONPAGESIZE rather than MAXPAGESIZE.  */
+  bool relro_use_commonpagesize;
+
   /* The rpath separation character.  Usually ':'.  */
   char rpath_separator;
 
diff --git a/ld/ldexp.c b/ld/ldexp.c
index a38cec7829d..ab724074732 100644
--- a/ld/ldexp.c
+++ b/ld/ldexp.c
@@ -481,7 +481,10 @@ fold_segment_align (seg_align_type *seg, etree_value_type *lhs)
 	      seg->base = expld.result.value;
 	      seg->commonpagesize = commonpage;
 	      seg->maxpagesize = maxpage;
-	      seg->relropagesize = maxpage;
+	      if (config.relro_use_commonpagesize)
+		seg->relropagesize = commonpage;
+	      else
+		seg->relropagesize = maxpage;
 	      seg->relro_end = 0;
 	    }
 	  else
diff --git a/ld/testsuite/ld-x86-64/pr18176.d b/ld/testsuite/ld-x86-64/pr18176.d
index a99ff15ac6b..728c15a3dd8 100644
--- a/ld/testsuite/ld-x86-64/pr18176.d
+++ b/ld/testsuite/ld-x86-64/pr18176.d
@@ -3,6 +3,7 @@
 #ld: -melf_x86_64 -shared -z relro -T pr18176.t -z max-page-size=0x200000 -z common-page-size=0x1000 $NO_DT_RELR_LDFLAGS
 #readelf: -l --wide
 #target: x86_64-*-linux*
+#xfail: *-*-*
 
 #...
   GNU_RELRO      0x04bd17 0x000000000024bd17 0x000000000024bd17 0x0022e9 0x0022e9 R   0x1

[PATCH 3/4] Remove bfd ELF_RELROPAGESIZE

[Alan Modra]

Now that ld properly aligns the end of the relro segment, the hack to
make relro work on powerpc can disappear.

bfd/
	* bfd.c (bfd_emul_get_commonpagesize): Remove relro param.
	Don't return bed->relropagesize.
	* elf-bfd.h (struct elf_backend_data): Remove relropagesize.
	* elfxx-target.h (ELF_RELROPAGESIZE): Remove.
	* elf32-ppc.c (ELF_RELROPAGESIZE): Don't define.
	* elf64-ppc.c: Likewise.
	* bfd-in2.h: Regenerate.
ld/
	* ldemul.c (after_parse_default): Adjust
	bfd_emul_get_commonpagesize call.

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 8e815bab624..3b2a4f49a9b 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -7273,7 +7273,7 @@ bool bfd_alt_mach_code (bfd *abfd, int alternative);
 
 bfd_vma bfd_emul_get_maxpagesize (const char *);
 
-bfd_vma bfd_emul_get_commonpagesize (const char *, bool);
+bfd_vma bfd_emul_get_commonpagesize (const char *);
 
 char *bfd_demangle (bfd *, const char *, int);
 
diff --git a/bfd/bfd.c b/bfd/bfd.c
index a2f294da983..9b82353f573 100644
--- a/bfd/bfd.c
+++ b/bfd/bfd.c
@@ -2344,7 +2344,7 @@ FUNCTION
 	bfd_emul_get_commonpagesize
 
 SYNOPSIS
-	bfd_vma bfd_emul_get_commonpagesize (const char *, bool);
+	bfd_vma bfd_emul_get_commonpagesize (const char *);
 
 DESCRIPTION
 	Returns the common page size, in bytes, as determined by
@@ -2355,7 +2355,7 @@ RETURNS
 */
 
 bfd_vma
-bfd_emul_get_commonpagesize (const char *emul, bool relro)
+bfd_emul_get_commonpagesize (const char *emul)
 {
   const bfd_target *target;
 
@@ -2366,10 +2366,7 @@ bfd_emul_get_commonpagesize (const char *emul, bool relro)
       const struct elf_backend_data *bed;
 
       bed = xvec_get_elf_backend_data (target);
-      if (relro)
-	return bed->relropagesize;
-      else
-	return bed->commonpagesize;
+      return bed->commonpagesize;
     }
   return 0;
 }
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 1d3ae76339a..889a4746371 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -946,9 +946,6 @@ struct elf_backend_data
   /* The common page size for this backend.  */
   bfd_vma commonpagesize;
 
-  /* The value of commonpagesize to use when -z relro for this backend.  */
-  bfd_vma relropagesize;
-
   /* The p_align value for this backend.  If it is set, p_align of
       PT_LOAD alignment will be to p_align by default.  */
   bfd_vma p_align;
diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index 53051308eb1..42273ca06fe 100644
--- a/bfd/elf32-ppc.c
+++ b/bfd/elf32-ppc.c
@@ -10373,7 +10373,6 @@ ppc_elf_finish_dynamic_sections (bfd *output_bfd,
 #define ELF_MACHINE_CODE	EM_PPC
 #define ELF_MAXPAGESIZE		0x10000
 #define ELF_COMMONPAGESIZE	0x1000
-#define ELF_RELROPAGESIZE	ELF_MAXPAGESIZE
 #define elf_info_to_howto	ppc_elf_info_to_howto
 
 #ifdef  EM_CYGNUS_POWERPC
diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index 7223c497d07..f9c808dd294 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -75,7 +75,6 @@ static bfd_vma opd_entry_value
 #define ELF_MACHINE_CODE	EM_PPC64
 #define ELF_MAXPAGESIZE		0x10000
 #define ELF_COMMONPAGESIZE	0x1000
-#define ELF_RELROPAGESIZE	ELF_MAXPAGESIZE
 #define elf_info_to_howto	ppc64_elf_info_to_howto
 
 #define elf_backend_want_got_sym 0
diff --git a/bfd/elfxx-target.h b/bfd/elfxx-target.h
index e31985ef777..0579f64d1a0 100644
--- a/bfd/elfxx-target.h
+++ b/bfd/elfxx-target.h
@@ -379,10 +379,6 @@
 #define ELF_COMMONPAGESIZE ELF_MAXPAGESIZE
 #endif
 
-#ifndef ELF_RELROPAGESIZE
-#define ELF_RELROPAGESIZE ELF_COMMONPAGESIZE
-#endif
-
 #ifndef ELF_MINPAGESIZE
 #define ELF_MINPAGESIZE ELF_COMMONPAGESIZE
 #endif
@@ -390,15 +386,9 @@
 #if ELF_COMMONPAGESIZE > ELF_MAXPAGESIZE
 # error ELF_COMMONPAGESIZE > ELF_MAXPAGESIZE
 #endif
-#if ELF_RELROPAGESIZE > ELF_MAXPAGESIZE
-# error ELF_RELROPAGESIZE > ELF_MAXPAGESIZE
-#endif
 #if ELF_MINPAGESIZE > ELF_COMMONPAGESIZE
 # error ELF_MINPAGESIZE > ELF_COMMONPAGESIZE
 #endif
-#if ELF_MINPAGESIZE > ELF_RELROPAGESIZE
-# error ELF_MINPAGESIZE > ELF_RELROPAGESIZE
-#endif
 
 #ifndef ELF_P_ALIGN
 #define ELF_P_ALIGN 0
@@ -822,7 +812,6 @@ static const struct elf_backend_data elfNN_bed =
   ELF_MAXPAGESIZE,		/* maxpagesize */
   ELF_MINPAGESIZE,		/* minpagesize */
   ELF_COMMONPAGESIZE,		/* commonpagesize */
-  ELF_RELROPAGESIZE,		/* commonpagesize to use with -z relro */
   ELF_P_ALIGN,			/* p_align */
   ELF_DYNAMIC_SEC_FLAGS,	/* dynamic_sec_flags */
   elf_backend_arch_data,
diff --git a/ld/ldemul.c b/ld/ldemul.c
index 5c5adef06bb..85c00ded75e 100644
--- a/ld/ldemul.c
+++ b/ld/ldemul.c
@@ -235,8 +235,7 @@ after_parse_default (void)
   if (link_info.maxpagesize == 0)
     link_info.maxpagesize = bfd_emul_get_maxpagesize (default_target);
   if (link_info.commonpagesize == 0)
-    link_info.commonpagesize = bfd_emul_get_commonpagesize (default_target,
-							    link_info.relro);
+    link_info.commonpagesize = bfd_emul_get_commonpagesize (default_target);
 }
 
 void

[PATCH 4/4] Don't pass around expld.dataseg pointer

[Alan Modra]

The better to see any code that accesses expld.dataseg.

	* ldexp.c (fold_segment_end): Remove seg parameter.  Adjust calls.
	(fold_segment_align, fold_segment_relro_end): Likewise.
	* ldlang.c (lang_size_segment): Likewise.
	(lang_size_relro_segment_1, lang_find_relro_sections_1): Likewise.

diff --git a/ld/ldexp.c b/ld/ldexp.c
index ab724074732..90760e0a8ca 100644
--- a/ld/ldexp.c
+++ b/ld/ldexp.c
@@ -340,8 +340,10 @@ update_definedness (const char *name, struct bfd_link_hash_entry *h)
 }
 
 static void
-fold_segment_end (seg_align_type *seg)
+fold_segment_end (void)
 {
+  seg_align_type *seg = &expld.dataseg;
+
   if (expld.phase == lang_first_phase_enum
       || expld.section != bfd_abs_section_ptr)
     {
@@ -410,7 +412,7 @@ fold_unary (etree_type *tree)
 	  break;
 
 	case DATA_SEGMENT_END:
-	  fold_segment_end (&expld.dataseg);
+	  fold_segment_end ();
 	  break;
 
 	default:
@@ -447,8 +449,10 @@ arith_result_section (const etree_value_type *lhs)
 }
 
 static void
-fold_segment_align (seg_align_type *seg, etree_value_type *lhs)
+fold_segment_align (etree_value_type *lhs)
 {
+  seg_align_type *seg = &expld.dataseg;
+
   seg->relro = exp_seg_relro_start;
   if (expld.phase == lang_first_phase_enum
       || expld.section != bfd_abs_section_ptr)
@@ -494,8 +498,10 @@ fold_segment_align (seg_align_type *seg, etree_value_type *lhs)
 }
 
 static void
-fold_segment_relro_end (seg_align_type *seg, etree_value_type *lhs)
+fold_segment_relro_end (etree_value_type *lhs)
 {
+  seg_align_type *seg = &expld.dataseg;
+
   /* Operands swapped!  XXX_SEGMENT_RELRO_END(offset,exp) has offset
      in expld.result and exp in lhs.  */
   seg->relro = exp_seg_relro_end;
@@ -662,11 +668,11 @@ fold_binary (etree_type *tree)
 	  break;
 
 	case DATA_SEGMENT_ALIGN:
-	  fold_segment_align (&expld.dataseg, &lhs);
+	  fold_segment_align (&lhs);
 	  break;
 
 	case DATA_SEGMENT_RELRO_END:
-	  fold_segment_relro_end (&expld.dataseg, &lhs);
+	  fold_segment_relro_end (&lhs);
 	  break;
 
 	default:
diff --git a/ld/ldlang.c b/ld/ldlang.c
index f481586a7ba..474784c874a 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -5665,9 +5665,10 @@ os_region_check (lang_output_section_statement_type *os,
 }
 
 static void
-ldlang_check_relro_region (lang_statement_union_type *s,
-			   seg_align_type *seg)
+ldlang_check_relro_region (lang_statement_union_type *s)
 {
+  seg_align_type *seg = &expld.dataseg;
+
   if (seg->relro == exp_seg_relro_start)
     {
       if (!seg->relro_start_stat)
@@ -6166,7 +6167,7 @@ lang_size_sections_1
 			   output_section_statement->bfd_section,
 			   &newdot);
 
-	    ldlang_check_relro_region (s, &expld.dataseg);
+	    ldlang_check_relro_region (s);
 
 	    expld.dataseg.relro = exp_seg_relro_none;
 
@@ -6346,10 +6347,11 @@ one_lang_size_sections_pass (bool *relax, bool check_regions)
 }
 
 static bool
-lang_size_segment (seg_align_type *seg)
+lang_size_segment (void)
 {
   /* If XXX_SEGMENT_ALIGN XXX_SEGMENT_END pair was seen, check whether
      a page could be saved in the data segment.  */
+  seg_align_type *seg = &expld.dataseg;
   bfd_vma first, last;
 
   first = -seg->base & (seg->commonpagesize - 1);
@@ -6368,8 +6370,9 @@ lang_size_segment (seg_align_type *seg)
 }
 
 static bfd_vma
-lang_size_relro_segment_1 (seg_align_type *seg)
+lang_size_relro_segment_1 (void)
 {
+  seg_align_type *seg = &expld.dataseg;
   bfd_vma relro_end, desired_end;
   asection *sec;
 
@@ -6415,7 +6418,7 @@ lang_size_relro_segment (bool *relax, bool check_regions)
   if (link_info.relro && expld.dataseg.relro_end)
     {
       bfd_vma data_initial_base = expld.dataseg.base;
-      bfd_vma data_relro_end = lang_size_relro_segment_1 (&expld.dataseg);
+      bfd_vma data_relro_end = lang_size_relro_segment_1 ();
 
       lang_reset_memory_regions ();
       one_lang_size_sections_pass (relax, check_regions);
@@ -6424,11 +6427,11 @@ lang_size_relro_segment (bool *relax, bool check_regions)
 	 script have increased padding over the original.  Revert.  */
       if (expld.dataseg.relro_end > data_relro_end)
 	{
-	  expld.dataseg.base = data_initial_base;;
+	  expld.dataseg.base = data_initial_base;
 	  do_reset = true;
 	}
     }
-  else if (lang_size_segment (&expld.dataseg))
+  else if (lang_size_segment ())
     do_reset = true;
 
   return do_reset;
@@ -7679,7 +7682,6 @@ find_relro_section_callback (lang_wild_statement_type *ptr ATTRIBUTE_UNUSED,
 
 static void
 lang_find_relro_sections_1 (lang_statement_union_type *s,
-			    seg_align_type *seg,
 			    bool *has_relro_section)
 {
   if (*has_relro_section)
@@ -7687,7 +7689,7 @@ lang_find_relro_sections_1 (lang_statement_union_type *s,
 
   for (; s != NULL; s = s->header.next)
     {
-      if (s == seg->relro_end_stat)
+      if (s == expld.dataseg.relro_end_stat)
 	break;
 
       switch (s->header.type)
@@ -7699,15 +7701,15 @@ lang_find_relro_sections_1 (lang_statement_union_type *s,
 	  break;
 	case lang_constructors_statement_enum:
 	  lang_find_relro_sections_1 (constructor_list.head,
-				      seg, has_relro_section);
+				      has_relro_section);
 	  break;
 	case lang_output_section_statement_enum:
 	  lang_find_relro_sections_1 (s->output_section_statement.children.head,
-				      seg, has_relro_section);
+				      has_relro_section);
 	  break;
 	case lang_group_statement_enum:
 	  lang_find_relro_sections_1 (s->group_statement.children.head,
-				      seg, has_relro_section);
+				      has_relro_section);
 	  break;
 	default:
 	  break;
@@ -7723,7 +7725,7 @@ lang_find_relro_sections (void)
   /* Check all sections in the link script.  */
 
   lang_find_relro_sections_1 (expld.dataseg.relro_start_stat,
-			      &expld.dataseg, &has_relro_section);
+			      &has_relro_section);
 
   if (!has_relro_section)
     link_info.relro = false;

Re: [PATCH 2/4] PR28824, relro security issues, x86 keep COMMONPAGESIZE relro

[Alan Modra]

On Tue, Feb 08, 2022 at 11:38:31AM +1030, Alan Modra wrote:
> x86 treats MAXPAGESIZE as a memory optimisation parameter, actual
> hardware paging is always COMMPAGESIZE of 4k.  Use COMMONPAGESIZE for
> the end of the relro segment alignment.

HJ, you may like to revert this patch and make x86 also align the end
of the relro segment to MAXPAGESIZE.  That's why I extracted this part
into a separate patch.  I'm not anywhere near 100% sure about my claim
that x86 always uses (or can use) 4k hardware memory pages.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/4] PR28824, relro security issues, x86 keep COMMONPAGESIZE relro

[H.J. Lu]

On Sun, Feb 13, 2022 at 6:18 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Tue, Feb 08, 2022 at 11:38:31AM +1030, Alan Modra wrote:
> > x86 treats MAXPAGESIZE as a memory optimisation parameter, actual
> > hardware paging is always COMMPAGESIZE of 4k.  Use COMMONPAGESIZE for
> > the end of the relro segment alignment.
>
> HJ, you may like to revert this patch and make x86 also align the end
> of the relro segment to MAXPAGESIZE.  That's why I extracted this part
> into a separate patch.  I'm not anywhere near 100% sure about my claim
> that x86 always uses (or can use) 4k hardware memory pages.

I will take a look tomorrow.

Thanks.

-- 
H.J.