[RFC 0/1] elf: mseal non-writable segments

[RFC 0/1] elf: mseal non-writable segments

[Stephen Roettger]

Hi all,

I'm trying to implement mseal support in the runtime loader and would
like to get some feedback on the approach.

Mseal is an upcoming syscall in Linux [0] similar to OpenBSD's
mimmutable. In short, it prevents changes to sealed mappings during the
process lifetime, for example unmapping or permission changes.

I would like to add support to the runtime loader to automatically seal
mappings if possible, in particular code and read-only data. I wrote a
simple prototype and did some basic testing on a non-graphical debian.
It currently seals any PT_LOAD segments if:
* the writable bit is not set
* the mode is RTLD_NODELETE
(TODO: I need to add support for GNU_RELRO)

And in addition, I added RTLD_NODELETE in a few places so that more
objects get sealed:
* when loading the main binary and libraries
* I added propagation for RTLD_NODELETE for any auxiliary libraries

In my basic testing, this seems to work fine. But a few questions that
I'd like some feedback on:
* Does it sound ok to apply sealing by default? Should this be a flag in
  the ELF, e.g. maybe the p_flags could have a sealable bit?
* Does it make sense to piggyback on the RTLD_NODELETE bit and apply it
  to more objects? It seems to have the right semantics: the object
  should never get deleted => it's ok to seal the mappings.

[0] https://lore.kernel.org/lkml/20240415163527.626541-1-jeffxu@chromium.org/

Thanks!
Stephen

Stephen Roettger (1):
  elf: mseal non-writable segments

 elf/dl-load.c         |  6 ++++++
 elf/dl-load.h         |  1 +
 elf/dl-map-segments.h |  6 ++++++
 elf/dl-open.c         |  3 ++-
 elf/rtld.c            | 12 +++++++++---
 5 files changed, 24 insertions(+), 4 deletions(-)

-- 
2.45.1.288.g0e0cd299f1-goog

[RFC 1/1] elf: mseal non-writable segments

[Stephen Roettger]

Mseal is a new Linux syscall that blocks any modifications to given
memory mappings like unmapping them or changing the permission bits.

This patch applies mseal to segments during loading if:
* the writable bit is not set
* mode is RTLD_NODELETE

In addition, it adds RTLD_NODELETE to the main binary/libraries and
propagates the RTLD_NODELETE to auxialliary library loads.
---
 elf/dl-load.c         |  6 ++++++
 elf/dl-load.h         |  1 +
 elf/dl-map-segments.h |  6 ++++++
 elf/dl-open.c         |  3 ++-
 elf/rtld.c            | 12 +++++++++---
 5 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/elf/dl-load.c b/elf/dl-load.c
index a34cb3559c..638028d6da 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1136,6 +1136,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
 	  c->mapend = ALIGN_UP (ph->p_vaddr + ph->p_filesz, GLRO(dl_pagesize));
 	  c->dataend = ph->p_vaddr + ph->p_filesz;
 	  c->allocend = ph->p_vaddr + ph->p_memsz;
+	  c->seal = false;
 	  /* Remember the maximum p_align.  */
 	  if (powerof2 (ph->p_align) && ph->p_align > p_align_max)
 	    p_align_max = ph->p_align;
@@ -1169,6 +1170,11 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
 	  if (ph->p_flags & PF_X)
 	    c->prot |= PROT_EXEC;
 #endif
+
+	  if (mode & RTLD_NODELETE && ((c->prot & PROT_WRITE) == 0)) {
+	    c->seal = true;
+	  }
+
 	  break;
 
 	case PT_TLS:
diff --git a/elf/dl-load.h b/elf/dl-load.h
index 656ec229bf..040e744908 100644
--- a/elf/dl-load.h
+++ b/elf/dl-load.h
@@ -78,6 +78,7 @@ struct loadcmd
   ElfW(Addr) mapstart, mapend, dataend, allocend, mapalign;
   ElfW(Off) mapoff;
   int prot;                             /* PROT_* bits.  */
+  bool seal;
 };
 
 
diff --git a/elf/dl-map-segments.h b/elf/dl-map-segments.h
index 30977cf800..4cddb4b294 100644
--- a/elf/dl-map-segments.h
+++ b/elf/dl-map-segments.h
@@ -19,6 +19,8 @@
 
 #include <dl-load.h>
 
+#define SYS_mseal 462
+
 /* Map a segment and align it properly.  */
 
 static __always_inline ElfW(Addr)
@@ -143,6 +145,10 @@ _dl_map_segments (struct link_map *l, int fd,
               == MAP_FAILED))
         return DL_MAP_SEGMENTS_ERROR_MAP_SEGMENT;
 
+      if (c->seal) {
+        syscall(SYS_mseal, (void*) mm, map_size, 0);
+      }
+
     postmap:
       _dl_postprocess_loadcmd (l, header, c);
 
diff --git a/elf/dl-open.c b/elf/dl-open.c
index c378da16c0..a6c89134f8 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -636,7 +636,8 @@ dl_open_worker_begin (void *a)
 
   /* Load that object's dependencies.  */
   _dl_map_object_deps (new, NULL, 0, 0,
-		       mode & (__RTLD_DLOPEN | RTLD_DEEPBIND | __RTLD_AUDIT));
+		       mode & (__RTLD_DLOPEN | RTLD_DEEPBIND |
+			       __RTLD_AUDIT | RTLD_NODELETE));
 
   /* So far, so good.  Now check the versions.  */
   for (unsigned int i = 0; i < new->l_searchlist.r_nlist; ++i)
diff --git a/elf/rtld.c b/elf/rtld.c
index e9525ea987..b471e4c0af 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -79,6 +79,8 @@
 # define RTLD_TIMING_SET(var, value) (var) = (value)
 # define RTLD_TIMING_REF(var)        &(var)
 
+#define SYS_mseal 462
+
 static inline void
 rtld_timer_start (hp_timing_t *var)
 {
@@ -809,7 +811,7 @@ do_preload (const char *fname, struct link_map *main_map, const char *where)
 
   args.str = fname;
   args.loader = main_map;
-  args.mode = __RTLD_SECURE;
+  args.mode = __RTLD_SECURE | RTLD_NODELETE;
 
   unsigned int old_nloaded = GL(dl_ns)[LM_ID_BASE]._ns_nloaded;
 
@@ -1214,6 +1216,10 @@ rtld_setup_main_map (struct link_map *main_map)
 	     segment.  */
 	  expected_load_address = ((allocend + GLRO(dl_pagesize) - 1)
 				   & ~(GLRO(dl_pagesize) - 1));
+
+	  if ((ph->p_flags & PF_W) == 0) {
+	    syscall(SYS_mseal, mapstart, expected_load_address - mapstart, 0);
+	  }
 	}
 	break;
 
@@ -1636,7 +1642,7 @@ dl_main (const ElfW(Phdr) *phdr,
       /* Create a link_map for the executable itself.
 	 This will be what dlopen on "" returns.  */
       main_map = _dl_new_object ((char *) "", "", lt_executable, NULL,
-				 __RTLD_OPENEXEC, LM_ID_BASE);
+				 __RTLD_OPENEXEC | RTLD_NODELETE, LM_ID_BASE);
       assert (main_map != NULL);
       main_map->l_phdr = phdr;
       main_map->l_phnum = phnum;
@@ -1964,7 +1970,7 @@ dl_main (const ElfW(Phdr) *phdr,
     RTLD_TIMING_VAR (start);
     rtld_timer_start (&start);
     _dl_map_object_deps (main_map, preloads, npreloads,
-			 state.mode == rtld_mode_trace, 0);
+			 state.mode == rtld_mode_trace, RTLD_NODELETE);
     rtld_timer_accum (&load_time, start);
   }
 
-- 
2.45.1.288.g0e0cd299f1-goog

Re: [RFC 1/1] elf: mseal non-writable segments

[Cristian Rodríguez]

On Wed, May 22, 2024 at 7:30 AM Stephen Roettger <sroettger@google.com> wrote:
>
> Mseal is a new Linux syscall that blocks any modifications to given
> memory mappings like unmapping them or changing the permission bits.

OOk but has this been merged into at least some tree that will be
pulled by linus at some point in the future AND you are sure the
syscall will not change arguments/semantics again?

Re: [RFC 1/1] elf: mseal non-writable segments

[Carlos O'Donell]

On 5/22/24 12:29 PM, Stephen Röttger wrote:
> The syscall has been merged to linux-next and I think it's unlikely
> it will change at this point: 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/mm/mseal.c
> <https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/mm/mseal.c>
>
>  But in any case, I think any glibc changes should wait until mseal
> made it to a Linux release.

Correct. I think an RFC is the right way forward for now to discuss the
semantics for the dynamic loader.

-- 
Cheers,
Carlos.

Re: [RFC 0/1] elf: mseal non-writable segments

[Carlos O'Donell]

On 5/22/24 7:29 AM, Stephen Roettger wrote:
> In my basic testing, this seems to work fine. But a few questions that
> I'd like some feedback on:
> * Does it sound ok to apply sealing by default? Should this be a flag in
>   the ELF, e.g. maybe the p_flags could have a sealable bit?

I think the sealing *should* be on by default and there should be no way
to disable that, but how do debuggers recover from this to patch code?

What happens to debuggers like gdb, lldb, dyninst, or valgrind when run
with a sealed process? Is there an early rendezvous that can disable
the sealing? Is attaching to such a process to debug it always going to
fail?

In many ways the sealing is equivalent to some of the same operations we
have with SELinux, but driven by the semantics of the operations rather
than any given policy e.g. deny_execmem.

The act of sealing is derived from the semantics that are already expressed
in the ELF file, particularly the PT_LOAD segment properties and
RTLD_NODELETE, which both express that the mapping should not be
removed.

> * Does it make sense to piggyback on the RTLD_NODELETE bit and apply it
>   to more objects? It seems to have the right semantics: the object
>   should never get deleted => it's ok to seal the mappings.

It does make sense.

The more difficult question is: Have these semantics been followed by userpace?

It would be interesting to carry out something like a mass-prebuild of a whole OS
(we do this in Fedora with mass-prebuild [1], and we did it for the GCC 14 transition
last December) with this patch applied and see what fails to build and run 
rpm %check phase. It is effectively A/B testing of rpm build and check.

I'm not suggesting you do that, but it is something we should be able to collaborate
on and evaluate the results.

-- 
Cheers,
Carlos.

[1] https://gitlab.com/fedora/packager-tools/mass-prebuild

Re: [RFC 0/1] elf: mseal non-writable segments

[Florian Weimer]

* Stephen Roettger:

> In my basic testing, this seems to work fine. But a few questions that
> I'd like some feedback on:
> * Does it sound ok to apply sealing by default? Should this be a flag in
>   the ELF, e.g. maybe the p_flags could have a sealable bit?

It depends on how the integration with RELRO should look like.

> * Does it make sense to piggyback on the RTLD_NODELETE bit and apply it
>   to more objects? It seems to have the right semantics: the object
>   should never get deleted => it's ok to seal the mappings.

Doesn't this inhibit many forms of debugging?  We wouldn't want to do
that by default, I think.

Thanks,
Florian

Re: [RFC 0/1] elf: mseal non-writable segments

[Stephen Röttger]

[-- Attachment #1: Type: text/plain, Size: 2584 bytes --]

On Wed, May 22, 2024 at 8:57 PM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 5/22/24 7:29 AM, Stephen Roettger wrote:
> > In my basic testing, this seems to work fine. But a few questions that
> > I'd like some feedback on:
> > * Does it sound ok to apply sealing by default? Should this be a flag in
> >   the ELF, e.g. maybe the p_flags could have a sealable bit?
>
> I think the sealing *should* be on by default and there should be no way
> to disable that, but how do debuggers recover from this to patch code?
>
> What happens to debuggers like gdb, lldb, dyninst, or valgrind when run
> with a sealed process? Is there an early rendezvous that can disable
> the sealing? Is attaching to such a process to debug it always going to
> fail?

I naively assumed that debuggers would use PTRACE_POKE* to write to
the tracee memory. That would still work as before since it doesn't change the
page permissions. So far, I didn't spot any problems with gdb.
It would be an issue if a debugger injects code into the process that then calls
mprotect on a sealed mapping, is that the issue that you have in mind?

Could an environment variable that disables sealing address this?
E.g. if LD_NOSEAL is set, then the loader doesn't seal any mappings.

> In many ways the sealing is equivalent to some of the same operations we
> have with SELinux, but driven by the semantics of the operations rather
> than any given policy e.g. deny_execmem.
>
> The act of sealing is derived from the semantics that are already expressed
> in the ELF file, particularly the PT_LOAD segment properties and
> RTLD_NODELETE, which both express that the mapping should not be
> removed.
>
> > * Does it make sense to piggyback on the RTLD_NODELETE bit and apply it
> >   to more objects? It seems to have the right semantics: the object
> >   should never get deleted => it's ok to seal the mappings.
>
> It does make sense.
>
> The more difficult question is: Have these semantics been followed by userpace?
>
> It would be interesting to carry out something like a mass-prebuild of a whole OS
> (we do this in Fedora with mass-prebuild [1], and we did it for the GCC 14 transition
> last December) with this patch applied and see what fails to build and run
> rpm %check phase. It is effectively A/B testing of rpm build and check.
>
> I'm not suggesting you do that, but it is something we should be able to collaborate
> on and evaluate the results.
>
> --
> Cheers,
> Carlos.
>
> [1] https://gitlab.com/fedora/packager-tools/mass-prebuild
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4016 bytes --]

Re: [RFC 0/1] elf: mseal non-writable segments

[Stephen Röttger]

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]

On Wed, May 22, 2024 at 9:42 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Stephen Roettger:
>
> > In my basic testing, this seems to work fine. But a few questions that
> > I'd like some feedback on:
> > * Does it sound ok to apply sealing by default? Should this be a flag in
> >   the ELF, e.g. maybe the p_flags could have a sealable bit?
>
> It depends on how the integration with RELRO should look like.

In that case, the PT_GNU_RELRO could have this sealable bit set, so rtld could
apply sealing just after the mprotecting it read only.

>
> > * Does it make sense to piggyback on the RTLD_NODELETE bit and apply it
> >   to more objects? It seems to have the right semantics: the object
> >   should never get deleted => it's ok to seal the mappings.
>
> Doesn't this inhibit many forms of debugging?  We wouldn't want to do
> that by default, I think.

Is the concern that a debugger injects code that then calls mprotect on the
mappings for binary patching?

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4016 bytes --]

Re: [RFC 0/1] elf: mseal non-writable segments

[Florian Weimer]

* Stephen Röttger:

> On Wed, May 22, 2024 at 8:57 PM Carlos O'Donell <carlos@redhat.com> wrote:
>>
>> On 5/22/24 7:29 AM, Stephen Roettger wrote:
>> > In my basic testing, this seems to work fine. But a few questions that
>> > I'd like some feedback on:
>> > * Does it sound ok to apply sealing by default? Should this be a flag in
>> >   the ELF, e.g. maybe the p_flags could have a sealable bit?
>>
>> I think the sealing *should* be on by default and there should be no way
>> to disable that, but how do debuggers recover from this to patch code?
>>
>> What happens to debuggers like gdb, lldb, dyninst, or valgrind when run
>> with a sealed process? Is there an early rendezvous that can disable
>> the sealing? Is attaching to such a process to debug it always going to
>> fail?
>
> I naively assumed that debuggers would use PTRACE_POKE* to write to
> the tracee memory. That would still work as before since it doesn't
> change the page permissions. So far, I didn't spot any problems with
> gdb.  It would be an issue if a debugger injects code into the process
> that then calls mprotect on a sealed mapping, is that the issue that
> you have in mind?

No, I had a different kind of seal in mind.  I think there have been
proposals to make userspace memory completely immutable, regardless of
kernel interface involved.  I assumed mseal was a continuation of that.

> Could an environment variable that disables sealing address this?
> E.g. if LD_NOSEAL is set, then the loader doesn't seal any mappings.

Yes, there are going to be applications that change protection flags on
memory that they have not mapped (although we do not really support
this).  I don't know how widespread it is.  Typically we only notice
because they assume a page size that is too small.  And for sealing,
it's going to break only if they try to patch RO or RELRO memory, not if
they turn RW memory from the data segment into RO memory via mprotect.

I had forgotten that we actually fixed bug 20802 for glibc 2.34, so we
don't need to special-case static dlopen anymore.  (We used to undo
RELRO for that.)

A few comments on your patch: In the loader, we use l_type ==
lt_executable and l_type == lt_library to identify initially loaded
objects, we don't use the RTLD_NODELETE flag for that.  The system call
number should come from an arch-syscall.h header (which we update for
every kernel release).  The actual system call needs to be perform with
INTERNAL_SYSCALL_CALL or INLINE_SYSCALL_CALL.  It has to be in a
separate inline function in a suitable header file (potentially a new
one), and the generic code should fail with ENOSYS.  This is needed
because Hurd does not have system calls.  We'll need positive/negative
test cases, including tests that check what happens if RTLD_NODELETE is
applied to an already-loaded object.

I would really like RELRO to be addressed from the start, even though I
expect it to be really tricky.  The reason is that I want to avoid two
transitions (first fix the .rodata patchers, then fix the RELRO
patchers).

I assume that it's going to be the responsibility of ld.so (not the
kernel) to apply sealing to the main program.  That needs to be tested
separately.

Ideally, we'd add this alongside with a feature that allows object to
delay RELRO activation until after their ELF constructors have been
executed, to address the mprotect-patching scenario mentioned above.
But that is fairly involved project (requires binutils/lld/mold changes,
too), so it has to be considered a separate change, I think.

Thanks,
Florian

Re: [RFC 0/1] elf: mseal non-writable segments

[Adhemerval Zanella Netto]

On 23/05/24 06:36, Stephen Röttger wrote:
> On Wed, May 22, 2024 at 9:42 PM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Stephen Roettger:
>>
>>> In my basic testing, this seems to work fine. But a few questions that
>>> I'd like some feedback on:
>>> * Does it sound ok to apply sealing by default? Should this be a flag in
>>>   the ELF, e.g. maybe the p_flags could have a sealable bit?
>>
>> It depends on how the integration with RELRO should look like.
> 
> In that case, the PT_GNU_RELRO could have this sealable bit set, so rtld could
> apply sealing just after the mprotecting it read only.

The OpenBSD added a PT_OPENBSD_MUTABLE extension [1], since the idea is to 
make everything immutable and only mark the exceptions.  But I am not sure
it would make sense it for Linux, this really seems to be a opt-in hardening 
feature and it would require either to add some hack like the glibc DT_RELR
symbol  (to avoid backward compatibility) or assume it is a hardening hint 
only enable if loader and kernel supports it.

For glibc I think it makes more sense to add it like a tunable (like
glibc.mem.tagging) and eventually make it default.

> I assume that it's going to be the responsibility of ld.so (not the
> kernel) to apply sealing to the main program.  That needs to be tested
> separately.
> 
> Ideally, we'd add this alongside with a feature that allows object to
> delay RELRO activation until after their ELF constructors have been
> executed, to address the mprotect-patching scenario mentioned above.
> But that is fairly involved project (requires binutils/lld/mold changes,
> too), so it has to be considered a separate change, I think.

Yes, on Linux it is up to the userland to setup the sealing. And I don't
think we should tie the sealing support to the delay RELRO refactor, it 
requires extensive changes on _dl_start_user to either make _dl_init
call RELRO/sealing setup after ctor/initarray is done, or calling
after _dl_init.  There is also the question whether to apply RELRO/sealing
per-module after ctor/initarray or in a bulk after _dt_init.

> 
>>
>>> * Does it make sense to piggyback on the RTLD_NODELETE bit and apply it
>>>   to more objects? It seems to have the right semantics: the object
>>>   should never get deleted => it's ok to seal the mappings.
>>
>> Doesn't this inhibit many forms of debugging?  We wouldn't want to do
>> that by default, I think.
> 
> Is the concern that a debugger injects code that then calls mprotect on the
> mappings for binary patching?

I see that with a tunable to disable sealing we can make it work with tools
that requires changing the protection.  Also, tools that uses ptrace can
always intercept mseal syscall and just disable it if required.

So I created a WIP support that at least does not trigger any regression
and seems to work on some programs [1].  The sealing is done in multiple 
places where the memory is supported to be immuatable over program 
execution:

  * All DT_NEEDED dependencies from the binary, including the RELRO
    segments after PT_GNU_RELRO setup.

  * The binary itself, including dynamic and static linked.  In both
    cases it is up either to binary or the loader to setup the sealing.

  * The ld.so.cache cache.

  * The vDSO segment (if existent).

  * Any preload libraries.

We need some care for non-contiguous memory segments, but it seems that
kernel is providing the correct information on the AT_PHDR.  Also, this
WIP patchset does not support audit libraries: the code that handles it
requires to load the object, resolve the symbols, call the la_version
and some other functions, and any issue would trigger the library and
dependencies to be unloaded.  It should be doable by walking, after
audit loading is done, on the audit namespace and sealing each object.

The patchset also lacks some proper testing that sealing was done by the
kernel, I had to manually inspect the segments flags to assure it.  The
mseal support lacks a proper way to query for the immutable flags,
/proc/self/maps does not present this information and /proc/self/smaps
only prints '??' when the immutable flag it done.

I have done this WIP with a tunable (glibc.rtld.seal) and run the glibc
regression with the enforce mode set manually on both x86_64 and aarch64.
I still need to test on more complex scenarios, specially for programs
that do non trivial stuff in ctors/initarray.

[1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/mseal