diff mbox series

elf: Add GNU_PROPERTY_NO_MEMORY_SEAL gnu property

Message ID 20240731200510.2270512-1-adhemerval.zanella@linaro.org
State New
Headers show
Series elf: Add GNU_PROPERTY_NO_MEMORY_SEAL gnu property | expand

Commit Message

Adhemerval Zanella Netto July 31, 2024, 8:05 p.m. UTC
On a glibc recent proposal [1] to add Linux mseal support [2],
Mike Hommey raised that this feature might potentially break Firefox
on Linux.  The issue is Firefox is built with DT_RELR support, and
post-processed with a tool to both remove the GLIBC_ABI_DT_RELR
dependency and instrument the binaries to apply the relocation
themselves so they can deploy Firefox regardless if loader supports
DT_RELR or not (some more details at [3]).

To accomplish it, the instrumentation mimics the
dynamic loader and temporarily undoes the RELRO machine to be
able to apply those relocations, and redoes it afterward.  And this
is exactly what mseal aims to prevent.

The GNU_PROPERTY_NO_MEMORY_SEAL gnu property is a way to mark such
objects are not sealed by glibc.  When linked with
-Wl,-z,no-memory-seal, glibc will not seal either the binary or
the shared library (the sealing will still be done by default, if
the kernel supports it).  The version 2 of glibc support for memory
sealing uses this new property [5].

[1] https://sourceware.org/pipermail/libc-alpha/2024-June/157359.html
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8be7258aad44b5e25977a98db136f677fa6f4370
[3] https://sourceware.org/pipermail/libc-alpha/2024-June/157668.html
[4] https://glandium.org/blog/?p=4297
[5] https://sourceware.org/pipermail/libc-alpha/2024-July/158804.html

Change-Id: Ibd799db05179332873e371fcc07f15a9bd949cb3
---
 bfd/elf-properties.c                  | 83 +++++++++++++++++++++------
 bfd/elfxx-x86.c                       |  3 +-
 binutils/readelf.c                    |  6 ++
 include/bfdlink.h                     |  3 +
 include/elf/common.h                  |  1 +
 ld/emultempl/elf.em                   |  2 +
 ld/ld.texi                            |  4 ++
 ld/ldlex.h                            |  1 +
 ld/lexsup.c                           |  2 +
 ld/testsuite/ld-elf/property-seal-1.d | 15 +++++
 ld/testsuite/ld-elf/property-seal-2.d | 15 +++++
 11 files changed, 117 insertions(+), 18 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/property-seal-1.d
 create mode 100644 ld/testsuite/ld-elf/property-seal-2.d

Comments

Florian Weimer July 31, 2024, 8:33 p.m. UTC | #1
* Adhemerval Zanella:

> On a glibc recent proposal [1] to add Linux mseal support [2],
> Mike Hommey raised that this feature might potentially break Firefox
> on Linux.  The issue is Firefox is built with DT_RELR support, and
> post-processed with a tool to both remove the GLIBC_ABI_DT_RELR
> dependency and instrument the binaries to apply the relocation
> themselves so they can deploy Firefox regardless if loader supports
> DT_RELR or not (some more details at [3]).
>
> To accomplish it, the instrumentation mimics the
> dynamic loader and temporarily undoes the RELRO machine to be
> able to apply those relocations, and redoes it afterward.  And this
> is exactly what mseal aims to prevent.
>
> The GNU_PROPERTY_NO_MEMORY_SEAL gnu property is a way to mark such
> objects are not sealed by glibc.  When linked with
> -Wl,-z,no-memory-seal, glibc will not seal either the binary or
> the shared library (the sealing will still be done by default, if
> the kernel supports it).  The version 2 of glibc support for memory
> sealing uses this new property [5].

Wouldn't it be easier to add a new flag in DT_GNU_FLAGS_1 (similar to
allocated, but not yet used in glibc DF_GNU_1_UNIQUE)?  This mechanism
seems to be rather heavyweight.

Thanks,
Florian
Adhemerval Zanella Netto Aug. 5, 2024, 5:19 p.m. UTC | #2
On 31/07/24 17:33, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On a glibc recent proposal [1] to add Linux mseal support [2],
>> Mike Hommey raised that this feature might potentially break Firefox
>> on Linux.  The issue is Firefox is built with DT_RELR support, and
>> post-processed with a tool to both remove the GLIBC_ABI_DT_RELR
>> dependency and instrument the binaries to apply the relocation
>> themselves so they can deploy Firefox regardless if loader supports
>> DT_RELR or not (some more details at [3]).
>>
>> To accomplish it, the instrumentation mimics the
>> dynamic loader and temporarily undoes the RELRO machine to be
>> able to apply those relocations, and redoes it afterward.  And this
>> is exactly what mseal aims to prevent.
>>
>> The GNU_PROPERTY_NO_MEMORY_SEAL gnu property is a way to mark such
>> objects are not sealed by glibc.  When linked with
>> -Wl,-z,no-memory-seal, glibc will not seal either the binary or
>> the shared library (the sealing will still be done by default, if
>> the kernel supports it).  The version 2 of glibc support for memory
>> sealing uses this new property [5].
> 
> Wouldn't it be easier to add a new flag in DT_GNU_FLAGS_1 (similar to
> allocated, but not yet used in glibc DF_GNU_1_UNIQUE)?  This mechanism
> seems to be rather heavyweight.

I don't have a strong opinion.  The .gnu.attributes seems to be the current
mechanism for current security sensitive that aims to be opt-in, such as
ARM BTI or Intel CET.  But I give you that GNU_PROPERTY_NO_MEMORY_SEAL seems
to fit more in a flag like DF_GNU_1_UNIQUE, so maybe DT_GNU_FLAGS_1 is a 
better option indeed.

But I am still not fully buying in the need for GNU_PROPERTY_NO_MEMORY_SEAL,
I added it as a way to give users to opt-out memory sealing for any reason
like the Firefox loader *bypass* (which I am also not sure if glibc should
really handle it a breakage). 

This option is essentially a security liability, since it defeats the whole
purpose of memory sealing. So I would like to hear if this is really a good
approach.
diff mbox series

Patch

diff --git a/bfd/elf-properties.c b/bfd/elf-properties.c
index ee8bd37f2bd..ed3e4752893 100644
--- a/bfd/elf-properties.c
+++ b/bfd/elf-properties.c
@@ -177,6 +177,20 @@  _bfd_elf_parse_gnu_properties (bfd *abfd, Elf_Internal_Note *note)
 	      prop->pr_kind = property_number;
 	      goto next;
 
+	    case GNU_PROPERTY_NO_MEMORY_SEAL:
+	      if (datasz != 0)
+		{
+		  _bfd_error_handler
+		    (_("warning: %pB: corrupt no memory sealing size: 0x%x"),
+		     abfd, datasz);
+		  /* Clear all properties.  */
+		  elf_properties (abfd) = NULL;
+		  return false;
+		}
+	      prop = _bfd_elf_get_property (abfd, type, datasz);
+	      prop->pr_kind = property_number;
+	      goto next;
+
 	    default:
 	      if ((type >= GNU_PROPERTY_UINT32_AND_LO
 		   && type <= GNU_PROPERTY_UINT32_AND_HI)
@@ -258,6 +272,9 @@  elf_merge_gnu_properties (struct bfd_link_info *info, bfd *abfd, bfd *bbfd,
 	 be added to ABFD.  */
       return aprop == NULL;
 
+    case GNU_PROPERTY_NO_MEMORY_SEAL:
+      return aprop == NULL;
+
     default:
       updated = false;
       if (pr_type >= GNU_PROPERTY_UINT32_OR_LO
@@ -607,6 +624,33 @@  elf_write_gnu_properties (struct bfd_link_info *info,
     }
 }
 
+static asection *
+_bfd_elf_link_create_gnu_property_sec (struct bfd_link_info *info, bfd *elf_bfd,
+				       unsigned int elfclass)
+{
+  asection *sec;
+
+  sec = bfd_make_section_with_flags (elf_bfd,
+				     NOTE_GNU_PROPERTY_SECTION_NAME,
+				     (SEC_ALLOC
+				      | SEC_LOAD
+				      | SEC_IN_MEMORY
+				      | SEC_READONLY
+				      | SEC_HAS_CONTENTS
+				      | SEC_DATA));
+  if (sec == NULL)
+    info->callbacks->einfo (_("%F%P: failed to create GNU property section\n"));
+
+  if (!bfd_set_section_alignment (sec,
+				  elfclass == ELFCLASS64 ? 3 : 2))
+    info->callbacks->einfo (_("%F%pA: failed to align section\n"),
+			    sec);
+
+  elf_section_type (sec) = SHT_NOTE;
+  return sec;
+}
+
+
 /* Set up GNU properties.  Return the first relocatable ELF input with
    GNU properties if found.  Otherwise, return NULL.  */
 
@@ -656,23 +700,7 @@  _bfd_elf_link_setup_gnu_properties (struct bfd_link_info *info)
       /* Support -z indirect-extern-access.  */
       if (first_pbfd == NULL)
 	{
-	  sec = bfd_make_section_with_flags (elf_bfd,
-					     NOTE_GNU_PROPERTY_SECTION_NAME,
-					     (SEC_ALLOC
-					      | SEC_LOAD
-					      | SEC_IN_MEMORY
-					      | SEC_READONLY
-					      | SEC_HAS_CONTENTS
-					      | SEC_DATA));
-	  if (sec == NULL)
-	    info->callbacks->einfo (_("%F%P: failed to create GNU property section\n"));
-
-	  if (!bfd_set_section_alignment (sec,
-					  elfclass == ELFCLASS64 ? 3 : 2))
-	    info->callbacks->einfo (_("%F%pA: failed to align section\n"),
-				    sec);
-
-	  elf_section_type (sec) = SHT_NOTE;
+	  sec = _bfd_elf_link_create_gnu_property_sec (info, elf_bfd, elfclass);
 	  first_pbfd = elf_bfd;
 	  has_properties = true;
 	}
@@ -690,6 +718,27 @@  _bfd_elf_link_setup_gnu_properties (struct bfd_link_info *info)
 	  |= GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS;
     }
 
+  if (info->no_memory_seal && elf_bfd != NULL)
+    {
+      /* Support -z no-memory-seal.  */
+      if (first_pbfd == NULL)
+	{
+	  sec = _bfd_elf_link_create_gnu_property_sec (info, elf_bfd, elfclass);
+	  first_pbfd = elf_bfd;
+	  has_properties = true;
+	}
+
+      p = _bfd_elf_get_property (first_pbfd, GNU_PROPERTY_NO_MEMORY_SEAL, 0);
+      if (p->pr_kind == property_unknown)
+	{
+	  /* Create GNU_PROPERTY_NO_MEMORY_SEAL.  */
+	  p->u.number = GNU_PROPERTY_NO_MEMORY_SEAL;
+	  p->pr_kind = property_number;
+	}
+      else
+	p->u.number |= GNU_PROPERTY_NO_MEMORY_SEAL;
+    }
+
   /* Do nothing if there is no .note.gnu.property section.  */
   if (!has_properties)
     return NULL;
diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
index 85737fc18b7..2639c58bc93 100644
--- a/bfd/elfxx-x86.c
+++ b/bfd/elfxx-x86.c
@@ -4813,7 +4813,8 @@  _bfd_x86_elf_link_fixup_gnu_properties
   for (p = *listp; p; p = p->next)
     {
       unsigned int type = p->property.pr_type;
-      if (type == GNU_PROPERTY_X86_COMPAT_ISA_1_USED
+      if (type == GNU_PROPERTY_NO_MEMORY_SEAL
+	  || type == GNU_PROPERTY_X86_COMPAT_ISA_1_USED
 	  || type == GNU_PROPERTY_X86_COMPAT_ISA_1_NEEDED
 	  || (type >= GNU_PROPERTY_X86_UINT32_AND_LO
 	      && type <= GNU_PROPERTY_X86_UINT32_AND_HI)
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 0f8dc1b9716..93cd1857271 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -21464,6 +21464,12 @@  print_gnu_property_note (Filedata * filedata, Elf_Internal_Note * pnote)
 		printf (_("<corrupt length: %#x> "), datasz);
 	      goto next;
 
+	    case GNU_PROPERTY_NO_MEMORY_SEAL:
+	      printf ("no memory sealing ");
+	      if (datasz)
+		printf (_("<corrupt length: %#x> "), datasz);
+	      goto next;
+
 	    default:
 	      if ((type >= GNU_PROPERTY_UINT32_AND_LO
 		   && type <= GNU_PROPERTY_UINT32_AND_HI)
diff --git a/include/bfdlink.h b/include/bfdlink.h
index f802ec627ef..4717b742afd 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -429,6 +429,9 @@  struct bfd_link_info
   /* TRUE if only one read-only, non-code segment should be created.  */
   unsigned int one_rosegment: 1;
 
+  /* TRUE if GNU_PROPERTY_NO_MEMORY_SEAL should be generated.  */
+  unsigned int no_memory_seal: 1;
+
   /* Nonzero if .eh_frame_hdr section and PT_GNU_EH_FRAME ELF segment
      should be created.  1 for DWARF2 tables, 2 for compact tables.  */
   unsigned int eh_frame_hdr_type: 2;
diff --git a/include/elf/common.h b/include/elf/common.h
index c9920e7731a..80851739e4d 100644
--- a/include/elf/common.h
+++ b/include/elf/common.h
@@ -890,6 +890,7 @@ 
 /* Values used in GNU .note.gnu.property notes (NT_GNU_PROPERTY_TYPE_0).  */
 #define GNU_PROPERTY_STACK_SIZE			1
 #define GNU_PROPERTY_NO_COPY_ON_PROTECTED	2
+#define GNU_PROPERTY_NO_MEMORY_SEAL		3
 
 /* A 4-byte unsigned integer property: A bit is set if it is set in all
    relocatable inputs.  */
diff --git a/ld/emultempl/elf.em b/ld/emultempl/elf.em
index 863657e12f5..b27288f2115 100644
--- a/ld/emultempl/elf.em
+++ b/ld/emultempl/elf.em
@@ -1030,6 +1030,8 @@  fragment <<EOF
 	config.no_section_header = false;
       else if (strcmp (optarg, "nosectionheader") == 0)
 	config.no_section_header = true;
+      else if (strcmp (optarg, "no-memory-seal") == 0)
+	link_info.no_memory_seal = true;
 EOF
 
 if test x"$GENERATE_SHLIB_SCRIPT" = xyes; then
diff --git a/ld/ld.texi b/ld/ld.texi
index 89e3913317a..f3b8a817b6b 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -1591,6 +1591,10 @@  Disable relocation overflow check.  This can be used to disable
 relocation overflow check if there will be no dynamic relocation
 overflow at run-time.  Supported for x86_64.
 
+@item no-memory-seal
+Generate GNU_PROPERTY_NO_MEMORY_SEAL in .note.gnu.property section to
+indicate that object file requires no memory sealing.
+
 @item now
 When generating an executable or shared library, mark it to tell the
 dynamic linker to resolve all symbols when the program is started, or
diff --git a/ld/ldlex.h b/ld/ldlex.h
index defe3fcbbb9..8f7bb1254b2 100644
--- a/ld/ldlex.h
+++ b/ld/ldlex.h
@@ -289,6 +289,7 @@  enum option_values
   OPTION_COMPRESS_DEBUG,
   OPTION_ROSEGMENT,
   OPTION_NO_ROSEGMENT,
+  OPTION_NOMEMORYSEAL,
   /* Used by emultempl/hppaelf.em.  */
   OPTION_MULTI_SUBSPACE,
   /* Used by emultempl/ia64elf.em.  */
diff --git a/ld/lexsup.c b/ld/lexsup.c
index 4aa0124ce2f..d9d6ba73081 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -2271,6 +2271,8 @@  elf_shlib_list_options (FILE *file)
       fprintf (file, _("\
   -z textoff                  Don't treat DT_TEXTREL in output as error\n"));
     }
+  fprintf (file, _("\
+  -z no-memory-seal           Generate GNU_PROPERTY_NO_MEMORY_SEAL\n"));
 }
 
 static void
diff --git a/ld/testsuite/ld-elf/property-seal-1.d b/ld/testsuite/ld-elf/property-seal-1.d
new file mode 100644
index 00000000000..9a0809256a7
--- /dev/null
+++ b/ld/testsuite/ld-elf/property-seal-1.d
@@ -0,0 +1,15 @@ 
+#source: empty.s
+#ld: -shared -z no-memory-seal
+#readelf: -n
+#xfail: ![check_shared_lib_support]
+#notarget: am33_2.0-*-* hppa*-*-hpux* mn10300-*-*
+# Assembly source file for the HPPA assembler is renamed and modifed by
+# sed.  mn10300 has relocations in .note.gnu.property section which
+# elf_parse_notes doesn't support.
+
+#...
+Displaying notes found in: .note.gnu.property
+[ 	]+Owner[ 	]+Data size[ 	]+Description
+  GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
+      Properties: no memory sealing 
+#pass
diff --git a/ld/testsuite/ld-elf/property-seal-2.d b/ld/testsuite/ld-elf/property-seal-2.d
new file mode 100644
index 00000000000..39caef4442d
--- /dev/null
+++ b/ld/testsuite/ld-elf/property-seal-2.d
@@ -0,0 +1,15 @@ 
+#source: empty.s
+#ld: -z no-memory-seal
+#readelf: -n
+#xfail: ![check_shared_lib_support]
+#notarget: am33_2.0-*-* hppa*-*-hpux* mn10300-*-*
+# Assembly source file for the HPPA assembler is renamed and modifed by
+# sed.  mn10300 has relocations in .note.gnu.property section which
+# elf_parse_notes doesn't support.
+
+#...
+Displaying notes found in: .note.gnu.property
+[ 	]+Owner[ 	]+Data size[ 	]+Description
+  GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
+      Properties: no memory sealing 
+#pass