On 17/11/2021 9:26 am, Marcin Wojtas wrote:
The branch main has been updated by mw:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=b014e0f15bc73d80ef49b64fd1f8c29f469467cb

commit b014e0f15bc73d80ef49b64fd1f8c29f469467cb
Author:     Marcin Wojtas <m...@freebsd.org>
AuthorDate: 2021-10-24 14:53:06 +0000
Commit:     Marcin Wojtas <m...@freebsd.org>
CommitDate: 2021-11-16 22:16:09 +0000

     Enable ASLR by default for 64-bit executables
Address Space Layout Randomization (ASLR) is an exploit mitigation
     technique implemented in the majority of modern operating systems.
     It involves randomly positioning the base address of an executable
     and the position of libraries, heap, and stack, in a process's address
     space. Although over the years ASLR proved to not guarantee full OS
     security on its own, this mechanism can make exploitation more difficult.
Tests on the tier 1 64-bit architectures demonstrated that the ASLR is
     stable and does not result in noticeable performance degradation,
     therefore it should be safe to enable this mechanism by default.
     Moreover its effectiveness is increased for PIE (Position Independent
     Executable) binaries. Thanks to commit 9a227a2fd642 ("Enable PIE by
     default on 64-bit architectures"), building from src is not necessary
     to have PIE binaries. It is enough to control usage of ASLR in the
     OS solely by setting the appropriate sysctls.
This patch toggles the kernel settings to use address map randomization
     for PIE & non-PIE 64-bit binaries. It also disables SBRK, in order
     to allow utilization of the bss grow region for mappings. The latter
     has no effect if ASLR is disabled, so apply it to all architectures.
As for the drawbacks, a consequence of using the ASLR is more
     significant VM fragmentation, hence the issues may be encountered
     in the systems with a limited address space in high memory consumption
     cases, such as buildworld. As a result, although the tests on 32-bit
     architectures with ASLR enabled were mostly on par with what was
     observed on 64-bit ones, the defaults for the former are not changed
     at this time. Also, for the sake of safety keep the feature disabled
     for 32-bit executables on 64-bit machines, too.
The committed change affects the overall OS operation, so the
     following should be taken into consideration:
     * Address space fragmentation.
     * A changed ABI due to modified layout of address space.
     * More complicated debugging due to:
       * Non-reproducible address space layout between runs.
       * Some debuggers automatically disable ASLR for spawned processes,
         making target's environment different between debug and
         non-debug runs.
In order to confirm/rule-out the dependency of any encountered issue
     on ASLR it is strongly advised to re-run the test with the feature
     disabled - it can be done by setting the following sysctls
     in the /etc/sysctl.conf file:
     kern.elf64.aslr.enable=0
     kern.elf64.aslr.pie_enable=0
Co-developed by: Dawid Gorecki <d...@semihalf.com>
     Reviewed by: emaste, kib
     Obtained from: Semihalf
     Sponsored by: Stormshield
     MFC after: 1 month
     Differential revision: https://reviews.freebsd.org/D27666
---
  sys/kern/imgact_elf.c | 20 +++++++++++++++++---
  1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/sys/kern/imgact_elf.c b/sys/kern/imgact_elf.c
index 898f0f66a532..38ad61d8720b 100644
--- a/sys/kern/imgact_elf.c
+++ b/sys/kern/imgact_elf.c
@@ -161,19 +161,33 @@ SYSCTL_NODE(__CONCAT(_kern_elf, __ELF_WORD_SIZE), 
OID_AUTO, aslr,
      "");
  #define       ASLR_NODE_OID   __CONCAT(__CONCAT(_kern_elf, __ELF_WORD_SIZE), 
_aslr)
-static int __elfN(aslr_enabled) = 0;
+/*
+ * While for 64-bit machines ASLR works properly, there are
+ * still some problems when using 32-bit architectures. For this
+ * reason ASLR is only enabled by default when running native
+ * 64-bit non-PIE executables.
+ */
+static int __elfN(aslr_enabled) = __ELF_WORD_SIZE == 64;
  SYSCTL_INT(ASLR_NODE_OID, OID_AUTO, enable, CTLFLAG_RWTUN,
      &__elfN(aslr_enabled), 0,
      __XSTRING(__CONCAT(ELF, __ELF_WORD_SIZE))
      ": enable address map randomization");
-static int __elfN(pie_aslr_enabled) = 0;
+/*
+ * Enable ASLR only for 64-bit PIE binaries by default.
+ */
+static int __elfN(pie_aslr_enabled) = __ELF_WORD_SIZE == 64;
  SYSCTL_INT(ASLR_NODE_OID, OID_AUTO, pie_enable, CTLFLAG_RWTUN,
      &__elfN(pie_aslr_enabled), 0,
      __XSTRING(__CONCAT(ELF, __ELF_WORD_SIZE))
      ": enable address map randomization for PIE binaries");

The current description seems ambiguous with respect to the added comment. If the sysctl (=1) applies ASLR "only" for PIE binaries, where the =0 (sysctl disabled) case applies it unconditionally, a better description might be:

"Enable address map randomization only for PIE binaries"

What is the actual/correct behaviour of the control?

Might aslr_enabled_pie_only also be a better OID name? Perhaps not worth the churn, but long term it would be great if OID names reflected what they are/do, rather than what they're not/don't do.

-static int __elfN(aslr_honor_sbrk) = 1;
+/*
+ * Sbrk is now deprecated and it can be assumed, that in most
+ * cases it will not be used anyway. This setting is valid only
+ * for the ASLR enabled and allows for utilizing the bss grow region.
+ */
+static int __elfN(aslr_honor_sbrk) = 0;
  SYSCTL_INT(ASLR_NODE_OID, OID_AUTO, honor_sbrk, CTLFLAG_RW,
      &__elfN(aslr_honor_sbrk), 0,
      __XSTRING(__CONCAT(ELF, __ELF_WORD_SIZE)) ": assume sbrk is used");


Can we add (DEPRECATED) to the control description, and/or otherwise mark the control as deprecated if the sysctl framework supports an attribute marking them as such?


Reply via email to