From: Kees Cook <keescook@chromium.org>
To: "H . J . Lu" <hjl.tools@gmail.com>
Cc: Kees Cook <keescook@chromium.org>,
Mike Rapoport <rppt@linux.ibm.com>,
Rui Salvaterra <rsalvaterra@gmail.com>,
Victor Stinner <vstinner@redhat.com>,
Jan Palus <jpalus@fastmail.com>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
Eric Biederman <ebiederm@xmission.com>,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
Chris Kennelly <ckennelly@google.com>,
Shuah Khan <shuah@kernel.org>,
Muhammad Usama Anjum <usama.anjum@collabora.com>,
John Hubbard <jhubbard@nvidia.com>,
Fangrui Song <maskray@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
Yang Yingliang <yangyingliang@huawei.com>,
Mike Rapoport <rppt@kernel.org>,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-hardening@vger.kernel.org
Subject: [PATCH 3/3] binfmt_elf: Honor PT_LOAD alignment for static PIE
Date: Wed, 8 May 2024 10:31:48 -0700 [thread overview]
Message-ID: <20240508173149.677910-3-keescook@chromium.org> (raw)
In-Reply-To: <20240508172848.work.131-kees@kernel.org>
The p_align values in PT_LOAD were ignored for static PIE executables
(i.e. ET_DYN without PT_INTERP). This is because there is no way to
request a non-fixed mmap region with a specific alignment. ET_DYN with
PT_INTERP uses a separate base address (ELF_ET_DYN_BASE) and binfmt_elf
performs the ASLR itself, which means it can also apply alignment. For
the mmap region, the address selection happens deep within the vm_mmap()
implementation (when the requested address is 0).
The earlier attempt to implement this:
commit 9630f0d60fec ("fs/binfmt_elf: use PT_LOAD p_align values for static PIE")
commit 925346c129da ("fs/binfmt_elf: fix PT_LOAD p_align values for loaders")
did not take into account the different base address origins, and were
eventually reverted:
aeb7923733d1 ("revert "fs/binfmt_elf: use PT_LOAD p_align values for static PIE"")
In order to get the correct alignment from an mmap base, binfmt_elf must
perform a 0-address load first, then tear down the mapping and perform
alignment on the resulting address. Since this is slightly more overhead,
only do this when it is needed (i.e. the alignment is not the default
ELF alignment). This does, however, have the benefit of being able to
use MAP_FIXED_NOREPLACE, to avoid potential collisions.
With this fixed, enable the static PIE self tests again.
Reported-by: H.J. Lu <hjl.tools@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=215275
Signed-off-by: Kees Cook <keescook@chromium.org>
---
Cc: H.J. Lu <hjl.tools@gmail.com>
Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: Rui Salvaterra <rsalvaterra@gmail.com>
Cc: Victor Stinner <vstinner@redhat.com>
Cc: Jan Palus <jpalus@fastmail.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-mm@kvack.org
---
fs/binfmt_elf.c | 42 +++++++++++++++++++++++----
tools/testing/selftests/exec/Makefile | 2 +-
2 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 56432e019d4e..cbb07a9c02d4 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1088,10 +1088,13 @@ static int load_elf_binary(struct linux_binprm *bprm)
goto out_free_dentry;
}
+ /* Calculate any requested alignment. */
+ alignment = maximum_alignment(elf_phdata, elf_ex->e_phnum);
+
/*
* There are effectively two types of ET_DYN
- * binaries: programs (i.e. PIE: ET_DYN with INTERP)
- * and loaders (ET_DYN without INTERP, since they
+ * binaries: programs (i.e. PIE: ET_DYN with PT_INTERP)
+ * and loaders (ET_DYN without PT_INTERP, since they
* _are_ the ELF interpreter). The loaders must
* be loaded away from programs since the program
* may otherwise collide with the loader (especially
@@ -1111,15 +1114,44 @@ static int load_elf_binary(struct linux_binprm *bprm)
* without MAP_FIXED nor MAP_FIXED_NOREPLACE).
*/
if (interpreter) {
+ /* On ET_DYN with PT_INTERP, we do the ASLR. */
load_bias = ELF_ET_DYN_BASE;
if (current->flags & PF_RANDOMIZE)
load_bias += arch_mmap_rnd();
- alignment = maximum_alignment(elf_phdata, elf_ex->e_phnum);
+ /* Adjust alignment as requested. */
if (alignment)
load_bias &= ~(alignment - 1);
elf_flags |= MAP_FIXED_NOREPLACE;
- } else
- load_bias = 0;
+ } else {
+ /*
+ * For ET_DYN without PT_INTERP, we rely on
+ * the architectures's (potentially ASLR) mmap
+ * base address (via a load_bias of 0).
+ *
+ * When a large alignment is requested, we
+ * must do the allocation at address "0" right
+ * now to discover where things will load so
+ * that we can adjust the resulting alignment.
+ * In this case (load_bias != 0), we can use
+ * MAP_FIXED_NOREPLACE to make sure the mapping
+ * doesn't collide with anything.
+ */
+ if (alignment > ELF_MIN_ALIGN) {
+ load_bias = elf_load(bprm->file, 0, elf_ppnt,
+ elf_prot, elf_flags, total_size);
+ if (BAD_ADDR(load_bias)) {
+ retval = IS_ERR_VALUE(load_bias) ?
+ PTR_ERR((void*)load_bias) : -EINVAL;
+ goto out_free_dentry;
+ }
+ vm_munmap(load_bias, total_size);
+ /* Adjust alignment as requested. */
+ if (alignment)
+ load_bias &= ~(alignment - 1);
+ elf_flags |= MAP_FIXED_NOREPLACE;
+ } else
+ load_bias = 0;
+ }
/*
* Since load_bias is used for all subsequent loading
diff --git a/tools/testing/selftests/exec/Makefile b/tools/testing/selftests/exec/Makefile
index 619cff81d796..ab67d58cfab7 100644
--- a/tools/testing/selftests/exec/Makefile
+++ b/tools/testing/selftests/exec/Makefile
@@ -6,7 +6,7 @@ CFLAGS += -D_GNU_SOURCE
ALIGNS := 0x1000 0x200000 0x1000000
ALIGN_PIES := $(patsubst %,load_address.%,$(ALIGNS))
ALIGN_STATIC_PIES := $(patsubst %,load_address.static.%,$(ALIGNS))
-ALIGNMENT_TESTS := $(ALIGN_PIES)
+ALIGNMENT_TESTS := $(ALIGN_PIES) $(ALIGN_STATIC_PIES)
TEST_PROGS := binfmt_script.py
TEST_GEN_PROGS := execveat non-regular $(ALIGNMENT_TESTS)
--
2.34.1
prev parent reply other threads:[~2024-05-08 17:31 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-08 17:31 [PATCH 0/3] binfmt_elf: Honor PT_LOAD alignment for static PIE Kees Cook
2024-05-08 17:31 ` [PATCH 1/3] selftests/exec: Build both static and non-static load_address tests Kees Cook
2024-05-09 2:54 ` John Hubbard
2024-05-09 6:16 ` Kees Cook
2024-05-08 17:31 ` [PATCH 2/3] binfmt_elf: Calculate total_size earlier Kees Cook
2024-05-08 17:31 ` Kees Cook [this message]
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=20240508173149.677910-3-keescook@chromium.org \
--to=keescook@chromium.org \
--cc=akpm@linux-foundation.org \
--cc=brauner@kernel.org \
--cc=ckennelly@google.com \
--cc=ebiederm@xmission.com \
--cc=hjl.tools@gmail.com \
--cc=jack@suse.cz \
--cc=jhubbard@nvidia.com \
--cc=jpalus@fastmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=maskray@google.com \
--cc=rppt@kernel.org \
--cc=rppt@linux.ibm.com \
--cc=rsalvaterra@gmail.com \
--cc=shuah@kernel.org \
--cc=usama.anjum@collabora.com \
--cc=viro@zeniv.linux.org.uk \
--cc=vstinner@redhat.com \
--cc=yangyingliang@huawei.com \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).