On 4.11.2020 16.35, Catalin Marinas wrote:
On Wed, Nov 04, 2020 at 11:55:57AM +0200, Topi Miettinen wrote:
On 4.11.2020 11.29, Florian Weimer wrote:
* Will Deacon:
Is there real value in this seccomp filter if it only looks at mprotect(),
or was it just implemented because it's easy to do and sounds like a good
idea?
It seems bogus to me. Everyone will just create alias mappings instead,
just like they did for the similar SELinux feature. See “Example code
to avoid execmem violations” in:
<https://www.akkadia.org/drepper/selinux-mem.html>
[...]
As you can see, this reference implementation creates a PROT_WRITE
mapping aliased to a PROT_EXEC mapping, so it actually reduces security
compared to something that generates the code in an anonymous mapping
and calls mprotect to make it executable.
[...]
If a service legitimately needs executable and writable mappings (due to
JIT, trampolines etc), it's easy to disable the filter whenever really
needed with "MemoryDenyWriteExecute=no" (which is the default) in case of
systemd or a TE rule like "allow type_t self:process { execmem };" for
SELinux. But this shouldn't be the default case, since there are many
services which don't need W&X.
I think Drepper's point is that separate X and W mappings, with enough
randomisation, would be more secure than allowing W&X at the same
address (but, of course, less secure than not having W at all, though
that's not always possible).
I'd also question what is the value of BTI if it can be easily circumvented
by removing PROT_BTI with mprotect()?
Well, BTI is a protection against JOP attacks. The assumption here is
that an attacker cannot invoke mprotect() to disable PROT_BTI. If it
can, it's probably not worth bothering with a subsequent JOP attack, it
can already call functions directly.
I suppose that the target for the attacker is to eventually perform
system calls rather than looping forever in JOP/ROP gadgets.
I see MDWX not as a way of detecting attacks but rather plugging
inadvertent security holes in certain programs. On arm64, such hardening
currently gets in the way of another hardening feature, BTI.
I don't think it has to get in the way at all. Why wouldn't something
simple like this work:
diff --git a/elf/dl-load.c b/elf/dl-load.c
index 646c5dca40..12a74d15e8 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1170,8 +1170,13 @@ _dl_map_object_from_fd (const char *name, const
char *origname, int fd,
c->prot |= PROT_READ;
if (ph->p_flags & PF_W)
c->prot |= PROT_WRITE;
- if (ph->p_flags & PF_X)
+ if (ph->p_flags & PF_X) {
c->prot |= PROT_EXEC;
+#ifdef PROT_BTI
+ if (GLRO(dl_bti) & 1)
+ c->prot |= PROT_BTI;
+#endif
+ }
#endif
break;
diff --git a/elf/dl-support.c b/elf/dl-support.c
index 7704c101c5..22c7cc7b81 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -222,7 +222,7 @@ __rtld_lock_define_initialized_recursive (,
_dl_load_write_lock)
#ifdef HAVE_AUX_VECTOR
-int _dl_clktck;
+int _dl_clktck, _dl_bti;
void
_dl_aux_init (ElfW(auxv_t) *av)
@@ -294,6 +294,11 @@ _dl_aux_init (ElfW(auxv_t) *av)
case AT_RANDOM:
_dl_random = (void *) av->a_un.a_val;
break;
+#ifdef PROT_BTI
+ case AT_BTI:
+ _dl_bti = (void *) av->a_un.a_val;
+ break;
+#endif
DL_PLATFORM_AUXV
}
if (seen == 0xf)
Kernel sets the aux vector to indicate that BTI should be enabled for
all segments and main exe is already protected.
-Topi
