From: Szabolcs Nagy <szabolcs.nagy@arm.com> To: Topi Miettinen <toiwoton@gmail.com> Cc: Catalin Marinas <catalin.marinas@arm.com>, Florian Weimer <fweimer@redhat.com>, Will Deacon <will@kernel.org>, Mark Brown <broonie@kernel.org>, libc-alpha@sourceware.org, Jeremy Linton <jeremy.linton@arm.com>, Mark Rutland <mark.rutland@arm.com>, Kees Cook <keescook@chromium.org>, Salvatore Mesoraca <s.mesoraca16@gmail.com>, Lennart Poettering <mzxreary@0pointer.de>, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com, linux-hardening@vger.kernel.org Subject: Re: [PATCH 0/4] aarch64: avoid mprotect(PROT_BTI|PROT_EXEC) [BZ #26831] Date: Wed, 4 Nov 2020 16:08:10 +0000 [thread overview] Message-ID: <20201104160810.GD24704@arm.com> (raw) In-Reply-To: <f595e572-40bc-a052-f3f2-763433d6762f@gmail.com> The 11/04/2020 17:19, Topi Miettinen wrote: > 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: PROT_BTI is only valid on binaries that are BTI compatible. to detect that, the load segments must be already mapped. AT_BTI does not solve this: we want to be able to load legacy elf modules. (a BTI enforcement setting may be useful where incompatible modules are rejected, but that cannot be the default for backward compatibility reasons.) > > 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
WARNING: multiple messages have this Message-ID (diff)
From: Szabolcs Nagy <szabolcs.nagy@arm.com> To: Topi Miettinen <toiwoton@gmail.com> Cc: Florian Weimer <fweimer@redhat.com>, Mark Rutland <mark.rutland@arm.com>, libc-alpha@sourceware.org, Kees Cook <keescook@chromium.org>, kernel-hardening@lists.openwall.com, Salvatore Mesoraca <s.mesoraca16@gmail.com>, Catalin Marinas <catalin.marinas@arm.com>, linux-kernel@vger.kernel.org, Jeremy Linton <jeremy.linton@arm.com>, Mark Brown <broonie@kernel.org>, Lennart Poettering <mzxreary@0pointer.de>, linux-hardening@vger.kernel.org, Will Deacon <will@kernel.org>, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 0/4] aarch64: avoid mprotect(PROT_BTI|PROT_EXEC) [BZ #26831] Date: Wed, 4 Nov 2020 16:08:10 +0000 [thread overview] Message-ID: <20201104160810.GD24704@arm.com> (raw) In-Reply-To: <f595e572-40bc-a052-f3f2-763433d6762f@gmail.com> The 11/04/2020 17:19, Topi Miettinen wrote: > 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: PROT_BTI is only valid on binaries that are BTI compatible. to detect that, the load segments must be already mapped. AT_BTI does not solve this: we want to be able to load legacy elf modules. (a BTI enforcement setting may be useful where incompatible modules are rejected, but that cannot be the default for backward compatibility reasons.) > > 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-11-04 16:08 UTC|newest] Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-03 10:25 [PATCH 0/4] aarch64: avoid mprotect(PROT_BTI|PROT_EXEC) [BZ #26831] Szabolcs Nagy 2020-11-03 10:25 ` Szabolcs Nagy 2020-11-03 10:25 ` [PATCH 1/4] elf: Pass the fd to note processing " Szabolcs Nagy 2020-11-03 10:25 ` Szabolcs Nagy 2020-11-03 10:26 ` [PATCH 2/4] elf: Move note processing after l_phdr is updated " Szabolcs Nagy 2020-11-03 10:26 ` Szabolcs Nagy 2020-11-03 10:38 ` Florian Weimer 2020-11-03 10:38 ` Florian Weimer 2020-11-03 10:38 ` Florian Weimer 2020-11-03 12:36 ` H.J. Lu 2020-11-03 12:36 ` H.J. Lu 2020-11-03 12:36 ` H.J. Lu 2020-11-03 15:04 ` Szabolcs Nagy 2020-11-03 15:04 ` Szabolcs Nagy 2020-11-03 15:27 ` H.J. Lu 2020-11-03 15:27 ` H.J. Lu 2020-11-03 15:27 ` H.J. Lu 2020-11-03 10:26 ` [PATCH 3/4] aarch64: Use mmap to add PROT_BTI instead of mprotect " Szabolcs Nagy 2020-11-03 10:26 ` Szabolcs Nagy 2020-11-03 10:34 ` Florian Weimer 2020-11-03 10:34 ` Florian Weimer 2020-11-03 10:34 ` Florian Weimer 2020-11-03 10:26 ` [PATCH 4/4] aarch64: Remove the bti link_map field " Szabolcs Nagy 2020-11-03 10:26 ` Szabolcs Nagy 2020-11-03 17:34 ` [PATCH 0/4] aarch64: avoid mprotect(PROT_BTI|PROT_EXEC) " Mark Brown 2020-11-03 17:34 ` Mark Brown 2020-11-04 5:41 ` Jeremy Linton 2020-11-04 5:41 ` Jeremy Linton 2020-11-04 8:57 ` Szabolcs Nagy 2020-11-04 8:57 ` Szabolcs Nagy 2020-11-04 14:41 ` Catalin Marinas 2020-11-04 14:41 ` Catalin Marinas 2020-11-04 14:45 ` Florian Weimer 2020-11-04 14:45 ` Florian Weimer 2020-11-04 14:45 ` Florian Weimer 2020-11-04 10:50 ` Mark Brown 2020-11-04 10:50 ` Mark Brown 2020-11-04 18:47 ` Jeremy Linton 2020-11-04 18:47 ` Jeremy Linton 2020-11-04 18:53 ` Mark Brown 2020-11-04 18:53 ` Mark Brown 2020-11-04 9:02 ` Topi Miettinen 2020-11-04 9:02 ` Topi Miettinen 2020-11-04 9:20 ` Will Deacon 2020-11-04 9:20 ` Will Deacon 2020-11-04 9:29 ` Florian Weimer 2020-11-04 9:29 ` Florian Weimer 2020-11-04 9:29 ` Florian Weimer 2020-11-04 9:55 ` Topi Miettinen 2020-11-04 9:55 ` Topi Miettinen 2020-11-04 14:35 ` Catalin Marinas 2020-11-04 14:35 ` Catalin Marinas 2020-11-04 15:19 ` Topi Miettinen 2020-11-04 15:19 ` Topi Miettinen 2020-11-04 16:08 ` Szabolcs Nagy [this message] 2020-11-04 16:08 ` Szabolcs Nagy 2020-11-04 15:20 ` Mark Rutland 2020-11-04 15:20 ` Mark Rutland 2020-11-04 18:59 ` Jeremy Linton 2020-11-04 18:59 ` Jeremy Linton 2020-11-05 11:31 ` Szabolcs Nagy 2020-11-05 11:31 ` Szabolcs Nagy
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=20201104160810.GD24704@arm.com \ --to=szabolcs.nagy@arm.com \ --cc=broonie@kernel.org \ --cc=catalin.marinas@arm.com \ --cc=fweimer@redhat.com \ --cc=jeremy.linton@arm.com \ --cc=keescook@chromium.org \ --cc=kernel-hardening@lists.openwall.com \ --cc=libc-alpha@sourceware.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-hardening@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mzxreary@0pointer.de \ --cc=s.mesoraca16@gmail.com \ --cc=toiwoton@gmail.com \ --cc=will@kernel.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.