* Re: [PATCH v21 1/4] mm: add VM_DROPPABLE for designating always lazily freeable mappings
From: David Hildenbrand @ 2024-07-07 7:42 UTC (permalink / raw)
To: Jason A. Donenfeld, linux-kernel, patches, tglx
Cc: linux-crypto, linux-api, x86, Linus Torvalds, Greg Kroah-Hartman,
Adhemerval Zanella Netto, Carlos O'Donell, Florian Weimer,
Arnd Bergmann, Jann Horn, Christian Brauner, David Hildenbrand,
linux-mm
In-Reply-To: <20240707002658.1917440-2-Jason@zx2c4.com>
On 07.07.24 02:26, Jason A. Donenfeld wrote:
Hi,
having more generic support for VM_DROPPABLE sounds great, I was myself
at some point looking for something like that.
> The vDSO getrandom() implementation works with a buffer allocated with a
> new system call that has certain requirements:
>
> - It shouldn't be written to core dumps.
> * Easy: VM_DONTDUMP.
> - It should be zeroed on fork.
> * Easy: VM_WIPEONFORK.
>
> - It shouldn't be written to swap.
> * Uh-oh: mlock is rlimited.
> * Uh-oh: mlock isn't inherited by forks.
>
> It turns out that the vDSO getrandom() function has three really nice
> characteristics that we can exploit to solve this problem:
>
> 1) Due to being wiped during fork(), the vDSO code is already robust to
> having the contents of the pages it reads zeroed out midway through
> the function's execution.
>
> 2) In the absolute worst case of whatever contingency we're coding for,
> we have the option to fallback to the getrandom() syscall, and
> everything is fine.
>
> 3) The buffers the function uses are only ever useful for a maximum of
> 60 seconds -- a sort of cache, rather than a long term allocation.
>
> These characteristics mean that we can introduce VM_DROPPABLE, which
> has the following semantics:
>
> a) It never is written out to swap.
> b) Under memory pressure, mm can just drop the pages (so that they're
> zero when read back again).
> c) It is inherited by fork.
> d) It doesn't count against the mlock budget, since nothing is locked.
>
> This is fairly simple to implement, with the one snag that we have to
> use 64-bit VM_* flags, but this shouldn't be a problem, since the only
> consumers will probably be 64-bit anyway.
>
> This way, allocations used by vDSO getrandom() can use:
>
> VM_DROPPABLE | VM_DONTDUMP | VM_WIPEONFORK | VM_NORESERVE
>
> And there will be no problem with using memory when not in use, not
> wiping on fork(), coredumps, or writing out to swap.
>
> In order to let vDSO getrandom() use this, expose these via mmap(2) as
> well, giving MAP_WIPEONFORK, MAP_DONTDUMP, and MAP_DROPPABLE.
Patch subject would be better to talk about MAP_DROPPABLE now.
But I don't immediately see why MAP_WIPEONFORK and MAP_DONTDUMP have to
be mmap() flags. Using mmap(MAP_NORESERVE|MAP_DROPPABLE) with madvise()
to configure these (for users that require that) should be good enough,
just like they are for existing users.
Thinking out loud, also MAP_DROPPABLE only sets a VMA flag (and does
snot affect memory commitiing like MAP_NORESERVE), right? So
MAP_DROPPABLE could easily become a madvise() option as well?
(as you know, we only have limited mmap bits but plenty of madvise
numbers available)
Interestingly, when looking into something comparable in the past I
stumbled over "vrange" [1], which would have had a slightly different
semantic (signal on reaccess). And that did turn out to be more sutibale
for madvise() flags [2], whereby vrange evolved into
MADV_VOLATILE/MADV_NONVOLATILE
A sticky MADV_VOLATILE vs. MADV_NONVOLATILE would actually sound pretty
handy. (again, with your semantics, not the signal-on-reaccess kind of
thing)
([2] is in general a good read; hey, it's been 10 years since that was
brought up the last time!)
There needs to be better reasoning why we have to consume three mmap
bits for something that can likely be achieved without any.
Maybe that was discussed with Linus and there is a pretty good reason
for that.
I'll also mention that I am unsure how MAP_DROPPABLE is supposed to
interact with mlock. Maybe just like MADV_FREE currently does (no idea
if that will work as intended ;) ).
[1] https://lwn.net/Articles/590991/
[2] https://lwn.net/Articles/602650/
>
> Finally, the provided self test ensures that this is working as desired.
>
> Cc: linux-mm@kvack.org
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
[...]
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 8c6cd8825273..57b8dad9adcc 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -623,7 +623,7 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,
> may_expand_vm(mm, oldflags, nrpages))
> return -ENOMEM;
> if (!(oldflags & (VM_ACCOUNT|VM_WRITE|VM_HUGETLB|
> - VM_SHARED|VM_NORESERVE))) {
> + VM_SHARED|VM_NORESERVE|VM_DROPPABLE))) {
> charged = nrpages;
> if (security_vm_enough_memory_mm(mm, charged))
> return -ENOMEM;
I don't quite understand this change here. If MAP_DROPPABLE does not
affect memory accounting during mmap(), it should not affect the same
during mprotect(). VM_NORESERVE / MAP_NORESERVE is responsible for that.
Did I missing something where MAP_DROPPABLE changes the memory
accounting during mmap()?
> diff --git a/mm/rmap.c b/mm/rmap.c
> index e8fc5ecb59b2..56d7535d5cf6 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1397,7 +1397,10 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
> VM_WARN_ON_FOLIO(folio_test_hugetlb(folio), folio);
> VM_BUG_ON_VMA(address < vma->vm_start ||
> address + (nr << PAGE_SHIFT) > vma->vm_end, vma);
> - __folio_set_swapbacked(folio);
> + /* VM_DROPPABLE mappings don't swap; instead they're just dropped when
> + * under memory pressure. */
> + if (!(vma->vm_flags & VM_DROPPABLE))
> + __folio_set_swapbacked(folio);
> __folio_set_anon(folio, vma, address, true);
>
> if (likely(!folio_test_large(folio))) {
> @@ -1841,7 +1844,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
> * plus the rmap(s) (dropped by discard:).
> */
> if (ref_count == 1 + map_count &&
> - !folio_test_dirty(folio)) {
> + (!folio_test_dirty(folio) ||
> + /* Unlike MADV_FREE mappings, VM_DROPPABLE
> + * ones can be dropped even if they've
> + * been dirtied. */
We use
/*
* Comment start
* Comment end
*/
styled comments in MM.
> + (vma->vm_flags & VM_DROPPABLE))) {
> dec_mm_counter(mm, MM_ANONPAGES);
> goto discard;
> }
> @@ -1851,7 +1858,10 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
> * discarded. Remap the page to page table.
> */
> set_pte_at(mm, address, pvmw.pte, pteval);
> - folio_set_swapbacked(folio);
> + /* Unlike MADV_FREE mappings, VM_DROPPABLE ones
> + * never get swap backed on failure to drop. */
> + if (!(vma->vm_flags & VM_DROPPABLE))
> + folio_set_swapbacked(folio);
> ret = false;
> page_vma_mapped_walk_done(&pvmw);
> break;
A note that in mm/mm-stable, "madvise_free_huge_pmd" exists to optimize
MADV_FREE on PMDs. I suspect we'd want to extend that one as well for
dropping support, but likely it would also only be a performance
improvmeent and not affect functonality if not handled.
--
Cheers,
David / dhildenb
^ permalink raw reply
* [PATCH v21 4/4] selftests/vDSO: add tests for vgetrandom
From: Jason A. Donenfeld @ 2024-07-07 0:26 UTC (permalink / raw)
To: linux-kernel, patches, tglx
Cc: Jason A. Donenfeld, linux-crypto, linux-api, x86, Linus Torvalds,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Jann Horn, Christian Brauner,
David Hildenbrand, linux-kselftest
In-Reply-To: <20240707002658.1917440-1-Jason@zx2c4.com>
This adds two tests for vgetrandom. The first one, vdso_test_chacha,
simply checks that the assembly implementation of chacha20 matches that
of libsodium, a basic sanity check that should catch most errors. The
second, vdso_test_getrandom, is a full "libc-like" implementation of the
userspace side of vgetrandom() support. It's meant to be used also as
example code for libcs that might be integrating this.
Cc: linux-kselftest@vger.kernel.org
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
tools/include/asm/rwonce.h | 0
tools/testing/selftests/vDSO/.gitignore | 2 +
tools/testing/selftests/vDSO/Makefile | 15 +
.../testing/selftests/vDSO/vdso_test_chacha.c | 43 +++
.../selftests/vDSO/vdso_test_getrandom.c | 288 ++++++++++++++++++
5 files changed, 348 insertions(+)
create mode 100644 tools/include/asm/rwonce.h
create mode 100644 tools/testing/selftests/vDSO/vdso_test_chacha.c
create mode 100644 tools/testing/selftests/vDSO/vdso_test_getrandom.c
diff --git a/tools/include/asm/rwonce.h b/tools/include/asm/rwonce.h
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tools/testing/selftests/vDSO/.gitignore b/tools/testing/selftests/vDSO/.gitignore
index a8dc51af5a9c..30d5c8f0e5c7 100644
--- a/tools/testing/selftests/vDSO/.gitignore
+++ b/tools/testing/selftests/vDSO/.gitignore
@@ -6,3 +6,5 @@ vdso_test_correctness
vdso_test_gettimeofday
vdso_test_getcpu
vdso_standalone_test_x86
+vdso_test_getrandom
+vdso_test_chacha
diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index d53a4d8008f9..12fdae3b3201 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -3,6 +3,7 @@ include ../lib.mk
uname_M := $(shell uname -m 2>/dev/null || echo not)
ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
+SODIUM := $(shell pkg-config --libs libsodium 2>/dev/null)
TEST_GEN_PROGS := $(OUTPUT)/vdso_test_gettimeofday $(OUTPUT)/vdso_test_getcpu
TEST_GEN_PROGS += $(OUTPUT)/vdso_test_abi
@@ -11,9 +12,19 @@ ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64))
TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86
endif
TEST_GEN_PROGS += $(OUTPUT)/vdso_test_correctness
+ifeq ($(uname_M),x86_64)
+TEST_GEN_PROGS += $(OUTPUT)/vdso_test_getrandom
+ifneq ($(SODIUM),)
+TEST_GEN_PROGS += $(OUTPUT)/vdso_test_chacha
+endif
+endif
CFLAGS := -std=gnu99
CFLAGS_vdso_standalone_test_x86 := -nostdlib -fno-asynchronous-unwind-tables -fno-stack-protector
+CFLAGS_vdso_test_getrandom := -isystem $(top_srcdir)/tools/include -isystem $(top_srcdir)/include/uapi
+CFLAGS_vdso_test_chacha := $(SODIUM) -idirafter $(top_srcdir)/tools/include -isystem $(top_srcdir)/include \
+ -isystem $(top_srcdir)/arch/$(ARCH)/include \
+ -D__ASSEMBLY__ -DBULID_VDSO -DCONFIG_FUNCTION_ALIGNMENT=0 -Wa,--noexecstack
LDFLAGS_vdso_test_correctness := -ldl
ifeq ($(CONFIG_X86_32),y)
LDLIBS += -lgcc_s
@@ -33,3 +44,7 @@ $(OUTPUT)/vdso_test_correctness: vdso_test_correctness.c
vdso_test_correctness.c \
-o $@ \
$(LDFLAGS_vdso_test_correctness)
+$(OUTPUT)/vdso_test_getrandom: CFLAGS += $(CFLAGS_vdso_test_getrandom)
+$(OUTPUT)/vdso_test_getrandom: parse_vdso.c
+$(OUTPUT)/vdso_test_chacha: CFLAGS += $(CFLAGS_vdso_test_chacha)
+$(OUTPUT)/vdso_test_chacha: $(top_srcdir)/arch/$(ARCH)/entry/vdso/vgetrandom-chacha.S
diff --git a/tools/testing/selftests/vDSO/vdso_test_chacha.c b/tools/testing/selftests/vDSO/vdso_test_chacha.c
new file mode 100644
index 000000000000..e38f44e5f803
--- /dev/null
+++ b/tools/testing/selftests/vDSO/vdso_test_chacha.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022-2024 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+#include <sodium/crypto_stream_chacha20.h>
+#include <sys/random.h>
+#include <string.h>
+#include <stdint.h>
+#include "../kselftest.h"
+
+extern void __arch_chacha20_blocks_nostack(uint8_t *dst_bytes, const uint8_t *key, uint32_t *counter, size_t nblocks);
+
+int main(int argc, char *argv[])
+{
+ enum { TRIALS = 1000, BLOCKS = 128, BLOCK_SIZE = 64 };
+ static const uint8_t nonce[8] = { 0 };
+ uint32_t counter[2];
+ uint8_t key[32];
+ uint8_t output1[BLOCK_SIZE * BLOCKS], output2[BLOCK_SIZE * BLOCKS];
+
+ ksft_print_header();
+ ksft_set_plan(1);
+
+ for (unsigned int trial = 0; trial < TRIALS; ++trial) {
+ if (getrandom(key, sizeof(key), 0) != sizeof(key)) {
+ printf("getrandom() failed!\n");
+ return KSFT_SKIP;
+ }
+ crypto_stream_chacha20(output1, sizeof(output1), nonce, key);
+ for (unsigned int split = 0; split < BLOCKS; ++split) {
+ memset(output2, 'X', sizeof(output2));
+ memset(counter, 0, sizeof(counter));
+ if (split)
+ __arch_chacha20_blocks_nostack(output2, key, counter, split);
+ __arch_chacha20_blocks_nostack(output2 + split * BLOCK_SIZE, key, counter, BLOCKS - split);
+ if (memcmp(output1, output2, sizeof(output1)))
+ return KSFT_FAIL;
+ }
+ }
+ ksft_test_result_pass("chacha: PASS\n");
+ return KSFT_PASS;
+}
diff --git a/tools/testing/selftests/vDSO/vdso_test_getrandom.c b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
new file mode 100644
index 000000000000..69f5833590d2
--- /dev/null
+++ b/tools/testing/selftests/vDSO/vdso_test_getrandom.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022-2024 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+#include <assert.h>
+#include <pthread.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <time.h>
+#include <unistd.h>
+#include <signal.h>
+#include <sys/auxv.h>
+#include <sys/mman.h>
+#include <sys/random.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <linux/random.h>
+
+#include "../kselftest.h"
+#include "parse_vdso.h"
+
+#ifndef timespecsub
+#define timespecsub(tsp, usp, vsp) \
+ do { \
+ (vsp)->tv_sec = (tsp)->tv_sec - (usp)->tv_sec; \
+ (vsp)->tv_nsec = (tsp)->tv_nsec - (usp)->tv_nsec; \
+ if ((vsp)->tv_nsec < 0) { \
+ (vsp)->tv_sec--; \
+ (vsp)->tv_nsec += 1000000000L; \
+ } \
+ } while (0)
+#endif
+
+static struct {
+ pthread_mutex_t lock;
+ void **states;
+ size_t len, cap;
+} grnd_allocator = {
+ .lock = PTHREAD_MUTEX_INITIALIZER
+};
+
+static struct {
+ ssize_t(*fn)(void *, size_t, unsigned long, void *, size_t);
+ pthread_key_t key;
+ pthread_once_t initialized;
+ struct vgetrandom_opaque_params params;
+} grnd_ctx = {
+ .initialized = PTHREAD_ONCE_INIT
+};
+
+static void *vgetrandom_get_state(void)
+{
+ void *state = NULL;
+
+ pthread_mutex_lock(&grnd_allocator.lock);
+ if (!grnd_allocator.len) {
+ size_t page_size = getpagesize();
+ size_t new_cap;
+ size_t alloc_size, num = sysconf(_SC_NPROCESSORS_ONLN); /* Just a decent heuristic. */
+ void *new_block, *new_states;
+
+ alloc_size = (num * grnd_ctx.params.size_of_opaque_state + page_size - 1) & (~(page_size - 1));
+ num = (page_size / grnd_ctx.params.size_of_opaque_state) * (alloc_size / page_size);
+ new_block = mmap(0, alloc_size, grnd_ctx.params.mmap_prot, grnd_ctx.params.mmap_flags, -1, 0);
+ if (new_block == MAP_FAILED)
+ return NULL;
+
+ new_cap = grnd_allocator.cap + num;
+ new_states = reallocarray(grnd_allocator.states, new_cap, sizeof(*grnd_allocator.states));
+ if (!new_states)
+ goto unmap;
+ grnd_allocator.cap = new_cap;
+ grnd_allocator.states = new_states;
+
+ for (size_t i = 0; i < num; ++i) {
+ if (((uintptr_t)new_block & (page_size - 1)) + grnd_ctx.params.size_of_opaque_state > page_size)
+ new_block = (void *)(((uintptr_t)new_block + page_size - 1) & (~(page_size - 1)));
+ grnd_allocator.states[i] = new_block;
+ new_block += grnd_ctx.params.size_of_opaque_state;
+ }
+ grnd_allocator.len = num;
+ goto success;
+
+ unmap:
+ munmap(new_block, alloc_size);
+ goto out;
+ }
+success:
+ state = grnd_allocator.states[--grnd_allocator.len];
+
+out:
+ pthread_mutex_unlock(&grnd_allocator.lock);
+ return state;
+}
+
+static void vgetrandom_put_state(void *state)
+{
+ if (!state)
+ return;
+ pthread_mutex_lock(&grnd_allocator.lock);
+ grnd_allocator.states[grnd_allocator.len++] = state;
+ pthread_mutex_unlock(&grnd_allocator.lock);
+}
+
+static void vgetrandom_init(void)
+{
+ if (pthread_key_create(&grnd_ctx.key, vgetrandom_put_state) != 0)
+ return;
+ unsigned long sysinfo_ehdr = getauxval(AT_SYSINFO_EHDR);
+ if (!sysinfo_ehdr) {
+ printf("AT_SYSINFO_EHDR is not present!\n");
+ exit(KSFT_SKIP);
+ }
+ vdso_init_from_sysinfo_ehdr(sysinfo_ehdr);
+ grnd_ctx.fn = (__typeof__(grnd_ctx.fn))vdso_sym("LINUX_2.6", "__vdso_getrandom");
+ if (!grnd_ctx.fn) {
+ printf("__vdso_getrandom is missing!\n");
+ exit(KSFT_FAIL);
+ }
+ if (grnd_ctx.fn(NULL, 0, 0, &grnd_ctx.params, ~0UL) != 0) {
+ printf("failed to fetch vgetrandom params!\n");
+ exit(KSFT_FAIL);
+ }
+}
+
+static ssize_t vgetrandom(void *buf, size_t len, unsigned long flags)
+{
+ void *state;
+
+ pthread_once(&grnd_ctx.initialized, vgetrandom_init);
+ state = pthread_getspecific(grnd_ctx.key);
+ if (!state) {
+ state = vgetrandom_get_state();
+ if (pthread_setspecific(grnd_ctx.key, state) != 0) {
+ vgetrandom_put_state(state);
+ state = NULL;
+ }
+ if (!state) {
+ printf("vgetrandom_get_state failed!\n");
+ exit(KSFT_FAIL);
+ }
+ }
+ return grnd_ctx.fn(buf, len, flags, state, grnd_ctx.params.size_of_opaque_state);
+}
+
+enum { TRIALS = 25000000, THREADS = 256 };
+
+static void *test_vdso_getrandom(void *)
+{
+ for (size_t i = 0; i < TRIALS; ++i) {
+ unsigned int val;
+ ssize_t ret = vgetrandom(&val, sizeof(val), 0);
+ assert(ret == sizeof(val));
+ }
+ return NULL;
+}
+
+static void *test_libc_getrandom(void *)
+{
+ for (size_t i = 0; i < TRIALS; ++i) {
+ unsigned int val;
+ ssize_t ret = getrandom(&val, sizeof(val), 0);
+ assert(ret == sizeof(val));
+ }
+ return NULL;
+}
+
+static void *test_syscall_getrandom(void *)
+{
+ for (size_t i = 0; i < TRIALS; ++i) {
+ unsigned int val;
+ ssize_t ret = syscall(__NR_getrandom, &val, sizeof(val), 0);
+ assert(ret == sizeof(val));
+ }
+ return NULL;
+}
+
+static void bench_single(void)
+{
+ struct timespec start, end, diff;
+
+ clock_gettime(CLOCK_MONOTONIC, &start);
+ test_vdso_getrandom(NULL);
+ clock_gettime(CLOCK_MONOTONIC, &end);
+ timespecsub(&end, &start, &diff);
+ printf(" vdso: %u times in %lu.%09lu seconds\n", TRIALS, diff.tv_sec, diff.tv_nsec);
+
+ clock_gettime(CLOCK_MONOTONIC, &start);
+ test_libc_getrandom(NULL);
+ clock_gettime(CLOCK_MONOTONIC, &end);
+ timespecsub(&end, &start, &diff);
+ printf(" libc: %u times in %lu.%09lu seconds\n", TRIALS, diff.tv_sec, diff.tv_nsec);
+
+ clock_gettime(CLOCK_MONOTONIC, &start);
+ test_syscall_getrandom(NULL);
+ clock_gettime(CLOCK_MONOTONIC, &end);
+ timespecsub(&end, &start, &diff);
+ printf("syscall: %u times in %lu.%09lu seconds\n", TRIALS, diff.tv_sec, diff.tv_nsec);
+}
+
+static void bench_multi(void)
+{
+ struct timespec start, end, diff;
+ pthread_t threads[THREADS];
+
+ clock_gettime(CLOCK_MONOTONIC, &start);
+ for (size_t i = 0; i < THREADS; ++i)
+ assert(pthread_create(&threads[i], NULL, test_vdso_getrandom, NULL) == 0);
+ for (size_t i = 0; i < THREADS; ++i)
+ pthread_join(threads[i], NULL);
+ clock_gettime(CLOCK_MONOTONIC, &end);
+ timespecsub(&end, &start, &diff);
+ printf(" vdso: %u x %u times in %lu.%09lu seconds\n", TRIALS, THREADS, diff.tv_sec, diff.tv_nsec);
+
+ clock_gettime(CLOCK_MONOTONIC, &start);
+ for (size_t i = 0; i < THREADS; ++i)
+ assert(pthread_create(&threads[i], NULL, test_libc_getrandom, NULL) == 0);
+ for (size_t i = 0; i < THREADS; ++i)
+ pthread_join(threads[i], NULL);
+ clock_gettime(CLOCK_MONOTONIC, &end);
+ timespecsub(&end, &start, &diff);
+ printf(" libc: %u x %u times in %lu.%09lu seconds\n", TRIALS, THREADS, diff.tv_sec, diff.tv_nsec);
+
+ clock_gettime(CLOCK_MONOTONIC, &start);
+ for (size_t i = 0; i < THREADS; ++i)
+ assert(pthread_create(&threads[i], NULL, test_syscall_getrandom, NULL) == 0);
+ for (size_t i = 0; i < THREADS; ++i)
+ pthread_join(threads[i], NULL);
+ clock_gettime(CLOCK_MONOTONIC, &end);
+ timespecsub(&end, &start, &diff);
+ printf(" syscall: %u x %u times in %lu.%09lu seconds\n", TRIALS, THREADS, diff.tv_sec, diff.tv_nsec);
+}
+
+static void fill(void)
+{
+ uint8_t weird_size[323929];
+ for (;;)
+ vgetrandom(weird_size, sizeof(weird_size), 0);
+}
+
+static void kselftest(void)
+{
+ uint8_t weird_size[1263];
+
+ ksft_print_header();
+ ksft_set_plan(1);
+
+ for (size_t i = 0; i < 1000; ++i) {
+ ssize_t ret = vgetrandom(weird_size, sizeof(weird_size), 0);
+ if (ret != sizeof(weird_size))
+ exit(KSFT_FAIL);
+ }
+
+ ksft_test_result_pass("getrandom: PASS\n");
+ exit(KSFT_PASS);
+}
+
+static void usage(const char *argv0)
+{
+ fprintf(stderr, "Usage: %s [bench-single|bench-multi|fill]\n", argv0);
+}
+
+int main(int argc, char *argv[])
+{
+ if (argc == 1) {
+ kselftest();
+ return 0;
+ }
+
+ if (argc != 2) {
+ usage(argv[0]);
+ return 1;
+ }
+ if (!strcmp(argv[1], "bench-single"))
+ bench_single();
+ else if (!strcmp(argv[1], "bench-multi"))
+ bench_multi();
+ else if (!strcmp(argv[1], "fill"))
+ fill();
+ else {
+ usage(argv[0]);
+ return 1;
+ }
+ return 0;
+}
--
2.45.2
^ permalink raw reply related
* [PATCH v21 3/4] x86: vdso: Wire up getrandom() vDSO implementation
From: Jason A. Donenfeld @ 2024-07-07 0:26 UTC (permalink / raw)
To: linux-kernel, patches, tglx
Cc: Jason A. Donenfeld, linux-crypto, linux-api, x86, Linus Torvalds,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Jann Horn, Christian Brauner,
David Hildenbrand, Samuel Neves
In-Reply-To: <20240707002658.1917440-1-Jason@zx2c4.com>
Hook up the generic vDSO implementation to the x86 vDSO data page. Since
the existing vDSO infrastructure is heavily based on the timekeeping
functionality, which works over arrays of bases, a new macro is
introduced for vvars that are not arrays.
The vDSO function requires a ChaCha20 implementation that does not write
to the stack, yet can still do an entire ChaCha20 permutation, so
provide this using SSE2, since this is userland code that must work on
all x86-64 processors.
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Samuel Neves <sneves@dei.uc.pt> # for vgetrandom-chacha.S
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
MAINTAINERS | 2 +
arch/x86/Kconfig | 1 +
arch/x86/entry/vdso/Makefile | 3 +-
arch/x86/entry/vdso/vdso.lds.S | 2 +
arch/x86/entry/vdso/vgetrandom-chacha.S | 178 ++++++++++++++++++++++++
arch/x86/entry/vdso/vgetrandom.c | 17 +++
arch/x86/include/asm/vdso/getrandom.h | 55 ++++++++
arch/x86/include/asm/vdso/vsyscall.h | 2 +
arch/x86/include/asm/vvar.h | 16 +++
9 files changed, 275 insertions(+), 1 deletion(-)
create mode 100644 arch/x86/entry/vdso/vgetrandom-chacha.S
create mode 100644 arch/x86/entry/vdso/vgetrandom.c
create mode 100644 arch/x86/include/asm/vdso/getrandom.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 798158329ad8..00cf0362482b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18749,6 +18749,8 @@ F: drivers/char/random.c
F: drivers/virt/vmgenid.c
F: include/vdso/getrandom.h
F: lib/vdso/getrandom.c
+F: arch/x86/entry/vdso/vgetrandom*
+F: arch/x86/include/asm/vdso/getrandom*
RAPIDIO SUBSYSTEM
M: Matt Porter <mporter@kernel.crashing.org>
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 1d7122a1883e..9c98b7a88cc2 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -287,6 +287,7 @@ config X86
select HAVE_UNSTABLE_SCHED_CLOCK
select HAVE_USER_RETURN_NOTIFIER
select HAVE_GENERIC_VDSO
+ select VDSO_GETRANDOM if X86_64
select HOTPLUG_PARALLEL if SMP && X86_64
select HOTPLUG_SMT if SMP
select HOTPLUG_SPLIT_STARTUP if SMP && X86_32
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index 215a1b202a91..c9216ac4fb1e 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -7,7 +7,7 @@
include $(srctree)/lib/vdso/Makefile
# Files to link into the vDSO:
-vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o
+vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o vgetrandom.o vgetrandom-chacha.o
vobjs32-y := vdso32/note.o vdso32/system_call.o vdso32/sigreturn.o
vobjs32-y += vdso32/vclock_gettime.o vdso32/vgetcpu.o
vobjs-$(CONFIG_X86_SGX) += vsgx.o
@@ -73,6 +73,7 @@ CFLAGS_REMOVE_vdso32/vclock_gettime.o = -pg
CFLAGS_REMOVE_vgetcpu.o = -pg
CFLAGS_REMOVE_vdso32/vgetcpu.o = -pg
CFLAGS_REMOVE_vsgx.o = -pg
+CFLAGS_REMOVE_vgetrandom.o = -pg
#
# X32 processes use x32 vDSO to access 64bit kernel data.
diff --git a/arch/x86/entry/vdso/vdso.lds.S b/arch/x86/entry/vdso/vdso.lds.S
index e8c60ae7a7c8..0bab5f4af6d1 100644
--- a/arch/x86/entry/vdso/vdso.lds.S
+++ b/arch/x86/entry/vdso/vdso.lds.S
@@ -30,6 +30,8 @@ VERSION {
#ifdef CONFIG_X86_SGX
__vdso_sgx_enter_enclave;
#endif
+ getrandom;
+ __vdso_getrandom;
local: *;
};
}
diff --git a/arch/x86/entry/vdso/vgetrandom-chacha.S b/arch/x86/entry/vdso/vgetrandom-chacha.S
new file mode 100644
index 000000000000..bcba5639b8ee
--- /dev/null
+++ b/arch/x86/entry/vdso/vgetrandom-chacha.S
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022-2024 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+#include <linux/linkage.h>
+#include <asm/frame.h>
+
+.section .rodata, "a"
+.align 16
+CONSTANTS: .octa 0x6b20657479622d323320646e61707865
+.text
+
+/*
+ * Very basic SSE2 implementation of ChaCha20. Produces a given positive number
+ * of blocks of output with a nonce of 0, taking an input key and 8-byte
+ * counter. Importantly does not spill to the stack. Its arguments are:
+ *
+ * rdi: output bytes
+ * rsi: 32-byte key input
+ * rdx: 8-byte counter input/output
+ * rcx: number of 64-byte blocks to write to output
+ */
+SYM_FUNC_START(__arch_chacha20_blocks_nostack)
+
+.set output, %rdi
+.set key, %rsi
+.set counter, %rdx
+.set nblocks, %rcx
+.set i, %al
+/* xmm registers are *not* callee-save. */
+.set temp, %xmm0
+.set state0, %xmm1
+.set state1, %xmm2
+.set state2, %xmm3
+.set state3, %xmm4
+.set copy0, %xmm5
+.set copy1, %xmm6
+.set copy2, %xmm7
+.set copy3, %xmm8
+.set one, %xmm9
+
+ /* copy0 = "expand 32-byte k" */
+ movaps CONSTANTS(%rip),copy0
+ /* copy1,copy2 = key */
+ movups 0x00(key),copy1
+ movups 0x10(key),copy2
+ /* copy3 = counter || zero nonce */
+ movq 0x00(counter),copy3
+ /* one = 1 || 0 */
+ movq $1,%rax
+ movq %rax,one
+
+.Lblock:
+ /* state0,state1,state2,state3 = copy0,copy1,copy2,copy3 */
+ movdqa copy0,state0
+ movdqa copy1,state1
+ movdqa copy2,state2
+ movdqa copy3,state3
+
+ movb $10,i
+.Lpermute:
+ /* state0 += state1, state3 = rotl32(state3 ^ state0, 16) */
+ paddd state1,state0
+ pxor state0,state3
+ movdqa state3,temp
+ pslld $16,temp
+ psrld $16,state3
+ por temp,state3
+
+ /* state2 += state3, state1 = rotl32(state1 ^ state2, 12) */
+ paddd state3,state2
+ pxor state2,state1
+ movdqa state1,temp
+ pslld $12,temp
+ psrld $20,state1
+ por temp,state1
+
+ /* state0 += state1, state3 = rotl32(state3 ^ state0, 8) */
+ paddd state1,state0
+ pxor state0,state3
+ movdqa state3,temp
+ pslld $8,temp
+ psrld $24,state3
+ por temp,state3
+
+ /* state2 += state3, state1 = rotl32(state1 ^ state2, 7) */
+ paddd state3,state2
+ pxor state2,state1
+ movdqa state1,temp
+ pslld $7,temp
+ psrld $25,state1
+ por temp,state1
+
+ /* state1[0,1,2,3] = state1[1,2,3,0] */
+ pshufd $0x39,state1,state1
+ /* state2[0,1,2,3] = state2[2,3,0,1] */
+ pshufd $0x4e,state2,state2
+ /* state3[0,1,2,3] = state3[3,0,1,2] */
+ pshufd $0x93,state3,state3
+
+ /* state0 += state1, state3 = rotl32(state3 ^ state0, 16) */
+ paddd state1,state0
+ pxor state0,state3
+ movdqa state3,temp
+ pslld $16,temp
+ psrld $16,state3
+ por temp,state3
+
+ /* state2 += state3, state1 = rotl32(state1 ^ state2, 12) */
+ paddd state3,state2
+ pxor state2,state1
+ movdqa state1,temp
+ pslld $12,temp
+ psrld $20,state1
+ por temp,state1
+
+ /* state0 += state1, state3 = rotl32(state3 ^ state0, 8) */
+ paddd state1,state0
+ pxor state0,state3
+ movdqa state3,temp
+ pslld $8,temp
+ psrld $24,state3
+ por temp,state3
+
+ /* state2 += state3, state1 = rotl32(state1 ^ state2, 7) */
+ paddd state3,state2
+ pxor state2,state1
+ movdqa state1,temp
+ pslld $7,temp
+ psrld $25,state1
+ por temp,state1
+
+ /* state1[0,1,2,3] = state1[3,0,1,2] */
+ pshufd $0x93,state1,state1
+ /* state2[0,1,2,3] = state2[2,3,0,1] */
+ pshufd $0x4e,state2,state2
+ /* state3[0,1,2,3] = state3[1,2,3,0] */
+ pshufd $0x39,state3,state3
+
+ decb i
+ jnz .Lpermute
+
+ /* output0 = state0 + copy0 */
+ paddd copy0,state0
+ movups state0,0x00(output)
+ /* output1 = state1 + copy1 */
+ paddd copy1,state1
+ movups state1,0x10(output)
+ /* output2 = state2 + copy2 */
+ paddd copy2,state2
+ movups state2,0x20(output)
+ /* output3 = state3 + copy3 */
+ paddd copy3,state3
+ movups state3,0x30(output)
+
+ /* ++copy3.counter */
+ paddq one,copy3
+
+ /* output += 64, --nblocks */
+ addq $64,output
+ decq nblocks
+ jnz .Lblock
+
+ /* counter = copy3.counter */
+ movq copy3,0x00(counter)
+
+ /* Zero out the potentially sensitive regs, in case nothing uses these again. */
+ pxor state0,state0
+ pxor state1,state1
+ pxor state2,state2
+ pxor state3,state3
+ pxor copy1,copy1
+ pxor copy2,copy2
+ pxor temp,temp
+
+ ret
+SYM_FUNC_END(__arch_chacha20_blocks_nostack)
diff --git a/arch/x86/entry/vdso/vgetrandom.c b/arch/x86/entry/vdso/vgetrandom.c
new file mode 100644
index 000000000000..52d3c7faae2e
--- /dev/null
+++ b/arch/x86/entry/vdso/vgetrandom.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022-2024 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+#include <linux/types.h>
+
+#include "../../../../lib/vdso/getrandom.c"
+
+ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len);
+
+ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len)
+{
+ return __cvdso_getrandom(buffer, len, flags, opaque_state, opaque_len);
+}
+
+ssize_t getrandom(void *, size_t, unsigned int, void *, size_t)
+ __attribute__((weak, alias("__vdso_getrandom")));
diff --git a/arch/x86/include/asm/vdso/getrandom.h b/arch/x86/include/asm/vdso/getrandom.h
new file mode 100644
index 000000000000..b96e674cafde
--- /dev/null
+++ b/arch/x86/include/asm/vdso/getrandom.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022-2024 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+#ifndef __ASM_VDSO_GETRANDOM_H
+#define __ASM_VDSO_GETRANDOM_H
+
+#ifndef __ASSEMBLY__
+
+#include <asm/unistd.h>
+#include <asm/vvar.h>
+
+/**
+ * getrandom_syscall - Invoke the getrandom() syscall.
+ * @buffer: Destination buffer to fill with random bytes.
+ * @len: Size of @buffer in bytes.
+ * @flags: Zero or more GRND_* flags.
+ * Returns: The number of random bytes written to @buffer, or a negative value indicating an error.
+ */
+static __always_inline ssize_t getrandom_syscall(void *buffer, size_t len, unsigned int flags)
+{
+ long ret;
+
+ asm ("syscall" : "=a" (ret) :
+ "0" (__NR_getrandom), "D" (buffer), "S" (len), "d" (flags) :
+ "rcx", "r11", "memory");
+
+ return ret;
+}
+
+#define __vdso_rng_data (VVAR(_vdso_rng_data))
+
+static __always_inline const struct vdso_rng_data *__arch_get_vdso_rng_data(void)
+{
+ if (IS_ENABLED(CONFIG_TIME_NS) && __vdso_data->clock_mode == VDSO_CLOCKMODE_TIMENS)
+ return (void *)&__vdso_rng_data + ((void *)&__timens_vdso_data - (void *)&__vdso_data);
+ return &__vdso_rng_data;
+}
+
+/**
+ * __arch_chacha20_blocks_nostack - Generate ChaCha20 stream without using the stack.
+ * @dst_bytes: Destination buffer to hold @nblocks * 64 bytes of output.
+ * @key: 32-byte input key.
+ * @counter: 8-byte counter, read on input and updated on return.
+ * @nblocks: Number of blocks to generate.
+ *
+ * Generates a given positive number of blocks of ChaCha20 output with nonce=0, and does not write
+ * to any stack or memory outside of the parameters passed to it, in order to mitigate stack data
+ * leaking into forked child processes.
+ */
+extern void __arch_chacha20_blocks_nostack(u8 *dst_bytes, const u32 *key, u32 *counter, size_t nblocks);
+
+#endif /* !__ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_GETRANDOM_H */
diff --git a/arch/x86/include/asm/vdso/vsyscall.h b/arch/x86/include/asm/vdso/vsyscall.h
index be199a9b2676..71c56586a22f 100644
--- a/arch/x86/include/asm/vdso/vsyscall.h
+++ b/arch/x86/include/asm/vdso/vsyscall.h
@@ -11,6 +11,8 @@
#include <asm/vvar.h>
DEFINE_VVAR(struct vdso_data, _vdso_data);
+DEFINE_VVAR_SINGLE(struct vdso_rng_data, _vdso_rng_data);
+
/*
* Update the vDSO data page to keep in sync with kernel timekeeping.
*/
diff --git a/arch/x86/include/asm/vvar.h b/arch/x86/include/asm/vvar.h
index 183e98e49ab9..9d9af37f7cab 100644
--- a/arch/x86/include/asm/vvar.h
+++ b/arch/x86/include/asm/vvar.h
@@ -26,6 +26,8 @@
*/
#define DECLARE_VVAR(offset, type, name) \
EMIT_VVAR(name, offset)
+#define DECLARE_VVAR_SINGLE(offset, type, name) \
+ EMIT_VVAR(name, offset)
#else
@@ -37,6 +39,10 @@ extern char __vvar_page;
extern type timens_ ## name[CS_BASES] \
__attribute__((visibility("hidden"))); \
+#define DECLARE_VVAR_SINGLE(offset, type, name) \
+ extern type vvar_ ## name \
+ __attribute__((visibility("hidden"))); \
+
#define VVAR(name) (vvar_ ## name)
#define TIMENS(name) (timens_ ## name)
@@ -44,12 +50,22 @@ extern char __vvar_page;
type name[CS_BASES] \
__attribute__((section(".vvar_" #name), aligned(16))) __visible
+#define DEFINE_VVAR_SINGLE(type, name) \
+ type name \
+ __attribute__((section(".vvar_" #name), aligned(16))) __visible
+
#endif
/* DECLARE_VVAR(offset, type, name) */
DECLARE_VVAR(128, struct vdso_data, _vdso_data)
+#if !defined(_SINGLE_DATA)
+#define _SINGLE_DATA
+DECLARE_VVAR_SINGLE(640, struct vdso_rng_data, _vdso_rng_data)
+#endif
+
#undef DECLARE_VVAR
+#undef DECLARE_VVAR_SINGLE
#endif
--
2.45.2
^ permalink raw reply related
* [PATCH v21 2/4] random: introduce generic vDSO getrandom() implementation
From: Jason A. Donenfeld @ 2024-07-07 0:26 UTC (permalink / raw)
To: linux-kernel, patches, tglx
Cc: Jason A. Donenfeld, linux-crypto, linux-api, x86, Linus Torvalds,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Jann Horn, Christian Brauner,
David Hildenbrand
In-Reply-To: <20240707002658.1917440-1-Jason@zx2c4.com>
Provide a generic C vDSO getrandom() implementation, which operates on
an opaque state returned by vgetrandom_alloc() and produces random bytes
the same way as getrandom(). This has the following API signature:
ssize_t vgetrandom(void *buffer, size_t len, unsigned int flags,
void *opaque_state, size_t opaque_len);
The return value and the first three arguments are the same as ordinary
getrandom(), while the last two arguments are a pointer to the opaque
allocated state and its size. Were all five arguments passed to the
getrandom() syscall, nothing different would happen, and the functions
would have the exact same behavior.
The actual vDSO RNG algorithm implemented is the same one implemented by
drivers/char/random.c, using the same fast-erasure techniques as that.
Should the in-kernel implementation change, so too will the vDSO one.
It requires an implementation of ChaCha20 that does not use any stack,
in order to maintain forward secrecy if a multi-threaded program forks
(though this does not account for a similar issue with SA_SIGINFO
copying registers to the stack), so this is left as an
architecture-specific fill-in. Stack-less ChaCha20 is an easy algorithm
to implement on a variety of architectures, so this shouldn't be too
onerous.
Initially, the state is keyless, and so the first call makes a
getrandom() syscall to generate that key, and then uses it for
subsequent calls. By keeping track of a generation counter, it knows
when its key is invalidated and it should fetch a new one using the
syscall. Later, more than just a generation counter might be used.
Since MADV_WIPEONFORK is set on the opaque state, the key and related
state is wiped during a fork(), so secrets don't roll over into new
processes, and the same state doesn't accidentally generate the same
random stream. The generation counter, as well, is always >0, so that
the 0 counter is a useful indication of a fork() or otherwise
uninitialized state.
If the kernel RNG is not yet initialized, then the vDSO always calls the
syscall, because that behavior cannot be emulated in userspace, but
fortunately that state is short lived and only during early boot. If it
has been initialized, then there is no need to inspect the `flags`
argument, because the behavior does not change post-initialization
regardless of the `flags` value.
Since the opaque state passed to it is mutated, vDSO getrandom() is not
reentrant, when used with the same opaque state, which libc should be
mindful of.
The function works over an opaque per-thread state of a particular size,
which must be marked VM_WIPEONFORK, VM_DONTDUMP, VM_NORESERVE, and
VM_DROPPABLE for proper operation. Over time, the nuances of these
allocations may change or grow or even differ based on architectural
features.
The opaque passed to vDSO getrandom() must be allocated using the
mmap_flags and mmap_prot parameters provided by the vgetrandom_opaque_params
struct, which also contains the size of each state. Then, libc can call
mmap(2) and slice up the returned array into a state per each thread,
while ensuring that no single state straddles a page boundary. Libc is
expected to allocate a chunk of these on first use, and then dole them
out to threads as they're created, allocating more when needed.
vDSO getrandom() provides the ability for userspace to generate random
bytes quickly and safely, and is intended to be integrated into libc's
thread management. As an illustrative example, the introduced code in
the vdso_test_getrandom self test later in this series might be used to
do the same outside of libc. In a libc the various pthread-isms are
expected to be elided into libc internals.
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
MAINTAINERS | 2 +
drivers/char/random.c | 18 ++-
include/uapi/linux/random.h | 15 +++
include/vdso/datapage.h | 11 ++
include/vdso/getrandom.h | 46 +++++++
lib/vdso/Kconfig | 6 +
lib/vdso/getrandom.c | 252 ++++++++++++++++++++++++++++++++++++
7 files changed, 349 insertions(+), 1 deletion(-)
create mode 100644 include/vdso/getrandom.h
create mode 100644 lib/vdso/getrandom.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 3c4fdf74a3f9..798158329ad8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18747,6 +18747,8 @@ T: git https://git.kernel.org/pub/scm/linux/kernel/git/crng/random.git
F: Documentation/devicetree/bindings/rng/microsoft,vmgenid.yaml
F: drivers/char/random.c
F: drivers/virt/vmgenid.c
+F: include/vdso/getrandom.h
+F: lib/vdso/getrandom.c
RAPIDIO SUBSYSTEM
M: Matt Porter <mporter@kernel.crashing.org>
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 2597cb43f438..b02a12436750 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
/*
- * Copyright (C) 2017-2022 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ * Copyright (C) 2017-2024 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
* Copyright Matt Mackall <mpm@selenic.com>, 2003, 2004, 2005
* Copyright Theodore Ts'o, 1994, 1995, 1996, 1997, 1998, 1999. All rights reserved.
*
@@ -56,6 +56,10 @@
#include <linux/sched/isolation.h>
#include <crypto/chacha.h>
#include <crypto/blake2s.h>
+#ifdef CONFIG_VDSO_GETRANDOM
+#include <vdso/getrandom.h>
+#include <vdso/datapage.h>
+#endif
#include <asm/archrandom.h>
#include <asm/processor.h>
#include <asm/irq.h>
@@ -271,6 +275,15 @@ static void crng_reseed(struct work_struct *work)
if (next_gen == ULONG_MAX)
++next_gen;
WRITE_ONCE(base_crng.generation, next_gen);
+#ifdef CONFIG_VDSO_GETRANDOM
+ /* base_crng.generation's invalid value is ULONG_MAX, while
+ * _vdso_rng_data.generation's invalid value is 0, so add one to the
+ * former to arrive at the latter. Use smp_store_release so that this
+ * is ordered with the write above to base_crng.generation. Pairs with
+ * the smp_rmb() before the syscall in the vDSO code.
+ */
+ smp_store_release(&_vdso_rng_data.generation, next_gen + 1);
+#endif
if (!static_branch_likely(&crng_is_ready))
crng_init = CRNG_READY;
spin_unlock_irqrestore(&base_crng.lock, flags);
@@ -721,6 +734,9 @@ static void __cold _credit_init_bits(size_t bits)
if (static_key_initialized && system_unbound_wq)
queue_work(system_unbound_wq, &set_ready);
atomic_notifier_call_chain(&random_ready_notifier, 0, NULL);
+#ifdef CONFIG_VDSO_GETRANDOM
+ WRITE_ONCE(_vdso_rng_data.is_ready, true);
+#endif
wake_up_interruptible(&crng_init_wait);
kill_fasync(&fasync, SIGIO, POLL_IN);
pr_notice("crng init done\n");
diff --git a/include/uapi/linux/random.h b/include/uapi/linux/random.h
index e744c23582eb..2a3fe4c2cdc9 100644
--- a/include/uapi/linux/random.h
+++ b/include/uapi/linux/random.h
@@ -55,4 +55,19 @@ struct rand_pool_info {
#define GRND_RANDOM 0x0002
#define GRND_INSECURE 0x0004
+/**
+ * struct vgetrandom_opaque_params - arguments for allocating memory for vgetrandom
+ *
+ * @size_per_opaque_state: Size of each state that is to be passed to vgetrandom().
+ * @mmap_prot: Value of the prot argument in mmap(2).
+ * @mmap_flags: Value of the flags argument in mmap(2).
+ * @reserved: Reserved for future use.
+ */
+struct vgetrandom_opaque_params {
+ __u32 size_of_opaque_state;
+ __u32 mmap_prot;
+ __u32 mmap_flags;
+ __u32 reserved[13];
+};
+
#endif /* _UAPI_LINUX_RANDOM_H */
diff --git a/include/vdso/datapage.h b/include/vdso/datapage.h
index d04d394db064..05e5787beb73 100644
--- a/include/vdso/datapage.h
+++ b/include/vdso/datapage.h
@@ -113,6 +113,16 @@ struct vdso_data {
struct arch_vdso_data arch_data;
};
+/**
+ * struct vdso_rng_data - vdso RNG state information
+ * @generation: counter representing the number of RNG reseeds
+ * @is_ready: boolean signaling whether the RNG is initialized
+ */
+struct vdso_rng_data {
+ u64 generation;
+ u8 is_ready;
+};
+
/*
* We use the hidden visibility to prevent the compiler from generating a GOT
* relocation. Not only is going through a GOT useless (the entry couldn't and
@@ -124,6 +134,7 @@ struct vdso_data {
*/
extern struct vdso_data _vdso_data[CS_BASES] __attribute__((visibility("hidden")));
extern struct vdso_data _timens_data[CS_BASES] __attribute__((visibility("hidden")));
+extern struct vdso_rng_data _vdso_rng_data __attribute__((visibility("hidden")));
/**
* union vdso_data_store - Generic vDSO data page
diff --git a/include/vdso/getrandom.h b/include/vdso/getrandom.h
new file mode 100644
index 000000000000..a8b7c14b0ae0
--- /dev/null
+++ b/include/vdso/getrandom.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022-2024 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+#ifndef _VDSO_GETRANDOM_H
+#define _VDSO_GETRANDOM_H
+
+#include <linux/types.h>
+
+#define CHACHA_KEY_SIZE 32
+#define CHACHA_BLOCK_SIZE 64
+
+/**
+ * struct vgetrandom_state - State used by vDSO getrandom().
+ *
+ * @batch: One and a half ChaCha20 blocks of buffered RNG output.
+ *
+ * @key: Key to be used for generating next batch.
+ *
+ * @batch_key: Union of the prior two members, which is exactly two full
+ * ChaCha20 blocks in size, so that @batch and @key can be filled
+ * together.
+ *
+ * @generation: Snapshot of @rng_info->generation in the vDSO data page at
+ * the time @key was generated.
+ *
+ * @pos: Offset into @batch of the next available random byte.
+ *
+ * @in_use: Reentrancy guard for reusing a state within the same thread
+ * due to signal handlers.
+ */
+struct vgetrandom_state {
+ union {
+ struct {
+ u8 batch[CHACHA_BLOCK_SIZE * 3 / 2];
+ u32 key[CHACHA_KEY_SIZE / sizeof(u32)];
+ };
+ u8 batch_key[CHACHA_BLOCK_SIZE * 2];
+ };
+ u64 generation;
+ u8 pos;
+ bool in_use;
+};
+
+#endif /* _VDSO_GETRANDOM_H */
diff --git a/lib/vdso/Kconfig b/lib/vdso/Kconfig
index c46c2300517c..99661b731834 100644
--- a/lib/vdso/Kconfig
+++ b/lib/vdso/Kconfig
@@ -38,3 +38,9 @@ config GENERIC_VDSO_OVERFLOW_PROTECT
in the hotpath.
endif
+
+config VDSO_GETRANDOM
+ bool
+ select NEED_VM_DROPPABLE
+ help
+ Selected by architectures that support vDSO getrandom().
diff --git a/lib/vdso/getrandom.c b/lib/vdso/getrandom.c
new file mode 100644
index 000000000000..663610831969
--- /dev/null
+++ b/lib/vdso/getrandom.c
@@ -0,0 +1,252 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022-2024 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+#include <linux/cache.h>
+#include <linux/kernel.h>
+#include <linux/time64.h>
+#include <vdso/datapage.h>
+#include <vdso/getrandom.h>
+#include <asm/vdso/getrandom.h>
+#include <asm/vdso/vsyscall.h>
+#include <asm/unaligned.h>
+#include <uapi/linux/mman.h>
+
+#define MEMCPY_AND_ZERO_SRC(type, dst, src, len) do { \
+ while (len >= sizeof(type)) { \
+ __put_unaligned_t(type, __get_unaligned_t(type, src), dst); \
+ __put_unaligned_t(type, 0, src); \
+ dst += sizeof(type); \
+ src += sizeof(type); \
+ len -= sizeof(type); \
+ } \
+} while (0)
+
+static void memcpy_and_zero_src(void *dst, void *src, size_t len)
+{
+ if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) {
+ if (IS_ENABLED(CONFIG_64BIT))
+ MEMCPY_AND_ZERO_SRC(u64, dst, src, len);
+ MEMCPY_AND_ZERO_SRC(u32, dst, src, len);
+ MEMCPY_AND_ZERO_SRC(u16, dst, src, len);
+ }
+ MEMCPY_AND_ZERO_SRC(u8, dst, src, len);
+}
+
+/**
+ * __cvdso_getrandom_data - Generic vDSO implementation of getrandom() syscall.
+ * @rng_info: Describes state of kernel RNG, memory shared with kernel.
+ * @buffer: Destination buffer to fill with random bytes.
+ * @len: Size of @buffer in bytes.
+ * @flags: Zero or more GRND_* flags.
+ * @opaque_state: Pointer to an opaque state area.
+ * @opaque_len: Length of opaque state area.
+ *
+ * This implements a "fast key erasure" RNG using ChaCha20, in the same way that the kernel's
+ * getrandom() syscall does. It periodically reseeds its key from the kernel's RNG, at the same
+ * schedule that the kernel's RNG is reseeded. If the kernel's RNG is not ready, then this always
+ * calls into the syscall.
+ *
+ * If @buffer, @len, and @flags are 0, and @opaque_len is -1, then @opaque_state is populated
+ * with a struct vgetrandom_opaque_params and the function returns 0:
+ *
+ * @opaque_state *must* be allocated by calling mmap(2) using the mmap_prot and mmap_flags fields
+ * from the struct vgetrandom_opaque_params, and states must not straddle pages. Unless external
+ * locking is used, one state must be allocated per thread, as it is not safe to call this function
+ * concurrently with the same @opaque_state. However, it is safe to call this using the same
+ * @opaque_state that is shared between main code and signal handling code, within the same thread.
+ *
+ * Returns: The number of random bytes written to @buffer, or a negative value indicating an error.
+ */
+static __always_inline ssize_t
+__cvdso_getrandom_data(const struct vdso_rng_data *rng_info, void *buffer, size_t len,
+ unsigned int flags, void *opaque_state, size_t opaque_len)
+{
+ ssize_t ret = min_t(size_t, INT_MAX & PAGE_MASK /* = MAX_RW_COUNT */, len);
+ struct vgetrandom_state *state = opaque_state;
+ size_t batch_len, nblocks, orig_len = len;
+ bool in_use, have_retried = false;
+ unsigned long current_generation;
+ void *orig_buffer = buffer;
+ u32 counter[2] = { 0 };
+
+ if (unlikely(opaque_len == ~0UL && !buffer && !len && !flags)) {
+ *(struct vgetrandom_opaque_params *)opaque_state = (struct vgetrandom_opaque_params) {
+ .size_of_opaque_state = sizeof(*state),
+ .mmap_prot = PROT_READ | PROT_WRITE,
+ .mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS |
+ MAP_DROPPABLE | MAP_NORESERVE |
+ MAP_WIPEONFORK | MAP_DONTDUMP
+ };
+ return 0;
+ }
+
+ /* The state must not straddle a page, since pages can be zeroed at any time. */
+ if (unlikely(((unsigned long)opaque_state & ~PAGE_MASK) + sizeof(*state) > PAGE_SIZE))
+ return -EFAULT;
+
+ /* If the caller passes the wrong size, which might happen due to CRIU, fallback. */
+ if (unlikely(opaque_len != sizeof(*state)))
+ goto fallback_syscall;
+
+ /*
+ * If the kernel's RNG is not yet ready, then it's not possible to provide random bytes from
+ * userspace, because A) the various @flags require this to block, or not, depending on
+ * various factors unavailable to userspace, and B) the kernel's behavior before the RNG is
+ * ready is to reseed from the entropy pool at every invocation.
+ */
+ if (unlikely(!READ_ONCE(rng_info->is_ready)))
+ goto fallback_syscall;
+
+ /*
+ * This condition is checked after @rng_info->is_ready, because before the kernel's RNG is
+ * initialized, the @flags parameter may require this to block or return an error, even when
+ * len is zero.
+ */
+ if (unlikely(!len))
+ return 0;
+
+ /*
+ * @state->in_use is basic reentrancy protection against this running in a signal handler
+ * with the same @opaque_state, but obviously not atomic wrt multiple CPUs or more than one
+ * level of reentrancy. If a signal interrupts this after reading @state->in_use, but before
+ * writing @state->in_use, there is still no race, because the signal handler will run to
+ * its completion before returning execution.
+ */
+ in_use = READ_ONCE(state->in_use);
+ if (unlikely(in_use))
+ /* The syscall simply fills the buffer and does not touch @state, so fallback. */
+ goto fallback_syscall;
+ WRITE_ONCE(state->in_use, true);
+
+retry_generation:
+ /*
+ * @rng_info->generation must always be read here, as it serializes @state->key with the
+ * kernel's RNG reseeding schedule.
+ */
+ current_generation = READ_ONCE(rng_info->generation);
+
+ /*
+ * If @state->generation doesn't match the kernel RNG's generation, then it means the
+ * kernel's RNG has reseeded, and so @state->key is reseeded as well.
+ */
+ if (unlikely(state->generation != current_generation)) {
+ /*
+ * Write the generation before filling the key, in case of fork. If there is a fork
+ * just after this line, the parent and child will get different random bytes from
+ * the syscall, which is good. However, were this line to occur after the getrandom
+ * syscall, then both child and parent could have the same bytes and the same
+ * generation counter, so the fork would not be detected. Therefore, write
+ * @state->generation before the call to the getrandom syscall.
+ */
+ WRITE_ONCE(state->generation, current_generation);
+
+ /*
+ * Prevent the syscall from being reordered wrt current_generation. Pairs with the
+ * smp_store_release(&_vdso_rng_data.generation) in random.c.
+ */
+ smp_rmb();
+
+ /* Reseed @state->key using fresh bytes from the kernel. */
+ if (getrandom_syscall(state->key, sizeof(state->key), 0) != sizeof(state->key)) {
+ /*
+ * If the syscall failed to refresh the key, then @state->key is now
+ * invalid, so invalidate the generation so that it is not used again, and
+ * fallback to using the syscall entirely.
+ */
+ WRITE_ONCE(state->generation, 0);
+
+ /*
+ * Set @state->in_use to false only after the last write to @state in the
+ * line above.
+ */
+ WRITE_ONCE(state->in_use, false);
+
+ goto fallback_syscall;
+ }
+
+ /*
+ * Set @state->pos to beyond the end of the batch, so that the batch is refilled
+ * using the new key.
+ */
+ state->pos = sizeof(state->batch);
+ }
+
+ /* Set len to the total amount of bytes that this function is allowed to read, ret. */
+ len = ret;
+more_batch:
+ /*
+ * First use bytes out of @state->batch, which may have been filled by the last call to this
+ * function.
+ */
+ batch_len = min_t(size_t, sizeof(state->batch) - state->pos, len);
+ if (batch_len) {
+ /* Zeroing at the same time as memcpying helps preserve forward secrecy. */
+ memcpy_and_zero_src(buffer, state->batch + state->pos, batch_len);
+ state->pos += batch_len;
+ buffer += batch_len;
+ len -= batch_len;
+ }
+
+ if (!len) {
+ /* Prevent the loop from being reordered wrt ->generation. */
+ barrier();
+
+ /*
+ * Since @rng_info->generation will never be 0, re-read @state->generation, rather
+ * than using the local current_generation variable, to learn whether a fork
+ * occurred or if @state was zeroed due to memory pressure. Primarily, though, this
+ * indicates whether the kernel's RNG has reseeded, in which case generate a new key
+ * and start over.
+ */
+ if (unlikely(READ_ONCE(state->generation) != READ_ONCE(rng_info->generation))) {
+ /*
+ * Prevent this from looping forever in case of low memory or racing with a
+ * user force-reseeding the kernel's RNG using the ioctl.
+ */
+ if (have_retried) {
+ WRITE_ONCE(state->in_use, false);
+ goto fallback_syscall;
+ }
+
+ have_retried = true;
+ buffer = orig_buffer;
+ goto retry_generation;
+ }
+
+ /*
+ * Set @state->in_use to false only when there will be no more reads or writes of
+ * @state.
+ */
+ WRITE_ONCE(state->in_use, false);
+ return ret;
+ }
+
+ /* Generate blocks of RNG output directly into @buffer while there's enough room left. */
+ nblocks = len / CHACHA_BLOCK_SIZE;
+ if (nblocks) {
+ __arch_chacha20_blocks_nostack(buffer, state->key, counter, nblocks);
+ buffer += nblocks * CHACHA_BLOCK_SIZE;
+ len -= nblocks * CHACHA_BLOCK_SIZE;
+ }
+
+ BUILD_BUG_ON(sizeof(state->batch_key) % CHACHA_BLOCK_SIZE != 0);
+
+ /* Refill the batch and overwrite the key, in order to preserve forward secrecy. */
+ __arch_chacha20_blocks_nostack(state->batch_key, state->key, counter,
+ sizeof(state->batch_key) / CHACHA_BLOCK_SIZE);
+
+ /* Since the batch was just refilled, set the position back to 0 to indicate a full batch. */
+ state->pos = 0;
+ goto more_batch;
+
+fallback_syscall:
+ return getrandom_syscall(orig_buffer, orig_len, flags);
+}
+
+static __always_inline ssize_t
+__cvdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len)
+{
+ return __cvdso_getrandom_data(__arch_get_vdso_rng_data(), buffer, len, flags, opaque_state, opaque_len);
+}
--
2.45.2
^ permalink raw reply related
* [PATCH v21 1/4] mm: add VM_DROPPABLE for designating always lazily freeable mappings
From: Jason A. Donenfeld @ 2024-07-07 0:26 UTC (permalink / raw)
To: linux-kernel, patches, tglx
Cc: Jason A. Donenfeld, linux-crypto, linux-api, x86, Linus Torvalds,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Jann Horn, Christian Brauner,
David Hildenbrand, linux-mm
In-Reply-To: <20240707002658.1917440-1-Jason@zx2c4.com>
The vDSO getrandom() implementation works with a buffer allocated with a
new system call that has certain requirements:
- It shouldn't be written to core dumps.
* Easy: VM_DONTDUMP.
- It should be zeroed on fork.
* Easy: VM_WIPEONFORK.
- It shouldn't be written to swap.
* Uh-oh: mlock is rlimited.
* Uh-oh: mlock isn't inherited by forks.
It turns out that the vDSO getrandom() function has three really nice
characteristics that we can exploit to solve this problem:
1) Due to being wiped during fork(), the vDSO code is already robust to
having the contents of the pages it reads zeroed out midway through
the function's execution.
2) In the absolute worst case of whatever contingency we're coding for,
we have the option to fallback to the getrandom() syscall, and
everything is fine.
3) The buffers the function uses are only ever useful for a maximum of
60 seconds -- a sort of cache, rather than a long term allocation.
These characteristics mean that we can introduce VM_DROPPABLE, which
has the following semantics:
a) It never is written out to swap.
b) Under memory pressure, mm can just drop the pages (so that they're
zero when read back again).
c) It is inherited by fork.
d) It doesn't count against the mlock budget, since nothing is locked.
This is fairly simple to implement, with the one snag that we have to
use 64-bit VM_* flags, but this shouldn't be a problem, since the only
consumers will probably be 64-bit anyway.
This way, allocations used by vDSO getrandom() can use:
VM_DROPPABLE | VM_DONTDUMP | VM_WIPEONFORK | VM_NORESERVE
And there will be no problem with using memory when not in use, not
wiping on fork(), coredumps, or writing out to swap.
In order to let vDSO getrandom() use this, expose these via mmap(2) as
well, giving MAP_WIPEONFORK, MAP_DONTDUMP, and MAP_DROPPABLE.
Finally, the provided self test ensures that this is working as desired.
Cc: linux-mm@kvack.org
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
arch/alpha/include/uapi/asm/mman.h | 3 ++
arch/mips/include/uapi/asm/mman.h | 3 ++
arch/parisc/include/uapi/asm/mman.h | 3 ++
arch/xtensa/include/uapi/asm/mman.h | 3 ++
fs/proc/task_mmu.c | 3 ++
include/linux/mm.h | 8 +++
include/trace/events/mmflags.h | 7 +++
include/uapi/asm-generic/mman-common.h | 4 ++
mm/Kconfig | 3 ++
mm/mmap.c | 15 ++++++
mm/mprotect.c | 2 +-
mm/rmap.c | 16 ++++--
tools/include/uapi/asm-generic/mman-common.h | 4 ++
tools/testing/selftests/mm/.gitignore | 1 +
tools/testing/selftests/mm/Makefile | 1 +
tools/testing/selftests/mm/droppable.c | 53 ++++++++++++++++++++
16 files changed, 125 insertions(+), 4 deletions(-)
create mode 100644 tools/testing/selftests/mm/droppable.c
diff --git a/arch/alpha/include/uapi/asm/mman.h b/arch/alpha/include/uapi/asm/mman.h
index 763929e814e9..951c54a45676 100644
--- a/arch/alpha/include/uapi/asm/mman.h
+++ b/arch/alpha/include/uapi/asm/mman.h
@@ -31,6 +31,9 @@
#define MAP_STACK 0x80000 /* give out an address that is best suited for process/thread stacks */
#define MAP_HUGETLB 0x100000 /* create a huge page mapping */
#define MAP_FIXED_NOREPLACE 0x200000/* MAP_FIXED which doesn't unmap underlying mapping */
+#define MAP_WIPEONFORK 0x08000000 /* Zero memory in child forks. */
+#define MAP_DONTDUMP 0x10000000 /* Do not write to coredumps. */
+#define MAP_DROPPABLE 0x20000000 /* Zero memory under memory pressure. */
#define MS_ASYNC 1 /* sync memory asynchronously */
#define MS_SYNC 2 /* synchronous memory sync */
diff --git a/arch/mips/include/uapi/asm/mman.h b/arch/mips/include/uapi/asm/mman.h
index 9c48d9a21aa0..7490a28ec960 100644
--- a/arch/mips/include/uapi/asm/mman.h
+++ b/arch/mips/include/uapi/asm/mman.h
@@ -49,6 +49,9 @@
#define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
#define MAP_HUGETLB 0x80000 /* create a huge page mapping */
#define MAP_FIXED_NOREPLACE 0x100000 /* MAP_FIXED which doesn't unmap underlying mapping */
+#define MAP_WIPEONFORK 0x08000000 /* Zero memory in child forks. */
+#define MAP_DONTDUMP 0x10000000 /* Do not write to coredumps. */
+#define MAP_DROPPABLE 0x20000000 /* Zero memory under memory pressure. */
/*
* Flags for msync
diff --git a/arch/parisc/include/uapi/asm/mman.h b/arch/parisc/include/uapi/asm/mman.h
index 68c44f99bc93..ed03f1d7d06c 100644
--- a/arch/parisc/include/uapi/asm/mman.h
+++ b/arch/parisc/include/uapi/asm/mman.h
@@ -26,6 +26,9 @@
#define MAP_HUGETLB 0x80000 /* create a huge page mapping */
#define MAP_FIXED_NOREPLACE 0x100000 /* MAP_FIXED which doesn't unmap underlying mapping */
#define MAP_UNINITIALIZED 0 /* uninitialized anonymous mmap */
+#define MAP_WIPEONFORK 0x08000000 /* Zero memory in child forks. */
+#define MAP_DONTDUMP 0x10000000 /* Do not write to coredumps. */
+#define MAP_DROPPABLE 0x20000000 /* Zero memory under memory pressure. */
#define MS_SYNC 1 /* synchronous memory sync */
#define MS_ASYNC 2 /* sync memory asynchronously */
diff --git a/arch/xtensa/include/uapi/asm/mman.h b/arch/xtensa/include/uapi/asm/mman.h
index 1ff0c858544f..2e777670b7fa 100644
--- a/arch/xtensa/include/uapi/asm/mman.h
+++ b/arch/xtensa/include/uapi/asm/mman.h
@@ -58,6 +58,9 @@
#define MAP_FIXED_NOREPLACE 0x100000 /* MAP_FIXED which doesn't unmap underlying mapping */
#define MAP_UNINITIALIZED 0x4000000 /* For anonymous mmap, memory could be
* uninitialized */
+#define MAP_WIPEONFORK 0x08000000 /* Zero memory in child forks. */
+#define MAP_DONTDUMP 0x10000000 /* Do not write to coredumps. */
+#define MAP_DROPPABLE 0x20000000 /* Zero memory under memory pressure. */
/*
* Flags for msync
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 71e5039d940d..b3bd8432f869 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -709,6 +709,9 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
#endif
#ifdef CONFIG_64BIT
[ilog2(VM_SEALED)] = "sl",
+#endif
+#ifdef CONFIG_NEED_VM_DROPPABLE
+ [ilog2(VM_DROPPABLE)] = "dp",
#endif
};
size_t i;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index eb7c96d24ac0..92454a0272ce 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -321,12 +321,14 @@ extern unsigned int kobjsize(const void *objp);
#define VM_HIGH_ARCH_BIT_3 35 /* bit only usable on 64-bit architectures */
#define VM_HIGH_ARCH_BIT_4 36 /* bit only usable on 64-bit architectures */
#define VM_HIGH_ARCH_BIT_5 37 /* bit only usable on 64-bit architectures */
+#define VM_HIGH_ARCH_BIT_6 38 /* bit only usable on 64-bit architectures */
#define VM_HIGH_ARCH_0 BIT(VM_HIGH_ARCH_BIT_0)
#define VM_HIGH_ARCH_1 BIT(VM_HIGH_ARCH_BIT_1)
#define VM_HIGH_ARCH_2 BIT(VM_HIGH_ARCH_BIT_2)
#define VM_HIGH_ARCH_3 BIT(VM_HIGH_ARCH_BIT_3)
#define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4)
#define VM_HIGH_ARCH_5 BIT(VM_HIGH_ARCH_BIT_5)
+#define VM_HIGH_ARCH_6 BIT(VM_HIGH_ARCH_BIT_6)
#endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */
#ifdef CONFIG_ARCH_HAS_PKEYS
@@ -357,6 +359,12 @@ extern unsigned int kobjsize(const void *objp);
# define VM_SHADOW_STACK VM_NONE
#endif
+#ifdef CONFIG_NEED_VM_DROPPABLE
+# define VM_DROPPABLE VM_HIGH_ARCH_6
+#else
+# define VM_DROPPABLE VM_NONE
+#endif
+
#if defined(CONFIG_X86)
# define VM_PAT VM_ARCH_1 /* PAT reserves whole VMA at once (x86) */
#elif defined(CONFIG_PPC)
diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
index e46d6e82765e..fab7848df50a 100644
--- a/include/trace/events/mmflags.h
+++ b/include/trace/events/mmflags.h
@@ -165,6 +165,12 @@ IF_HAVE_PG_ARCH_X(arch_3)
# define IF_HAVE_UFFD_MINOR(flag, name)
#endif
+#ifdef CONFIG_NEED_VM_DROPPABLE
+# define IF_HAVE_VM_DROPPABLE(flag, name) {flag, name},
+#else
+# define IF_HAVE_VM_DROPPABLE(flag, name)
+#endif
+
#define __def_vmaflag_names \
{VM_READ, "read" }, \
{VM_WRITE, "write" }, \
@@ -197,6 +203,7 @@ IF_HAVE_VM_SOFTDIRTY(VM_SOFTDIRTY, "softdirty" ) \
{VM_MIXEDMAP, "mixedmap" }, \
{VM_HUGEPAGE, "hugepage" }, \
{VM_NOHUGEPAGE, "nohugepage" }, \
+IF_HAVE_VM_DROPPABLE(VM_DROPPABLE, "droppable" ) \
{VM_MERGEABLE, "mergeable" } \
#define show_vma_flags(flags) \
diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index 6ce1f1ceb432..65a3069462a8 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -33,6 +33,10 @@
#define MAP_UNINITIALIZED 0x4000000 /* For anonymous mmap, memory could be
* uninitialized */
+#define MAP_WIPEONFORK 0x08000000 /* Zero memory in child forks. */
+#define MAP_DONTDUMP 0x10000000 /* Do not write to coredumps. */
+#define MAP_DROPPABLE 0x20000000 /* Zero memory under memory pressure. */
+
/*
* Flags for mlock
*/
diff --git a/mm/Kconfig b/mm/Kconfig
index b4cb45255a54..6cd65ea4b3ad 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -1056,6 +1056,9 @@ config ARCH_USES_HIGH_VMA_FLAGS
bool
config ARCH_HAS_PKEYS
bool
+config NEED_VM_DROPPABLE
+ select ARCH_USES_HIGH_VMA_FLAGS
+ bool
config ARCH_USES_PG_ARCH_X
bool
diff --git a/mm/mmap.c b/mm/mmap.c
index 83b4682ec85c..e361f6750201 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1278,6 +1278,21 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
vm_flags |= calc_vm_prot_bits(prot, pkey) | calc_vm_flag_bits(flags) |
mm->def_flags | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC;
+ if (flags & MAP_WIPEONFORK) {
+ /* MAP_WIPEONFORK is only supported on anonymous memory. */
+ if (file || !(flags & MAP_PRIVATE))
+ return -EINVAL;
+ vm_flags |= VM_WIPEONFORK;
+ }
+ if (flags & MAP_DONTDUMP)
+ vm_flags |= VM_DONTDUMP;
+ if (flags & MAP_DROPPABLE) {
+ /* MAP_DROPPABLE is only supported on anonymous memory. */
+ if (file || !(flags & MAP_PRIVATE))
+ return -EINVAL;
+ vm_flags |= VM_DROPPABLE;
+ }
+
/* Obtain the address to map to. we verify (or select) it and ensure
* that it represents a valid section of the address space.
*/
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 8c6cd8825273..57b8dad9adcc 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -623,7 +623,7 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,
may_expand_vm(mm, oldflags, nrpages))
return -ENOMEM;
if (!(oldflags & (VM_ACCOUNT|VM_WRITE|VM_HUGETLB|
- VM_SHARED|VM_NORESERVE))) {
+ VM_SHARED|VM_NORESERVE|VM_DROPPABLE))) {
charged = nrpages;
if (security_vm_enough_memory_mm(mm, charged))
return -ENOMEM;
diff --git a/mm/rmap.c b/mm/rmap.c
index e8fc5ecb59b2..56d7535d5cf6 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1397,7 +1397,10 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
VM_WARN_ON_FOLIO(folio_test_hugetlb(folio), folio);
VM_BUG_ON_VMA(address < vma->vm_start ||
address + (nr << PAGE_SHIFT) > vma->vm_end, vma);
- __folio_set_swapbacked(folio);
+ /* VM_DROPPABLE mappings don't swap; instead they're just dropped when
+ * under memory pressure. */
+ if (!(vma->vm_flags & VM_DROPPABLE))
+ __folio_set_swapbacked(folio);
__folio_set_anon(folio, vma, address, true);
if (likely(!folio_test_large(folio))) {
@@ -1841,7 +1844,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
* plus the rmap(s) (dropped by discard:).
*/
if (ref_count == 1 + map_count &&
- !folio_test_dirty(folio)) {
+ (!folio_test_dirty(folio) ||
+ /* Unlike MADV_FREE mappings, VM_DROPPABLE
+ * ones can be dropped even if they've
+ * been dirtied. */
+ (vma->vm_flags & VM_DROPPABLE))) {
dec_mm_counter(mm, MM_ANONPAGES);
goto discard;
}
@@ -1851,7 +1858,10 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
* discarded. Remap the page to page table.
*/
set_pte_at(mm, address, pvmw.pte, pteval);
- folio_set_swapbacked(folio);
+ /* Unlike MADV_FREE mappings, VM_DROPPABLE ones
+ * never get swap backed on failure to drop. */
+ if (!(vma->vm_flags & VM_DROPPABLE))
+ folio_set_swapbacked(folio);
ret = false;
page_vma_mapped_walk_done(&pvmw);
break;
diff --git a/tools/include/uapi/asm-generic/mman-common.h b/tools/include/uapi/asm-generic/mman-common.h
index 6ce1f1ceb432..65a3069462a8 100644
--- a/tools/include/uapi/asm-generic/mman-common.h
+++ b/tools/include/uapi/asm-generic/mman-common.h
@@ -33,6 +33,10 @@
#define MAP_UNINITIALIZED 0x4000000 /* For anonymous mmap, memory could be
* uninitialized */
+#define MAP_WIPEONFORK 0x08000000 /* Zero memory in child forks. */
+#define MAP_DONTDUMP 0x10000000 /* Do not write to coredumps. */
+#define MAP_DROPPABLE 0x20000000 /* Zero memory under memory pressure. */
+
/*
* Flags for mlock
*/
diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
index 0b9ab987601c..a8beeb43c2b5 100644
--- a/tools/testing/selftests/mm/.gitignore
+++ b/tools/testing/selftests/mm/.gitignore
@@ -49,3 +49,4 @@ hugetlb_fault_after_madv
hugetlb_madv_vs_map
mseal_test
seal_elf
+droppable
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index 3b49bc3d0a3b..e3e5740e13e1 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -73,6 +73,7 @@ TEST_GEN_FILES += ksm_functional_tests
TEST_GEN_FILES += mdwe_test
TEST_GEN_FILES += hugetlb_fault_after_madv
TEST_GEN_FILES += hugetlb_madv_vs_map
+TEST_GEN_FILES += droppable
ifneq ($(ARCH),arm64)
TEST_GEN_FILES += soft-dirty
diff --git a/tools/testing/selftests/mm/droppable.c b/tools/testing/selftests/mm/droppable.c
new file mode 100644
index 000000000000..846fb9aea4d1
--- /dev/null
+++ b/tools/testing/selftests/mm/droppable.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+#include <assert.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <signal.h>
+#include <sys/mman.h>
+#include <linux/mman.h>
+
+#include "../kselftest.h"
+
+int main(int argc, char *argv[])
+{
+ size_t alloc_size = 134217728;
+ size_t page_size = getpagesize();
+ void *alloc;
+ pid_t child;
+
+ ksft_print_header();
+ ksft_set_plan(1);
+
+ alloc = mmap(0, alloc_size, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE | MAP_DROPPABLE, -1, 0);
+ assert(alloc != MAP_FAILED);
+ memset(alloc, 'A', alloc_size);
+ for (size_t i = 0; i < alloc_size; i += page_size)
+ assert(*(uint8_t *)(alloc + i));
+
+ child = fork();
+ assert(child >= 0);
+ if (!child) {
+ for (;;)
+ memset(malloc(page_size), 'A', page_size);
+ }
+
+ for (bool done = false; !done;) {
+ for (size_t i = 0; i < alloc_size; i += page_size) {
+ if (!*(uint8_t *)(alloc + i)) {
+ done = true;
+ break;
+ }
+ }
+ }
+ kill(child, SIGTERM);
+
+ ksft_test_result_pass("VM_DROPPABLE: PASS\n");
+ exit(KSFT_PASS);
+}
--
2.45.2
^ permalink raw reply related
* [PATCH v21 0/4] implement getrandom() in vDSO
From: Jason A. Donenfeld @ 2024-07-07 0:26 UTC (permalink / raw)
To: linux-kernel, patches, tglx
Cc: Jason A. Donenfeld, linux-crypto, linux-api, x86, Linus Torvalds,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Jann Horn, Christian Brauner,
David Hildenbrand
The plan for this series is to take it through my random.git tree for 6.11.
It's cooking in linux-next now.
Changes v20->v21:
- After extensive conversation with Linus, we're nixing the entire
vgetrandom_alloc() syscall, in favor of just exposing the functionality
needed through mmap() and having the kernel communicate to the caller what
arguments/sizes it should pass to mmap(). This simplifies the series
considerably. It also means that the first commit adds some new MAP_*
constants for mmap().
- Separate vDSO selftests out into separate commit.
--------------
Useful links:
- This series:
- https://git.kernel.org/pub/scm/linux/kernel/git/crng/random.git/log/
- In case you're actually interested in the v≤14 design where faults were
non-fatal and instructions were skipped (which I think is more coherent, even
if the implementation is controversial), this lives in my branch here:
- https://git.kernel.org/pub/scm/linux/kernel/git/crng/random.git/log/?h=jd/vdso-skip-insn
Note that I'm *not* actually proposing this for upstream at this time. But it
may be of conversational interest.
-------------
Two statements:
1) Userspace wants faster cryptographically secure random numbers of
arbitrary size, big or small.
2) Userspace is currently unable to safely roll its own RNG with the
same security profile as getrandom().
Statement (1) has been debated for years, with arguments ranging from
"we need faster cryptographically secure card shuffling!" to "the only
things that actually need good randomness are keys, which are few and
far between" to "actually, TLS CBC nonces are frequent" and so on. I
don't intend to wade into that debate substantially, except to note that
recently glibc added arc4random(), whose goal is to return a
cryptographically secure uint32_t, and there are real user reports of it
being too slow. So here we are.
Statement (2) is more interesting. The kernel is the nexus of all
entropic inputs that influence the RNG. It is in the best position, and
probably the only position, to decide anything at all about the current
state of the RNG and of its entropy. One of the things it uniquely knows
about is when reseeding is necessary.
For example, when a virtual machine is forked, restored, or duplicated,
it's imparative that the RNG doesn't generate the same outputs. For this
reason, there's a small protocol between hypervisors and the kernel that
indicates this has happened, alongside some ID, which the RNG uses to
immediately reseed, so as not to return the same numbers. Were userspace
to expand a getrandom() seed from time T1 for the next hour, and at some
point T2 < hour, the virtual machine forked, userspace would continue to
provide the same numbers to two (or more) different virtual machines,
resulting in potential cryptographic catastrophe. Something similar
happens on resuming from hibernation (or even suspend), with various
compromise scenarios there in mind.
There's a more general reason why userspace rolling its own RNG from a
getrandom() seed is fraught. There's a lot of attention paid to this
particular Linuxism we have of the RNG being initialized and thus
non-blocking or uninitialized and thus blocking until it is initialized.
These are our Two Big States that many hold to be the holy
differentiating factor between safe and not safe, between
cryptographically secure and garbage. The fact is, however, that the
distinction between these two states is a hand-wavy wishy-washy inexact
approximation. Outside of a few exceptional cases (e.g. a HW RNG is
available), we actually don't really ever know with any rigor at all
when the RNG is safe and ready (nor when it's compromised). We do the
best we can to "estimate" it, but entropy estimation is fundamentally
impossible in the general case. So really, we're just doing guess work,
and hoping it's good and conservative enough. Let's then assume that
there's always some potential error involved in this differentiator.
In fact, under the surface, the RNG is engineered around a different
principle, and that is trying to *use* new entropic inputs regularly and
at the right specific moments in time. For example, close to boot time,
the RNG reseeds itself more often than later. At certain events, like VM
fork, the RNG reseeds itself immediately. The various heuristics for
when the RNG will use new entropy and how often is really a core aspect
of what the RNG has some potential to do decently enough (and something
that will probably continue to improve in the future from random.c's
present set of algorithms). So in your mind, put away the metal
attachment to the Two Big States, which represent an approximation with
a potential margin of error. Instead keep in mind that the RNG's primary
operating heuristic is how often and exactly when it's going to reseed.
So, if userspace takes a seed from getrandom() at point T1, and uses it
for the next hour (or N megabytes or some other meaningless metric),
during that time, potential errors in the Two Big States approximation
are amplified. During that time potential reseeds are being lost,
forgotten, not reflected in the output stream. That's not good.
The simplest statement you could make is that userspace RNGs that expand
a getrandom() seed at some point T1 are nearly always *worse*, in some
way, than just calling getrandom() every time a random number is
desired.
For those reasons, after some discussion on libc-alpha, glibc's
arc4random() now just calls getrandom() on each invocation. That's
trivially safe, and gives us latitude to then make the safe thing faster
without becoming unsafe at our leasure. Card shuffling isn't
particularly fast, however.
How do we rectify this? By putting a safe implementation of getrandom()
in the vDSO, which has access to whatever information a
particular iteration of random.c is using to make its decisions. I use
that careful language of "particular iteration of random.c", because the
set of things that a vDSO getrandom() implementation might need for making
decisions as good as the kernel's will likely change over time. This
isn't just a matter of exporting certain *data* to userspace. We're not
going to commit to a "data API" where the various heuristics used are
exposed, locking in how the kernel works for decades to come, and then
leave it to various userspaces to roll something on top and shoot
themselves in the foot and have all sorts of complexity disasters.
Rather, vDSO getrandom() is supposed to be the *same exact algorithm*
that runs in the kernel, except it's been hoisted into userspace as
much as possible. And so vDSO getrandom() and kernel getrandom() will
always mirror each other hermetically.
API-wise, the vDSO gains this function:
ssize_t vgetrandom(void *buffer, size_t len, unsigned int flags,
void *opaque_state, size_t opaque_len);
The return value and the first 3 arguments are the same as ordinary
getrandom(), while the penultimate argument is a pointer to some state
allocated with the right flags passed to mmap(2), explained below. Were all
five arguments passed to the getrandom syscall, nothing different would happen,
and the functions would have the exact same behavior.
If vgetrandom(NULL, 0, 0, ¶ms, ~0UL) is called, then params gets populated
with information about what flags and prot fields to pass to mmap(2), as well
as how big each state should be, so that the caller can slice up returned
memory from mmap(2) into chunks for passing to vgetrandom().
Libc is expected to allocate a chunk of these on first use, and then
dole them out to threads as they're created, allocating more when
needed.
The interesting meat of the implementation is in lib/vdso/getrandom.c,
as generic C code, and it aims to mainly follow random.c's buffered fast
key erasure logic. Before the RNG is initialized, it falls back to the
syscall. Right now it uses a simple generation counter to make its decisions
on reseeding (though this could be made more extensive over time).
The actual place that has the most work to do is in all of the other
files. Most of the vDSO shared page infrastructure is centered around
gettimeofday, and so the main structs are all in arrays for different
timestamp types, and attached to time namespaces, and so forth. I've
done the best I could to add onto this in an unintrusive way.
In my test results, performance is pretty stellar (around 15x for uint32_t
generation), and it seems to be working. There's an extended example in the
last commit of this series, showing how the syscall and the vDSO function
are meant to be used together.
Cc: linux-crypto@vger.kernel.org
Cc: linux-api@vger.kernel.org
Cc: x86@kernel.org
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
Cc: Carlos O'Donell <carlos@redhat.com>
Cc: Florian Weimer <fweimer@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Jann Horn <jannh@google.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: David Hildenbrand <dhildenb@redhat.com>
Jason A. Donenfeld (4):
mm: add VM_DROPPABLE for designating always lazily freeable mappings
random: introduce generic vDSO getrandom() implementation
x86: vdso: Wire up getrandom() vDSO implementation
selftests/vDSO: add tests for vgetrandom
MAINTAINERS | 4 +
arch/alpha/include/uapi/asm/mman.h | 3 +
arch/mips/include/uapi/asm/mman.h | 3 +
arch/parisc/include/uapi/asm/mman.h | 3 +
arch/x86/Kconfig | 1 +
arch/x86/entry/vdso/Makefile | 3 +-
arch/x86/entry/vdso/vdso.lds.S | 2 +
arch/x86/entry/vdso/vgetrandom-chacha.S | 178 +++++++++++
arch/x86/entry/vdso/vgetrandom.c | 17 ++
arch/x86/include/asm/vdso/getrandom.h | 55 ++++
arch/x86/include/asm/vdso/vsyscall.h | 2 +
arch/x86/include/asm/vvar.h | 16 +
arch/xtensa/include/uapi/asm/mman.h | 3 +
drivers/char/random.c | 18 +-
fs/proc/task_mmu.c | 3 +
include/linux/mm.h | 8 +
include/trace/events/mmflags.h | 7 +
include/uapi/asm-generic/mman-common.h | 4 +
include/uapi/linux/random.h | 15 +
include/vdso/datapage.h | 11 +
include/vdso/getrandom.h | 46 +++
lib/vdso/Kconfig | 6 +
lib/vdso/getrandom.c | 252 +++++++++++++++
mm/Kconfig | 3 +
mm/mmap.c | 15 +
mm/mprotect.c | 2 +-
mm/rmap.c | 16 +-
tools/include/asm/rwonce.h | 0
tools/include/uapi/asm-generic/mman-common.h | 4 +
tools/testing/selftests/mm/.gitignore | 1 +
tools/testing/selftests/mm/Makefile | 1 +
tools/testing/selftests/mm/droppable.c | 53 ++++
tools/testing/selftests/vDSO/.gitignore | 2 +
tools/testing/selftests/vDSO/Makefile | 15 +
.../testing/selftests/vDSO/vdso_test_chacha.c | 43 +++
.../selftests/vDSO/vdso_test_getrandom.c | 288 ++++++++++++++++++
36 files changed, 1097 insertions(+), 6 deletions(-)
create mode 100644 arch/x86/entry/vdso/vgetrandom-chacha.S
create mode 100644 arch/x86/entry/vdso/vgetrandom.c
create mode 100644 arch/x86/include/asm/vdso/getrandom.h
create mode 100644 include/vdso/getrandom.h
create mode 100644 lib/vdso/getrandom.c
create mode 100644 tools/include/asm/rwonce.h
create mode 100644 tools/testing/selftests/mm/droppable.c
create mode 100644 tools/testing/selftests/vDSO/vdso_test_chacha.c
create mode 100644 tools/testing/selftests/vDSO/vdso_test_getrandom.c
base-commit: 22a40d14b572deb80c0648557f4bd502d7e83826
--
2.45.2
^ permalink raw reply
* Re: [RFC PATCH v19 2/5] security: Add new SHOULD_EXEC_CHECK and SHOULD_EXEC_RESTRICT securebits
From: Jarkko Sakkinen @ 2024-07-06 17:28 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Kees Cook, Al Viro, Christian Brauner, Linus Torvalds, Paul Moore,
Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20240706.SieHeiMie8fa@digikod.net>
On Sat Jul 6, 2024 at 5:56 PM EEST, Mickaël Salaün wrote:
> On Sat, Jul 06, 2024 at 01:22:06AM +0300, Jarkko Sakkinen wrote:
> > On Sat Jul 6, 2024 at 12:44 AM EEST, Kees Cook wrote:
> > > > As explained in the UAPI comments, all parent processes need to be
> > > > trusted. This meeans that their code is trusted, their seccomp filters
> > > > are trusted, and that they are patched, if needed, to check file
> > > > executability.
> > >
> > > But we have launchers that apply arbitrary seccomp policy, e.g. minijail
> > > on Chrome OS, or even systemd on regular distros. In theory, this should
> > > be handled via other ACLs.
> >
> > Or a regular web browser? AFAIK seccomp filtering was the tool to make
> > secure browser tabs in the first place.
>
> Yes, and that't OK. Web browsers embedded their own seccomp filters and
> they are then as trusted as the browser code.
I'd recommend to slice of tech detail from cover letter, as long as
those details are in the commit messages.
Then, in the cover letter I'd go through maybe two familiar scenarios,
with interactions to this functionality.
A desktop web browser could be perhaps one of them...
BR, Jarkko
^ permalink raw reply
* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Florian Weimer @ 2024-07-06 15:32 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20240706.poo9ahd3La9b@digikod.net>
* Mickaël Salaün:
> On Fri, Jul 05, 2024 at 08:03:14PM +0200, Florian Weimer wrote:
>> * Mickaël Salaün:
>>
>> > Add a new AT_CHECK flag to execveat(2) to check if a file would be
>> > allowed for execution. The main use case is for script interpreters and
>> > dynamic linkers to check execution permission according to the kernel's
>> > security policy. Another use case is to add context to access logs e.g.,
>> > which script (instead of interpreter) accessed a file. As any
>> > executable code, scripts could also use this check [1].
>>
>> Some distributions no longer set executable bits on most shared objects,
>> which I assume would interfere with AT_CHECK probing for shared objects.
>
> A file without the execute permission is not considered as executable by
> the kernel. The AT_CHECK flag doesn't change this semantic. Please
> note that this is just a check, not a restriction. See the next patch
> for the optional policy enforcement.
>
> Anyway, we need to define the policy, and for Linux this is done with
> the file permission bits. So for systems willing to have a consistent
> execution policy, we need to rely on the same bits.
Yes, that makes complete sense. I just wanted to point out the odd
interaction with the old binutils bug and the (sadly still current)
kernel bug.
>> Removing the executable bit is attractive because of a combination of
>> two bugs: a binutils wart which until recently always set the entry
>> point address in the ELF header to zero, and the kernel not checking for
>> a zero entry point (maybe in combination with an absent program
>> interpreter) and failing the execve with ELIBEXEC, instead of doing the
>> execve and then faulting at virtual address zero. Removing the
>> executable bit is currently the only way to avoid these confusing
>> crashes, so I understand the temptation.
>
> Interesting. Can you please point to the bug report and the fix? I
> don't see any ELIBEXEC in the kernel.
The kernel hasn't been fixed yet. I do think this should be fixed, so
that distributions can bring back the executable bit.
Thanks,
Florian
^ permalink raw reply
* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Mickaël Salaün @ 2024-07-06 14:55 UTC (permalink / raw)
To: Florian Weimer
Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <87bk3bvhr1.fsf@oldenburg.str.redhat.com>
On Fri, Jul 05, 2024 at 08:03:14PM +0200, Florian Weimer wrote:
> * Mickaël Salaün:
>
> > Add a new AT_CHECK flag to execveat(2) to check if a file would be
> > allowed for execution. The main use case is for script interpreters and
> > dynamic linkers to check execution permission according to the kernel's
> > security policy. Another use case is to add context to access logs e.g.,
> > which script (instead of interpreter) accessed a file. As any
> > executable code, scripts could also use this check [1].
>
> Some distributions no longer set executable bits on most shared objects,
> which I assume would interfere with AT_CHECK probing for shared objects.
A file without the execute permission is not considered as executable by
the kernel. The AT_CHECK flag doesn't change this semantic. Please
note that this is just a check, not a restriction. See the next patch
for the optional policy enforcement.
Anyway, we need to define the policy, and for Linux this is done with
the file permission bits. So for systems willing to have a consistent
execution policy, we need to rely on the same bits.
> Removing the executable bit is attractive because of a combination of
> two bugs: a binutils wart which until recently always set the entry
> point address in the ELF header to zero, and the kernel not checking for
> a zero entry point (maybe in combination with an absent program
> interpreter) and failing the execve with ELIBEXEC, instead of doing the
> execve and then faulting at virtual address zero. Removing the
> executable bit is currently the only way to avoid these confusing
> crashes, so I understand the temptation.
Interesting. Can you please point to the bug report and the fix? I
don't see any ELIBEXEC in the kernel.
FYI, AT_CHECK doesn't check the content of the file (unlike a full
execve call).
Anyway, I think we should not design a new kernel interface to work
around a current user space bug.
^ permalink raw reply
* Re: [RFC PATCH v19 2/5] security: Add new SHOULD_EXEC_CHECK and SHOULD_EXEC_RESTRICT securebits
From: Mickaël Salaün @ 2024-07-06 14:56 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Kees Cook, Al Viro, Christian Brauner, Linus Torvalds, Paul Moore,
Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <D2HYFLLXVYLS.ORASE7L62L3N@kernel.org>
On Sat, Jul 06, 2024 at 01:22:06AM +0300, Jarkko Sakkinen wrote:
> On Sat Jul 6, 2024 at 12:44 AM EEST, Kees Cook wrote:
> > > As explained in the UAPI comments, all parent processes need to be
> > > trusted. This meeans that their code is trusted, their seccomp filters
> > > are trusted, and that they are patched, if needed, to check file
> > > executability.
> >
> > But we have launchers that apply arbitrary seccomp policy, e.g. minijail
> > on Chrome OS, or even systemd on regular distros. In theory, this should
> > be handled via other ACLs.
>
> Or a regular web browser? AFAIK seccomp filtering was the tool to make
> secure browser tabs in the first place.
Yes, and that't OK. Web browsers embedded their own seccomp filters and
they are then as trusted as the browser code.
^ permalink raw reply
* Re: [RFC PATCH v19 2/5] security: Add new SHOULD_EXEC_CHECK and SHOULD_EXEC_RESTRICT securebits
From: Mickaël Salaün @ 2024-07-06 14:56 UTC (permalink / raw)
To: Kees Cook
Cc: Al Viro, Christian Brauner, Linus Torvalds, Paul Moore,
Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <202407051425.32AF9D2@keescook>
On Fri, Jul 05, 2024 at 02:44:03PM -0700, Kees Cook wrote:
> On Fri, Jul 05, 2024 at 07:54:16PM +0200, Mickaël Salaün wrote:
> > On Thu, Jul 04, 2024 at 05:18:04PM -0700, Kees Cook wrote:
> > > On Thu, Jul 04, 2024 at 09:01:34PM +0200, Mickaël Salaün wrote:
> > > > Such a secure environment can be achieved with an appropriate access
> > > > control policy (e.g. mount's noexec option, file access rights, LSM
> > > > configuration) and an enlighten ld.so checking that libraries are
> > > > allowed for execution e.g., to protect against illegitimate use of
> > > > LD_PRELOAD.
> > > >
> > > > Scripts may need some changes to deal with untrusted data (e.g. stdin,
> > > > environment variables), but that is outside the scope of the kernel.
> > >
> > > If the threat model includes an attacker sitting at a shell prompt, we
> > > need to be very careful about how process perform enforcement. E.g. even
> > > on a locked down system, if an attacker has access to LD_PRELOAD or a
> >
> > LD_PRELOAD should be OK once ld.so will be patched to check the
> > libraries. We can still imagine a debug library used to bypass security
> > checks, but in this case the issue would be that this library is
> > executable in the first place.
>
> Ah yes, that's fair: the shell would discover the malicious library
> while using AT_CHECK during resolution of the LD_PRELOAD.
That's the idea, but it would be checked by ld.so, not the shell.
>
> > > seccomp wrapper (which you both mention here), it would be possible to
> > > run commands where the resulting process is tricked into thinking it
> > > doesn't have the bits set.
> >
> > As explained in the UAPI comments, all parent processes need to be
> > trusted. This meeans that their code is trusted, their seccomp filters
> > are trusted, and that they are patched, if needed, to check file
> > executability.
>
> But we have launchers that apply arbitrary seccomp policy, e.g. minijail
> on Chrome OS, or even systemd on regular distros. In theory, this should
> be handled via other ACLs.
Processes running with untrusted seccomp filter should be considered
untrusted. It would then make sense for these seccomp filters/programs
to be considered executable code, and then for minijail and systemd to
check them with AT_CHECK (according to the securebits policy).
>
> > > But this would be exactly true for calling execveat(): LD_PRELOAD or
> > > seccomp policy could have it just return 0.
> >
> > If an attacker is allowed/able to load an arbitrary seccomp filter on a
> > process, we cannot trust this process.
> >
> > >
> > > While I like AT_CHECK, I do wonder if it's better to do the checks via
> > > open(), as was originally designed with O_MAYEXEC. Because then
> > > enforcement is gated by the kernel -- the process does not get a file
> > > descriptor _at all_, no matter what LD_PRELOAD or seccomp tricks it into
> > > doing.
> >
> > Being able to check a path name or a file descriptor (with the same
> > syscall) is more flexible and cover more use cases.
>
> If flexibility costs us reliability, I think that flexibility is not
> a benefit.
Well, it's a matter of letting user space do what they think is best,
and I think there are legitimate and safe uses of path names, even if I
agree that this should not be used in most use cases. Would we want
faccessat2(2) to only take file descriptor as argument and not file
path? I don't think so but I'd defer to the VFS maintainers.
Christian, Al, Linus?
Steve, could you share a use case with file paths?
>
> > The execveat(2)
> > interface, including current and future flags, is dedicated to file
> > execution. I then think that using execveat(2) for this kind of check
> > makes more sense, and will easily evolve with this syscall.
>
> Yeah, I do recognize that is feels much more natural, but I remain
> unhappy about how difficult it will become to audit a system for safety
> when the check is strictly per-process opt-in, and not enforced by the
> kernel for a given process tree. But, I think this may have always been
> a fiction in my mind. :)
Hmm, I'm not sure to follow. Securebits are inherited, so process tree.
And we need the parent processes to be trusted anyway.
>
> > > And this thinking also applies to faccessat() too: if a process can be
> > > tricked into thinking the access check passed, it'll happily interpret
> > > whatever. :( But not being able to open the fd _at all_ when O_MAYEXEC
> > > is being checked seems substantially safer to me...
> >
> > If attackers can filter execveat(2), they can also filter open(2) and
> > any other syscalls. In all cases, that would mean an issue in the
> > security policy.
>
> Hm, as in, make a separate call to open(2) without O_MAYEXEC, and pass
> that fd back to the filtered open(2) that did have O_MAYEXEC. Yes, true.
>
> I guess it does become morally equivalent.
>
> Okay. Well, let me ask about usability. Right now, a process will need
> to do:
>
> - should I use AT_CHECK? (check secbit)
> - if yes: perform execveat(AT_CHECK)
>
> Why not leave the secbit test up to the kernel, and then the program can
> just unconditionally call execveat(AT_CHECK)?
That was kind of the approach of the previous patch series and Linus
wanted the new interface to follow the kernel semantic. Enforcing this
kind of restriction will always be the duty of user space anyway, so I
think it's simpler (i.e. no mix of policy definition, access check, and
policy enforcement, but a standalone execveat feature), more flexible,
and it fully delegates the policy enforcement to user space instead of
trying to enforce some part in the kernel which would only give the
illusion of security/policy enforcement.
>
> Though perhaps the issue here is that an execveat() EINVAL doesn't
> tell the program if AT_CHECK is unimplemented or if something else
> went wrong, and the secbit prctl() will give the correct signal about
> AT_CHECK availability?
This kind of check could indeed help to identify the issue.
^ permalink raw reply
* Re: [PATCH v3] platform/x86: asus-wmi: support the disable camera LED on F10 of Zenbook 2023
From: Ilpo Järvinen @ 2024-07-06 13:44 UTC (permalink / raw)
To: Devin Bayer
Cc: corentin.chary, luke, Hans de Goede, platform-driver-x86, LKML,
linux-api
In-Reply-To: <20240628084603.217106-1-dev@doubly.so>
On Fri, 28 Jun 2024, Devin Bayer wrote:
> Adds a sysfs entry for the LED on F10 above the crossed out camera icon on 2023 Zenbooks.
Thanks for the update, I've applied this into review-ilpo branch with
small (non-code) edits.
In future, please fold the changelog to 72 characters and include patch
history only after the first --- line so tools don't put it into the
commit message. I've handled these for you this time around so there's no
need to send another version because of those problems.
--
i.
> v3
> - add docs for WMI devices
> - remove duplicate #define
>
> v2
> - Changed name from `platform::camera` to `asus::camera`
> - Separated patch from patchset
>
> v1
> - https://lore.kernel.org/platform-driver-x86/20240620082223.20178-1-dev@doubly.so/
>
> Signed-off-by: Devin Bayer <dev@doubly.so>
> ---
> drivers/platform/x86/asus-wmi.c | 35 ++++++++++++++++++++++
> include/linux/platform_data/x86/asus-wmi.h | 4 +++
> 2 files changed, 39 insertions(+)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 3f07bbf809ef..4a9ad8b313e6 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -227,6 +227,7 @@ struct asus_wmi {
> struct led_classdev lightbar_led;
> int lightbar_led_wk;
> struct led_classdev micmute_led;
> + struct led_classdev camera_led;
> struct workqueue_struct *led_workqueue;
> struct work_struct tpd_led_work;
> struct work_struct wlan_led_work;
> @@ -1533,6 +1534,27 @@ static int micmute_led_set(struct led_classdev *led_cdev,
> return err < 0 ? err : 0;
> }
>
> +static enum led_brightness camera_led_get(struct led_classdev *led_cdev)
> +{
> + struct asus_wmi *asus;
> + u32 result;
> +
> + asus = container_of(led_cdev, struct asus_wmi, camera_led);
> + asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_CAMERA_LED, &result);
> +
> + return result & ASUS_WMI_DSTS_BRIGHTNESS_MASK;
> +}
> +
> +static int camera_led_set(struct led_classdev *led_cdev,
> + enum led_brightness brightness)
> +{
> + int state = brightness != LED_OFF;
> + int err;
> +
> + err = asus_wmi_set_devstate(ASUS_WMI_DEVID_CAMERA_LED, state, NULL);
> + return err < 0 ? err : 0;
> +}
> +
> static void asus_wmi_led_exit(struct asus_wmi *asus)
> {
> led_classdev_unregister(&asus->kbd_led);
> @@ -1540,6 +1562,7 @@ static void asus_wmi_led_exit(struct asus_wmi *asus)
> led_classdev_unregister(&asus->wlan_led);
> led_classdev_unregister(&asus->lightbar_led);
> led_classdev_unregister(&asus->micmute_led);
> + led_classdev_unregister(&asus->camera_led);
>
> if (asus->led_workqueue)
> destroy_workqueue(asus->led_workqueue);
> @@ -1631,6 +1654,18 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
> goto error;
> }
>
> + if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_CAMERA_LED)) {
> + asus->camera_led.name = "asus::camera";
> + asus->camera_led.max_brightness = 1;
> + asus->camera_led.brightness_get = camera_led_get;
> + asus->camera_led.brightness_set_blocking = camera_led_set;
> +
> + rv = led_classdev_register(&asus->platform_device->dev,
> + &asus->camera_led);
> + if (rv)
> + goto error;
> + }
> +
> error:
> if (rv)
> asus_wmi_led_exit(asus);
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index ab1c7deff118..d020fcbbcfb7 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -51,6 +51,10 @@
> #define ASUS_WMI_DEVID_LED6 0x00020016
> #define ASUS_WMI_DEVID_MICMUTE_LED 0x00040017
>
> +/* Disable Camera LED */
> +#define ASUS_WMI_DEVID_CAMERA_LED_NEG 0x00060078 /* 0 = on (unused) */
> +#define ASUS_WMI_DEVID_CAMERA_LED 0x00060079 /* 1 = on */
> +
> /* Backlight and Brightness */
> #define ASUS_WMI_DEVID_ALS_ENABLE 0x00050001 /* Ambient Light Sensor */
> #define ASUS_WMI_DEVID_BACKLIGHT 0x00050011
>
> base-commit: 103a4e4a4351d3d5214c4f54fdf89f0f81b692ef
>
^ permalink raw reply
* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Andy Lutomirski @ 2024-07-06 8:52 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20240704190137.696169-2-mic@digikod.net>
On Fri, Jul 5, 2024 at 3:03 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> Add a new AT_CHECK flag to execveat(2) to check if a file would be
> allowed for execution. The main use case is for script interpreters and
> dynamic linkers to check execution permission according to the kernel's
> security policy. Another use case is to add context to access logs e.g.,
> which script (instead of interpreter) accessed a file. As any
> executable code, scripts could also use this check [1].
>
Can you give a worked-out example of how this is useful?
I assume the idea is that a program could open a file, then pass the
fd to execveat() to get the kernel's idea of whether it's permissible
to execute it. And then the program would interpret the file, which
is morally like executing it. And there would be a big warning in the
manpage that passing a *path* is subject to a TOCTOU race.
This type of usage will do the wrong thing if LSM policy intends to
lock down the task if the task were to actually exec the file. I
personally think this is a mis-design (let the program doing the
exec-ing lock itself down, possibly by querying a policy, but having
magic happen on exec seems likely to do the wrong thing more often
that it does the wright thing), but that ship sailed a long time ago.
So maybe what's actually needed is a rather different API: a way to
check *and perform* the security transition for an exec without
actually execing. This would need to be done NO_NEW_PRIVS style for
reasons that are hopefully obvious, but it would permit:
fd = open(some script);
if (do_exec_transition_without_exec(fd) != 0)
return; // don't actually do it
// OK, we may have just lost privileges. But that's okay, because we
meant to do that.
// Make sure we've munmapped anything sensitive and erased any secrets
from memory,
// and then interpret the script!
I think this would actually be straightforward to implement in the
kernel -- one would need to make sure that all the relevant
no_new_privs checks are looking in the right place (as the task might
not actually have no_new_privs set, but LSM_UNSAFE_NO_NEW_PRIVS would
still be set), but I don't see any reason this would be
insurmountable, nor do I expect there would be any fundamental
problems.
> This is different than faccessat(2) which only checks file access
> rights, but not the full context e.g. mount point's noexec, stack limit,
> and all potential LSM extra checks (e.g. argv, envp, credentials).
> Since the use of AT_CHECK follows the exact kernel semantic as for a
> real execution, user space gets the same error codes.
>
> With the information that a script interpreter is about to interpret a
> script, an LSM security policy can adjust caller's access rights or log
> execution request as for native script execution (e.g. role transition).
> This is possible thanks to the call to security_bprm_creds_for_exec().
>
> Because LSMs may only change bprm's credentials, use of AT_CHECK with
> current kernel code should not be a security issue (e.g. unexpected role
> transition). LSMs willing to update the caller's credential could now
> do so when bprm->is_check is set. Of course, such policy change should
> be in line with the new user space code.
>
> Because AT_CHECK is dedicated to user space interpreters, it doesn't
> make sense for the kernel to parse the checked files, look for
> interpreters known to the kernel (e.g. ELF, shebang), and return ENOEXEC
> if the format is unknown. Because of that, security_bprm_check() is
> never called when AT_CHECK is used.
>
> It should be noted that script interpreters cannot directly use
> execveat(2) (without this new AT_CHECK flag) because this could lead to
> unexpected behaviors e.g., `python script.sh` could lead to Bash being
> executed to interpret the script. Unlike the kernel, script
> interpreters may just interpret the shebang as a simple comment, which
> should not change for backward compatibility reasons.
>
> Because scripts or libraries files might not currently have the
> executable permission set, or because we might want specific users to be
> allowed to run arbitrary scripts, the following patch provides a dynamic
> configuration mechanism with the SECBIT_SHOULD_EXEC_CHECK and
> SECBIT_SHOULD_EXEC_RESTRICT securebits.
Can you explain what those bits do? And why they're useful?
>
> This is a redesign of the CLIP OS 4's O_MAYEXEC:
> https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
> This patch has been used for more than a decade with customized script
> interpreters. Some examples can be found here:
> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
This one at least returns an fd, so it looks less likely to get
misused in a way that adds a TOCTOU race.
^ permalink raw reply
* Re: [RFC PATCH v19 2/5] security: Add new SHOULD_EXEC_CHECK and SHOULD_EXEC_RESTRICT securebits
From: Jarkko Sakkinen @ 2024-07-05 22:22 UTC (permalink / raw)
To: Kees Cook, Mickaël Salaün
Cc: Al Viro, Christian Brauner, Linus Torvalds, Paul Moore,
Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <202407051425.32AF9D2@keescook>
On Sat Jul 6, 2024 at 12:44 AM EEST, Kees Cook wrote:
> > As explained in the UAPI comments, all parent processes need to be
> > trusted. This meeans that their code is trusted, their seccomp filters
> > are trusted, and that they are patched, if needed, to check file
> > executability.
>
> But we have launchers that apply arbitrary seccomp policy, e.g. minijail
> on Chrome OS, or even systemd on regular distros. In theory, this should
> be handled via other ACLs.
Or a regular web browser? AFAIK seccomp filtering was the tool to make
secure browser tabs in the first place.
BR, Jarkko
^ permalink raw reply
* Re: [RFC PATCH v19 2/5] security: Add new SHOULD_EXEC_CHECK and SHOULD_EXEC_RESTRICT securebits
From: Kees Cook @ 2024-07-05 21:44 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Al Viro, Christian Brauner, Linus Torvalds, Paul Moore,
Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20240705.IeTheequ7Ooj@digikod.net>
On Fri, Jul 05, 2024 at 07:54:16PM +0200, Mickaël Salaün wrote:
> On Thu, Jul 04, 2024 at 05:18:04PM -0700, Kees Cook wrote:
> > On Thu, Jul 04, 2024 at 09:01:34PM +0200, Mickaël Salaün wrote:
> > > Such a secure environment can be achieved with an appropriate access
> > > control policy (e.g. mount's noexec option, file access rights, LSM
> > > configuration) and an enlighten ld.so checking that libraries are
> > > allowed for execution e.g., to protect against illegitimate use of
> > > LD_PRELOAD.
> > >
> > > Scripts may need some changes to deal with untrusted data (e.g. stdin,
> > > environment variables), but that is outside the scope of the kernel.
> >
> > If the threat model includes an attacker sitting at a shell prompt, we
> > need to be very careful about how process perform enforcement. E.g. even
> > on a locked down system, if an attacker has access to LD_PRELOAD or a
>
> LD_PRELOAD should be OK once ld.so will be patched to check the
> libraries. We can still imagine a debug library used to bypass security
> checks, but in this case the issue would be that this library is
> executable in the first place.
Ah yes, that's fair: the shell would discover the malicious library
while using AT_CHECK during resolution of the LD_PRELOAD.
> > seccomp wrapper (which you both mention here), it would be possible to
> > run commands where the resulting process is tricked into thinking it
> > doesn't have the bits set.
>
> As explained in the UAPI comments, all parent processes need to be
> trusted. This meeans that their code is trusted, their seccomp filters
> are trusted, and that they are patched, if needed, to check file
> executability.
But we have launchers that apply arbitrary seccomp policy, e.g. minijail
on Chrome OS, or even systemd on regular distros. In theory, this should
be handled via other ACLs.
> > But this would be exactly true for calling execveat(): LD_PRELOAD or
> > seccomp policy could have it just return 0.
>
> If an attacker is allowed/able to load an arbitrary seccomp filter on a
> process, we cannot trust this process.
>
> >
> > While I like AT_CHECK, I do wonder if it's better to do the checks via
> > open(), as was originally designed with O_MAYEXEC. Because then
> > enforcement is gated by the kernel -- the process does not get a file
> > descriptor _at all_, no matter what LD_PRELOAD or seccomp tricks it into
> > doing.
>
> Being able to check a path name or a file descriptor (with the same
> syscall) is more flexible and cover more use cases.
If flexibility costs us reliability, I think that flexibility is not
a benefit.
> The execveat(2)
> interface, including current and future flags, is dedicated to file
> execution. I then think that using execveat(2) for this kind of check
> makes more sense, and will easily evolve with this syscall.
Yeah, I do recognize that is feels much more natural, but I remain
unhappy about how difficult it will become to audit a system for safety
when the check is strictly per-process opt-in, and not enforced by the
kernel for a given process tree. But, I think this may have always been
a fiction in my mind. :)
> > And this thinking also applies to faccessat() too: if a process can be
> > tricked into thinking the access check passed, it'll happily interpret
> > whatever. :( But not being able to open the fd _at all_ when O_MAYEXEC
> > is being checked seems substantially safer to me...
>
> If attackers can filter execveat(2), they can also filter open(2) and
> any other syscalls. In all cases, that would mean an issue in the
> security policy.
Hm, as in, make a separate call to open(2) without O_MAYEXEC, and pass
that fd back to the filtered open(2) that did have O_MAYEXEC. Yes, true.
I guess it does become morally equivalent.
Okay. Well, let me ask about usability. Right now, a process will need
to do:
- should I use AT_CHECK? (check secbit)
- if yes: perform execveat(AT_CHECK)
Why not leave the secbit test up to the kernel, and then the program can
just unconditionally call execveat(AT_CHECK)?
Though perhaps the issue here is that an execveat() EINVAL doesn't
tell the program if AT_CHECK is unimplemented or if something else
went wrong, and the secbit prctl() will give the correct signal about
AT_CHECK availability?
--
Kees Cook
^ permalink raw reply
* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Florian Weimer @ 2024-07-05 18:03 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20240704190137.696169-2-mic@digikod.net>
* Mickaël Salaün:
> Add a new AT_CHECK flag to execveat(2) to check if a file would be
> allowed for execution. The main use case is for script interpreters and
> dynamic linkers to check execution permission according to the kernel's
> security policy. Another use case is to add context to access logs e.g.,
> which script (instead of interpreter) accessed a file. As any
> executable code, scripts could also use this check [1].
Some distributions no longer set executable bits on most shared objects,
which I assume would interfere with AT_CHECK probing for shared objects.
Removing the executable bit is attractive because of a combination of
two bugs: a binutils wart which until recently always set the entry
point address in the ELF header to zero, and the kernel not checking for
a zero entry point (maybe in combination with an absent program
interpreter) and failing the execve with ELIBEXEC, instead of doing the
execve and then faulting at virtual address zero. Removing the
executable bit is currently the only way to avoid these confusing
crashes, so I understand the temptation.
Thanks,
Florian
^ permalink raw reply
* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Mickaël Salaün @ 2024-07-05 17:53 UTC (permalink / raw)
To: Kees Cook
Cc: Al Viro, Christian Brauner, Linus Torvalds, Paul Moore,
Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <202407041656.3A05153@keescook>
On Thu, Jul 04, 2024 at 05:04:03PM -0700, Kees Cook wrote:
> On Thu, Jul 04, 2024 at 09:01:33PM +0200, Mickaël Salaün wrote:
> > Add a new AT_CHECK flag to execveat(2) to check if a file would be
> > allowed for execution. The main use case is for script interpreters and
> > dynamic linkers to check execution permission according to the kernel's
> > security policy. Another use case is to add context to access logs e.g.,
> > which script (instead of interpreter) accessed a file. As any
> > executable code, scripts could also use this check [1].
> >
> > This is different than faccessat(2) which only checks file access
> > rights, but not the full context e.g. mount point's noexec, stack limit,
> > and all potential LSM extra checks (e.g. argv, envp, credentials).
> > Since the use of AT_CHECK follows the exact kernel semantic as for a
> > real execution, user space gets the same error codes.
>
> Nice! I much prefer this method of going through the exec machinery so
> we always have a single code path for these kinds of checks.
>
> > Because AT_CHECK is dedicated to user space interpreters, it doesn't
> > make sense for the kernel to parse the checked files, look for
> > interpreters known to the kernel (e.g. ELF, shebang), and return ENOEXEC
> > if the format is unknown. Because of that, security_bprm_check() is
> > never called when AT_CHECK is used.
>
> I'd like some additional comments in the code that reminds us that
> access control checks have finished past a certain point.
Where in the code? Just before the bprm->is_check assignment?
>
> [...]
> > diff --git a/fs/exec.c b/fs/exec.c
> > index 40073142288f..ea2a1867afdc 100644
> > --- a/fs/exec.c
> > +++ b/fs/exec.c
> > @@ -931,7 +931,7 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags)
> > .lookup_flags = LOOKUP_FOLLOW,
> > };
> >
> > - if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> > + if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH | AT_CHECK)) != 0)
> > return ERR_PTR(-EINVAL);
> > if (flags & AT_SYMLINK_NOFOLLOW)
> > open_exec_flags.lookup_flags &= ~LOOKUP_FOLLOW;
> [...]
> > + * To avoid race conditions leading to time-of-check to time-of-use issues,
> > + * AT_CHECK should be used with AT_EMPTY_PATH to check against a file
> > + * descriptor instead of a path.
>
> I want this enforced by the kernel. Let's not leave trivial ToCToU
> foot-guns around. i.e.:
>
> if ((flags & AT_CHECK) == AT_CHECK && (flags & AT_EMPTY_PATH) == 0)
> return ERR_PTR(-EBADF);
There are valid use cases relying on pathnames. See Linus's comment:
https://lore.kernel.org/r/CAHk-=whb=XuU=LGKnJWaa7LOYQz9VwHs8SLfgLbT5sf2VAbX1A@mail.gmail.com
^ permalink raw reply
* Re: [RFC PATCH v19 2/5] security: Add new SHOULD_EXEC_CHECK and SHOULD_EXEC_RESTRICT securebits
From: Mickaël Salaün @ 2024-07-05 17:54 UTC (permalink / raw)
To: Kees Cook
Cc: Al Viro, Christian Brauner, Linus Torvalds, Paul Moore,
Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <202407041711.B7CD16B2@keescook>
On Thu, Jul 04, 2024 at 05:18:04PM -0700, Kees Cook wrote:
> On Thu, Jul 04, 2024 at 09:01:34PM +0200, Mickaël Salaün wrote:
> > Such a secure environment can be achieved with an appropriate access
> > control policy (e.g. mount's noexec option, file access rights, LSM
> > configuration) and an enlighten ld.so checking that libraries are
> > allowed for execution e.g., to protect against illegitimate use of
> > LD_PRELOAD.
> >
> > Scripts may need some changes to deal with untrusted data (e.g. stdin,
> > environment variables), but that is outside the scope of the kernel.
>
> If the threat model includes an attacker sitting at a shell prompt, we
> need to be very careful about how process perform enforcement. E.g. even
> on a locked down system, if an attacker has access to LD_PRELOAD or a
LD_PRELOAD should be OK once ld.so will be patched to check the
libraries. We can still imagine a debug library used to bypass security
checks, but in this case the issue would be that this library is
executable in the first place.
> seccomp wrapper (which you both mention here), it would be possible to
> run commands where the resulting process is tricked into thinking it
> doesn't have the bits set.
As explained in the UAPI comments, all parent processes need to be
trusted. This meeans that their code is trusted, their seccomp filters
are trusted, and that they are patched, if needed, to check file
executability.
>
> But this would be exactly true for calling execveat(): LD_PRELOAD or
> seccomp policy could have it just return 0.
If an attacker is allowed/able to load an arbitrary seccomp filter on a
process, we cannot trust this process.
>
> While I like AT_CHECK, I do wonder if it's better to do the checks via
> open(), as was originally designed with O_MAYEXEC. Because then
> enforcement is gated by the kernel -- the process does not get a file
> descriptor _at all_, no matter what LD_PRELOAD or seccomp tricks it into
> doing.
Being able to check a path name or a file descriptor (with the same
syscall) is more flexible and cover more use cases. The execveat(2)
interface, including current and future flags, is dedicated to file
execution. I then think that using execveat(2) for this kind of check
makes more sense, and will easily evolve with this syscall.
>
> And this thinking also applies to faccessat() too: if a process can be
> tricked into thinking the access check passed, it'll happily interpret
> whatever. :( But not being able to open the fd _at all_ when O_MAYEXEC
> is being checked seems substantially safer to me...
If attackers can filter execveat(2), they can also filter open(2) and
any other syscalls. In all cases, that would mean an issue in the
security policy.
^ permalink raw reply
* Re: [RFC PATCH v19 2/5] security: Add new SHOULD_EXEC_CHECK and SHOULD_EXEC_RESTRICT securebits
From: Kees Cook @ 2024-07-05 0:18 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Al Viro, Christian Brauner, Linus Torvalds, Paul Moore,
Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20240704190137.696169-3-mic@digikod.net>
On Thu, Jul 04, 2024 at 09:01:34PM +0200, Mickaël Salaün wrote:
> Such a secure environment can be achieved with an appropriate access
> control policy (e.g. mount's noexec option, file access rights, LSM
> configuration) and an enlighten ld.so checking that libraries are
> allowed for execution e.g., to protect against illegitimate use of
> LD_PRELOAD.
>
> Scripts may need some changes to deal with untrusted data (e.g. stdin,
> environment variables), but that is outside the scope of the kernel.
If the threat model includes an attacker sitting at a shell prompt, we
need to be very careful about how process perform enforcement. E.g. even
on a locked down system, if an attacker has access to LD_PRELOAD or a
seccomp wrapper (which you both mention here), it would be possible to
run commands where the resulting process is tricked into thinking it
doesn't have the bits set.
But this would be exactly true for calling execveat(): LD_PRELOAD or
seccomp policy could have it just return 0.
While I like AT_CHECK, I do wonder if it's better to do the checks via
open(), as was originally designed with O_MAYEXEC. Because then
enforcement is gated by the kernel -- the process does not get a file
descriptor _at all_, no matter what LD_PRELOAD or seccomp tricks it into
doing.
And this thinking also applies to faccessat() too: if a process can be
tricked into thinking the access check passed, it'll happily interpret
whatever. :( But not being able to open the fd _at all_ when O_MAYEXEC
is being checked seems substantially safer to me...
--
Kees Cook
^ permalink raw reply
* Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Kees Cook @ 2024-07-05 0:04 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Al Viro, Christian Brauner, Linus Torvalds, Paul Moore,
Theodore Ts'o, Alejandro Colomar, Aleksa Sarai, Andrew Morton,
Andy Lutomirski, Arnd Bergmann, Casey Schaufler, Christian Heimes,
Dmitry Vyukov, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Luca Boccassi, Luis Chamberlain, Madhavan T . Venkataraman,
Matt Bobrowski, Matthew Garrett, Matthew Wilcox, Miklos Szeredi,
Mimi Zohar, Nicolas Bouchinet, Scott Shell, Shuah Khan,
Stephen Rothwell, Steve Dower, Steve Grubb, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20240704190137.696169-2-mic@digikod.net>
On Thu, Jul 04, 2024 at 09:01:33PM +0200, Mickaël Salaün wrote:
> Add a new AT_CHECK flag to execveat(2) to check if a file would be
> allowed for execution. The main use case is for script interpreters and
> dynamic linkers to check execution permission according to the kernel's
> security policy. Another use case is to add context to access logs e.g.,
> which script (instead of interpreter) accessed a file. As any
> executable code, scripts could also use this check [1].
>
> This is different than faccessat(2) which only checks file access
> rights, but not the full context e.g. mount point's noexec, stack limit,
> and all potential LSM extra checks (e.g. argv, envp, credentials).
> Since the use of AT_CHECK follows the exact kernel semantic as for a
> real execution, user space gets the same error codes.
Nice! I much prefer this method of going through the exec machinery so
we always have a single code path for these kinds of checks.
> Because AT_CHECK is dedicated to user space interpreters, it doesn't
> make sense for the kernel to parse the checked files, look for
> interpreters known to the kernel (e.g. ELF, shebang), and return ENOEXEC
> if the format is unknown. Because of that, security_bprm_check() is
> never called when AT_CHECK is used.
I'd like some additional comments in the code that reminds us that
access control checks have finished past a certain point.
[...]
> diff --git a/fs/exec.c b/fs/exec.c
> index 40073142288f..ea2a1867afdc 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -931,7 +931,7 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags)
> .lookup_flags = LOOKUP_FOLLOW,
> };
>
> - if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> + if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH | AT_CHECK)) != 0)
> return ERR_PTR(-EINVAL);
> if (flags & AT_SYMLINK_NOFOLLOW)
> open_exec_flags.lookup_flags &= ~LOOKUP_FOLLOW;
[...]
> + * To avoid race conditions leading to time-of-check to time-of-use issues,
> + * AT_CHECK should be used with AT_EMPTY_PATH to check against a file
> + * descriptor instead of a path.
I want this enforced by the kernel. Let's not leave trivial ToCToU
foot-guns around. i.e.:
if ((flags & AT_CHECK) == AT_CHECK && (flags & AT_EMPTY_PATH) == 0)
return ERR_PTR(-EBADF);
--
Kees Cook
^ permalink raw reply
* [RFC PATCH v19 3/5] selftests/exec: Add tests for AT_CHECK and related securebits
From: Mickaël Salaün @ 2024-07-04 19:01 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
Theodore Ts'o
Cc: Mickaël Salaün, Alejandro Colomar, Aleksa Sarai,
Andrew Morton, Andy Lutomirski, Arnd Bergmann, Casey Schaufler,
Christian Heimes, Dmitry Vyukov, Eric Biggers, Eric Chiang,
Fan Wu, Florian Weimer, Geert Uytterhoeven, James Morris,
Jan Kara, Jann Horn, Jeff Xu, Jonathan Corbet, Jordan R Abrahams,
Lakshmi Ramasubramanian, Luca Boccassi, Luis Chamberlain,
Madhavan T . Venkataraman, Matt Bobrowski, Matthew Garrett,
Matthew Wilcox, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Scott Shell, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Thibaut Sautereau, Vincent Strubel, Xiaoming Ni,
Yin Fengwei, kernel-hardening, linux-api, linux-fsdevel,
linux-integrity, linux-kernel, linux-security-module
In-Reply-To: <20240704190137.696169-1-mic@digikod.net>
Test that checks performed by execveat(..., AT_CHECK) are consistent
with noexec mount points and file execute permissions.
Test that SECBIT_SHOULD_EXEC_CHECK and SECBIT_SHOULD_EXEC_RESTRICT are
inherited by child processes and that they can be pinned with the
appropriate SECBIT_SHOULD_EXEC_CHECK_LOCKED and
SECBIT_SHOULD_EXEC_RESTRICT_LOCKED bits.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20240704190137.696169-4-mic@digikod.net
---
Changes since v18:
* Rewrite tests with the new design: execveat/AT_CHECK and securebits.
* Simplify the capability dropping and improve it with the NOROOT
securebits.
* Replace most ASSERT with EXPECT.
* Fix NULL execve's argv to avoid kernel warning.
* Move tests to exec/
* Build a "false" static binary to test full execution path.
Changes since v14:
* Add Reviewed-by Kees Cook.
Changes since v13:
* Move -I to CFLAGS (suggested by Kees Cook).
* Update sysctl name.
Changes since v12:
* Fix Makefile's license.
Changes since v10:
* Update selftest Makefile.
Changes since v9:
* Rename the syscall and the sysctl.
* Update tests for enum trusted_for_usage
Changes since v8:
* Update with the dedicated syscall introspect_access(2) and the renamed
fs.introspection_policy sysctl.
* Remove check symlink which can't be use as is anymore.
* Use socketpair(2) to test UNIX socket.
Changes since v7:
* Update tests with faccessat2/AT_INTERPRETED, including new ones to
check that setting R_OK or W_OK returns EINVAL.
* Add tests for memfd, pipefs and nsfs.
* Rename and move back tests to a standalone directory.
Changes since v6:
* Add full combination tests for all file types, including block
devices, character devices, fifos, sockets and symlinks.
* Properly save and restore initial sysctl value for all tests.
Changes since v5:
* Refactor with FIXTURE_VARIANT, which make the tests much more easy to
read and maintain.
* Save and restore initial sysctl value (suggested by Kees Cook).
* Test with a sysctl value of 0.
* Check errno in sysctl_access_write test.
* Update tests for the CAP_SYS_ADMIN switch.
* Update tests to check -EISDIR (replacing -EACCES).
* Replace FIXTURE_DATA() with FIXTURE() (spotted by Kees Cook).
* Use global const strings.
Changes since v3:
* Replace RESOLVE_MAYEXEC with O_MAYEXEC.
* Add tests to check that O_MAYEXEC is ignored by open(2) and openat(2).
Changes since v2:
* Move tests from exec/ to openat2/ .
* Replace O_MAYEXEC with RESOLVE_MAYEXEC from openat2(2).
* Cleanup tests.
Changes since v1:
* Move tests from yama/ to exec/ .
* Fix _GNU_SOURCE in kselftest_harness.h .
* Add a new test sysctl_access_write to check if CAP_MAC_ADMIN is taken
into account.
* Test directory execution which is always forbidden since commit
73601ea5b7b1 ("fs/open.c: allow opening only regular files during
execve()"), and also check that even the root user can not bypass file
execution checks.
* Make sure delete_workspace() always as enough right to succeed.
* Cosmetic cleanup.
---
tools/testing/selftests/exec/.gitignore | 2 +
tools/testing/selftests/exec/Makefile | 8 +
tools/testing/selftests/exec/config | 2 +
tools/testing/selftests/exec/false.c | 5 +
tools/testing/selftests/exec/should-exec.c | 449 +++++++++++++++++++++
5 files changed, 466 insertions(+)
create mode 100644 tools/testing/selftests/exec/config
create mode 100644 tools/testing/selftests/exec/false.c
create mode 100644 tools/testing/selftests/exec/should-exec.c
diff --git a/tools/testing/selftests/exec/.gitignore b/tools/testing/selftests/exec/.gitignore
index 90c238ba6a4b..20e965dcc98e 100644
--- a/tools/testing/selftests/exec/.gitignore
+++ b/tools/testing/selftests/exec/.gitignore
@@ -9,8 +9,10 @@ execveat.ephemeral
execveat.denatured
non-regular
null-argv
+/false
/load_address_*
/recursion-depth
+/should-exec
xxxxxxxx*
pipe
S_I*.test
diff --git a/tools/testing/selftests/exec/Makefile b/tools/testing/selftests/exec/Makefile
index fb4472ddffd8..fc0cb8925b02 100644
--- a/tools/testing/selftests/exec/Makefile
+++ b/tools/testing/selftests/exec/Makefile
@@ -2,15 +2,20 @@
CFLAGS = -Wall
CFLAGS += -Wno-nonnull
CFLAGS += -D_GNU_SOURCE
+CFLAGS += $(KHDR_INCLUDES)
+
+LDLIBS += -lcap
TEST_PROGS := binfmt_script.py
TEST_GEN_PROGS := execveat load_address_4096 load_address_2097152 load_address_16777216 non-regular
+TEST_GEN_PROGS_EXTENDED := false
TEST_GEN_FILES := execveat.symlink execveat.denatured script subdir
# Makefile is a run-time dependency, since it's accessed by the execveat test
TEST_FILES := Makefile
TEST_GEN_PROGS += recursion-depth
TEST_GEN_PROGS += null-argv
+TEST_GEN_PROGS += should-exec
EXTRA_CLEAN := $(OUTPUT)/subdir.moved $(OUTPUT)/execveat.moved $(OUTPUT)/xxxxx* \
$(OUTPUT)/S_I*.test
@@ -34,3 +39,6 @@ $(OUTPUT)/load_address_2097152: load_address.c
$(CC) $(CFLAGS) $(LDFLAGS) -Wl,-z,max-page-size=0x200000 -pie -static $< -o $@
$(OUTPUT)/load_address_16777216: load_address.c
$(CC) $(CFLAGS) $(LDFLAGS) -Wl,-z,max-page-size=0x1000000 -pie -static $< -o $@
+
+$(OUTPUT)/false: false.c
+ $(CC) $(CFLAGS) $(LDFLAGS) -static $< -o $@
diff --git a/tools/testing/selftests/exec/config b/tools/testing/selftests/exec/config
new file mode 100644
index 000000000000..c308079867b3
--- /dev/null
+++ b/tools/testing/selftests/exec/config
@@ -0,0 +1,2 @@
+CONFIG_BLK_DEV=y
+CONFIG_BLK_DEV_LOOP=y
diff --git a/tools/testing/selftests/exec/false.c b/tools/testing/selftests/exec/false.c
new file mode 100644
index 000000000000..104383ec3a79
--- /dev/null
+++ b/tools/testing/selftests/exec/false.c
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+int main(void)
+{
+ return 1;
+}
diff --git a/tools/testing/selftests/exec/should-exec.c b/tools/testing/selftests/exec/should-exec.c
new file mode 100644
index 000000000000..166276a39b4e
--- /dev/null
+++ b/tools/testing/selftests/exec/should-exec.c
@@ -0,0 +1,449 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test execveat(2) with AT_CHECK, and prctl(2) with SECBIT_SHOULD_EXEC_CHECK,
+ * SECBIT_SHOULD_EXEC_RESTRIC, and their locked counterparts.
+ *
+ * Copyright © 2018-2020 ANSSI
+ * Copyright © 2024 Microsoft Corporation
+ *
+ * Author: Mickaël Salaün <mic@digikod.net>
+ */
+
+#include <asm-generic/unistd.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/prctl.h>
+#include <linux/securebits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/capability.h>
+#include <sys/mount.h>
+#include <sys/prctl.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/sysmacros.h>
+#include <unistd.h>
+
+/* Defines AT_CHECK without type conflicts. */
+#define _ASM_GENERIC_FCNTL_H
+#include <linux/fcntl.h>
+
+#include "../kselftest_harness.h"
+
+static void drop_privileges(struct __test_metadata *const _metadata)
+{
+ const unsigned int noroot = SECBIT_NOROOT | SECBIT_NOROOT_LOCKED;
+ cap_t cap_p;
+
+ if ((cap_get_secbits() & noroot) != noroot)
+ EXPECT_EQ(0, cap_set_secbits(noroot));
+
+ cap_p = cap_get_proc();
+ EXPECT_NE(NULL, cap_p);
+ EXPECT_NE(-1, cap_clear(cap_p));
+
+ /*
+ * Drops everything, especially CAP_SETPCAP, CAP_DAC_OVERRIDE, and
+ * CAP_DAC_READ_SEARCH.
+ */
+ EXPECT_NE(-1, cap_set_proc(cap_p));
+ EXPECT_NE(-1, cap_free(cap_p));
+}
+
+static int test_secbits_set(const unsigned int secbits)
+{
+ int err;
+
+ err = prctl(PR_SET_SECUREBITS, secbits);
+ if (err)
+ return errno;
+ return 0;
+}
+
+FIXTURE(access)
+{
+ int memfd, pipefd;
+ int pipe_fds[2], socket_fds[2];
+};
+
+FIXTURE_VARIANT(access)
+{
+ const bool mount_exec;
+ const bool file_exec;
+};
+
+FIXTURE_VARIANT_ADD(access, mount_exec_file_exec){
+ .mount_exec = true,
+ .file_exec = true,
+};
+
+FIXTURE_VARIANT_ADD(access, mount_exec_file_noexec){
+ .mount_exec = true,
+ .file_exec = false,
+};
+
+FIXTURE_VARIANT_ADD(access, mount_noexec_file_exec){
+ .mount_exec = false,
+ .file_exec = true,
+};
+
+FIXTURE_VARIANT_ADD(access, mount_noexec_file_noexec){
+ .mount_exec = false,
+ .file_exec = false,
+};
+
+static const char binary_path[] = "./false";
+static const char workdir_path[] = "./test-mount";
+static const char reg_file_path[] = "./test-mount/regular_file";
+static const char dir_path[] = "./test-mount/directory";
+static const char block_dev_path[] = "./test-mount/block_device";
+static const char char_dev_path[] = "./test-mount/character_device";
+static const char fifo_path[] = "./test-mount/fifo";
+
+FIXTURE_SETUP(access)
+{
+ int procfd_path_size;
+ static const char path_template[] = "/proc/self/fd/%d";
+ char procfd_path[sizeof(path_template) + 10];
+
+ /* Makes sure we are not already restricted nor locked. */
+ EXPECT_EQ(0, test_secbits_set(0));
+
+ /*
+ * Cleans previous workspace if any error previously happened (don't
+ * check errors).
+ */
+ umount(workdir_path);
+ rmdir(workdir_path);
+
+ /* Creates a clean mount point. */
+ ASSERT_EQ(0, mkdir(workdir_path, 00700));
+ ASSERT_EQ(0, mount("test", workdir_path, "tmpfs",
+ MS_MGC_VAL | (variant->mount_exec ? 0 : MS_NOEXEC),
+ "mode=0700,size=9m"));
+
+ /* Creates a regular file. */
+ ASSERT_EQ(0, mknod(reg_file_path,
+ S_IFREG | (variant->file_exec ? 0700 : 0600), 0));
+ /* Creates a directory. */
+ ASSERT_EQ(0, mkdir(dir_path, variant->file_exec ? 0700 : 0600));
+ /* Creates a character device: /dev/null. */
+ ASSERT_EQ(0, mknod(char_dev_path, S_IFCHR | 0400, makedev(1, 3)));
+ /* Creates a block device: /dev/loop0 */
+ ASSERT_EQ(0, mknod(block_dev_path, S_IFBLK | 0400, makedev(7, 0)));
+ /* Creates a fifo. */
+ ASSERT_EQ(0, mknod(fifo_path, S_IFIFO | 0600, 0));
+
+ /* Creates a regular file without user mount point. */
+ self->memfd = memfd_create("test-exec-probe", MFD_CLOEXEC);
+ ASSERT_LE(0, self->memfd);
+ /* Sets mode, which must be ignored by the exec check. */
+ ASSERT_EQ(0, fchmod(self->memfd, variant->file_exec ? 0700 : 0600));
+
+ /* Creates a pipefs file descriptor. */
+ ASSERT_EQ(0, pipe(self->pipe_fds));
+ procfd_path_size = snprintf(procfd_path, sizeof(procfd_path),
+ path_template, self->pipe_fds[0]);
+ ASSERT_LT(procfd_path_size, sizeof(procfd_path));
+ self->pipefd = open(procfd_path, O_RDWR | O_CLOEXEC);
+ ASSERT_LE(0, self->pipefd);
+ ASSERT_EQ(0, fchmod(self->pipefd, variant->file_exec ? 0700 : 0600));
+
+ /* Creates a socket file descriptor. */
+ ASSERT_EQ(0, socketpair(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0,
+ self->socket_fds));
+}
+
+FIXTURE_TEARDOWN_PARENT(access)
+{
+ /* There is no need to unlink the test files. */
+ EXPECT_EQ(0, umount(workdir_path));
+ EXPECT_EQ(0, rmdir(workdir_path));
+}
+
+static void fill_exec_fd(struct __test_metadata *_metadata, const int fd_out)
+{
+ char buf[1024];
+ size_t len;
+ int fd_in;
+
+ fd_in = open(binary_path, O_CLOEXEC | O_RDONLY);
+ ASSERT_LE(0, fd_in);
+ /* Cannot use copy_file_range(2) because of EXDEV. */
+ len = read(fd_in, buf, sizeof(buf));
+ EXPECT_LE(0, len);
+ while (len > 0) {
+ EXPECT_EQ(len, write(fd_out, buf, len))
+ {
+ TH_LOG("Failed to write: %s (%d)", strerror(errno),
+ errno);
+ }
+ len = read(fd_in, buf, sizeof(buf));
+ EXPECT_LE(0, len);
+ }
+ EXPECT_EQ(0, close(fd_in));
+}
+
+static void fill_exec_path(struct __test_metadata *_metadata,
+ const char *const path)
+{
+ int fd_out;
+
+ fd_out = open(path, O_CLOEXEC | O_WRONLY);
+ ASSERT_LE(0, fd_out)
+ {
+ TH_LOG("Failed to open %s: %s", path, strerror(errno));
+ }
+ fill_exec_fd(_metadata, fd_out);
+ EXPECT_EQ(0, close(fd_out));
+}
+
+static void test_exec_fd(struct __test_metadata *_metadata, const int fd,
+ const int err_code)
+{
+ char *const argv[] = { "", NULL };
+ int access_ret, access_errno;
+
+ /*
+ * If we really execute fd, filled with the "false" binary, the current
+ * thread will exits with an error, which will be interpreted by the
+ * test framework as an error. With AT_CHECK, we only check a
+ * potential successful execution.
+ */
+ access_ret = execveat(fd, "", argv, NULL, AT_EMPTY_PATH | AT_CHECK);
+ access_errno = errno;
+ if (err_code) {
+ EXPECT_EQ(-1, access_ret);
+ EXPECT_EQ(err_code, access_errno)
+ {
+ TH_LOG("Wrong error for execveat(2): %s (%d)",
+ strerror(access_errno), errno);
+ }
+ } else {
+ EXPECT_EQ(0, access_ret)
+ {
+ TH_LOG("Access denied: %s", strerror(access_errno));
+ }
+ }
+}
+
+static void test_exec_path(struct __test_metadata *_metadata,
+ const char *const path, const int err_code)
+{
+ int flags = O_CLOEXEC;
+ int fd;
+
+ /* Do not block on pipes. */
+ if (path == fifo_path)
+ flags |= O_NONBLOCK;
+
+ fd = open(path, flags | O_RDONLY);
+ ASSERT_LE(0, fd)
+ {
+ TH_LOG("Failed to open %s: %s", path, strerror(errno));
+ }
+ test_exec_fd(_metadata, fd, err_code);
+ EXPECT_EQ(0, close(fd));
+}
+
+/* Tests that we don't get ENOEXEC. */
+TEST_F(access, regular_file_empty)
+{
+ const int exec = variant->mount_exec && variant->file_exec;
+
+ test_exec_path(_metadata, reg_file_path, exec ? 0 : EACCES);
+
+ drop_privileges(_metadata);
+ test_exec_path(_metadata, reg_file_path, exec ? 0 : EACCES);
+}
+
+TEST_F(access, regular_file_elf)
+{
+ const int exec = variant->mount_exec && variant->file_exec;
+
+ fill_exec_path(_metadata, reg_file_path);
+
+ test_exec_path(_metadata, reg_file_path, exec ? 0 : EACCES);
+
+ drop_privileges(_metadata);
+ test_exec_path(_metadata, reg_file_path, exec ? 0 : EACCES);
+}
+
+/* Tests that we don't get ENOEXEC. */
+TEST_F(access, memfd_empty)
+{
+ const int exec = variant->file_exec;
+
+ test_exec_fd(_metadata, self->memfd, exec ? 0 : EACCES);
+
+ drop_privileges(_metadata);
+ test_exec_fd(_metadata, self->memfd, exec ? 0 : EACCES);
+}
+
+TEST_F(access, memfd_elf)
+{
+ const int exec = variant->file_exec;
+
+ fill_exec_fd(_metadata, self->memfd);
+
+ test_exec_fd(_metadata, self->memfd, exec ? 0 : EACCES);
+
+ drop_privileges(_metadata);
+ test_exec_fd(_metadata, self->memfd, exec ? 0 : EACCES);
+}
+
+TEST_F(access, non_regular_files)
+{
+ test_exec_path(_metadata, dir_path, EACCES);
+ test_exec_path(_metadata, block_dev_path, EACCES);
+ test_exec_path(_metadata, char_dev_path, EACCES);
+ test_exec_path(_metadata, fifo_path, EACCES);
+ test_exec_fd(_metadata, self->socket_fds[0], EACCES);
+ test_exec_fd(_metadata, self->pipefd, EACCES);
+}
+
+
+/* clang-format off */
+FIXTURE(secbits) {};
+/* clang-format on */
+
+FIXTURE_VARIANT(secbits)
+{
+ const bool is_privileged;
+ const int error;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(secbits, priv) {
+ /* clang-format on */
+ .is_privileged = true,
+ .error = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(secbits, unpriv) {
+ /* clang-format on */
+ .is_privileged = false,
+ .error = EPERM,
+};
+
+FIXTURE_SETUP(secbits)
+{
+ /* Makes sure no should-exec bits are set. */
+ EXPECT_EQ(0, test_secbits_set(0));
+ EXPECT_EQ(0, prctl(PR_GET_SECUREBITS));
+
+ if (!variant->is_privileged)
+ drop_privileges(_metadata);
+}
+
+FIXTURE_TEARDOWN(secbits)
+{
+}
+
+TEST_F(secbits, legacy)
+{
+ EXPECT_EQ(variant->error, test_secbits_set(0));
+}
+
+#define CHILD(...) \
+ do { \
+ pid_t child = vfork(); \
+ EXPECT_LE(0, child); \
+ if (child == 0) { \
+ __VA_ARGS__; \
+ _exit(0); \
+ } \
+ } while (0)
+
+TEST_F(secbits, should_exec)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_SHOULD_EXEC_CHECK;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS));
+ CHILD(EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS)));
+
+ secbits |= SECBIT_SHOULD_EXEC_RESTRICT;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS));
+ CHILD(EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS)));
+
+ secbits &= ~(SECBIT_SHOULD_EXEC_CHECK | SECBIT_SHOULD_EXEC_RESTRICT);
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS));
+ CHILD(EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS)));
+}
+
+TEST_F(secbits, check_locked_set)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_SHOULD_EXEC_CHECK;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ secbits |= SECBIT_SHOULD_EXEC_CHECK_LOCKED;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+
+ /* Checks lock set but unchanged. */
+ EXPECT_EQ(variant->error, test_secbits_set(secbits));
+ CHILD(EXPECT_EQ(variant->error, test_secbits_set(secbits)));
+
+ secbits &= ~SECBIT_SHOULD_EXEC_CHECK;
+ EXPECT_EQ(EPERM, test_secbits_set(0));
+ CHILD(EXPECT_EQ(EPERM, test_secbits_set(0)));
+}
+
+TEST_F(secbits, check_locked_unset)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_SHOULD_EXEC_CHECK_LOCKED;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+
+ /* Checks lock unset but unchanged. */
+ EXPECT_EQ(variant->error, test_secbits_set(secbits));
+ CHILD(EXPECT_EQ(variant->error, test_secbits_set(secbits)));
+
+ secbits &= ~SECBIT_SHOULD_EXEC_CHECK;
+ EXPECT_EQ(EPERM, test_secbits_set(0));
+ CHILD(EXPECT_EQ(EPERM, test_secbits_set(0)));
+}
+
+TEST_F(secbits, restrict_locked_set)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_SHOULD_EXEC_RESTRICT;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ secbits |= SECBIT_SHOULD_EXEC_RESTRICT_LOCKED;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+
+ /* Checks lock set but unchanged. */
+ EXPECT_EQ(variant->error, test_secbits_set(secbits));
+ CHILD(EXPECT_EQ(variant->error, test_secbits_set(secbits)));
+
+ secbits &= ~SECBIT_SHOULD_EXEC_RESTRICT;
+ EXPECT_EQ(EPERM, test_secbits_set(0));
+ CHILD(EXPECT_EQ(EPERM, test_secbits_set(0)));
+}
+
+TEST_F(secbits, restrict_locked_unset)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_SHOULD_EXEC_RESTRICT_LOCKED;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+
+ /* Checks lock unset but unchanged. */
+ EXPECT_EQ(variant->error, test_secbits_set(secbits));
+ CHILD(EXPECT_EQ(variant->error, test_secbits_set(secbits)));
+
+ secbits &= ~SECBIT_SHOULD_EXEC_RESTRICT;
+ EXPECT_EQ(EPERM, test_secbits_set(0));
+ CHILD(EXPECT_EQ(EPERM, test_secbits_set(0)));
+}
+
+/* TODO: Add ptrace tests */
+
+TEST_HARNESS_MAIN
--
2.45.2
^ permalink raw reply related
* [RFC PATCH v19 4/5] selftests/landlock: Add tests for execveat + AT_CHECK
From: Mickaël Salaün @ 2024-07-04 19:01 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
Theodore Ts'o
Cc: Mickaël Salaün, Alejandro Colomar, Aleksa Sarai,
Andrew Morton, Andy Lutomirski, Arnd Bergmann, Casey Schaufler,
Christian Heimes, Dmitry Vyukov, Eric Biggers, Eric Chiang,
Fan Wu, Florian Weimer, Geert Uytterhoeven, James Morris,
Jan Kara, Jann Horn, Jeff Xu, Jonathan Corbet, Jordan R Abrahams,
Lakshmi Ramasubramanian, Luca Boccassi, Luis Chamberlain,
Madhavan T . Venkataraman, Matt Bobrowski, Matthew Garrett,
Matthew Wilcox, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Scott Shell, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Thibaut Sautereau, Vincent Strubel, Xiaoming Ni,
Yin Fengwei, kernel-hardening, linux-api, linux-fsdevel,
linux-integrity, linux-kernel, linux-security-module,
Günther Noack
In-Reply-To: <20240704190137.696169-1-mic@digikod.net>
Extend layout1.execute with the new AT_CHECK flag. The semantic with
AT_CHECK is the same as with a simple execve(2),
LANDLOCK_ACCESS_FS_EXECUTE is enforced the same way.
Cc: Günther Noack <gnoack@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20240704190137.696169-5-mic@digikod.net
---
tools/testing/selftests/landlock/fs_test.c | 26 ++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 7d063c652be1..85ef36b09a37 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -37,6 +37,10 @@
#include <linux/fs.h>
#include <linux/mount.h>
+/* Defines AT_CHECK without type conflicts. */
+#define _ASM_GENERIC_FCNTL_H
+#include <linux/fcntl.h>
+
#include "common.h"
#ifndef renameat2
@@ -2009,6 +2013,21 @@ static void test_execute(struct __test_metadata *const _metadata, const int err,
};
}
+static void test_check_exec(struct __test_metadata *const _metadata,
+ const int err, const char *const path)
+{
+ int ret;
+ char *const argv[] = { (char *)path, NULL };
+
+ ret = execveat(AT_FDCWD, path, argv, NULL, AT_EMPTY_PATH | AT_CHECK);
+ if (err) {
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(errno, err);
+ } else {
+ EXPECT_EQ(0, ret);
+ }
+}
+
TEST_F_FORK(layout1, execute)
{
const struct rule rules[] = {
@@ -2026,20 +2045,27 @@ TEST_F_FORK(layout1, execute)
copy_binary(_metadata, file1_s1d2);
copy_binary(_metadata, file1_s1d3);
+ /* Checks before file1_s1d1 being denied. */
+ test_execute(_metadata, 0, file1_s1d1);
+ test_check_exec(_metadata, 0, file1_s1d1);
+
enforce_ruleset(_metadata, ruleset_fd);
ASSERT_EQ(0, close(ruleset_fd));
ASSERT_EQ(0, test_open(dir_s1d1, O_RDONLY));
ASSERT_EQ(0, test_open(file1_s1d1, O_RDONLY));
test_execute(_metadata, EACCES, file1_s1d1);
+ test_check_exec(_metadata, EACCES, file1_s1d1);
ASSERT_EQ(0, test_open(dir_s1d2, O_RDONLY));
ASSERT_EQ(0, test_open(file1_s1d2, O_RDONLY));
test_execute(_metadata, 0, file1_s1d2);
+ test_check_exec(_metadata, 0, file1_s1d2);
ASSERT_EQ(0, test_open(dir_s1d3, O_RDONLY));
ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY));
test_execute(_metadata, 0, file1_s1d3);
+ test_check_exec(_metadata, 0, file1_s1d3);
}
TEST_F_FORK(layout1, link)
--
2.45.2
^ permalink raw reply related
* [RFC PATCH v19 5/5] samples/should-exec: Add set-should-exec
From: Mickaël Salaün @ 2024-07-04 19:01 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
Theodore Ts'o
Cc: Mickaël Salaün, Alejandro Colomar, Aleksa Sarai,
Andrew Morton, Andy Lutomirski, Arnd Bergmann, Casey Schaufler,
Christian Heimes, Dmitry Vyukov, Eric Biggers, Eric Chiang,
Fan Wu, Florian Weimer, Geert Uytterhoeven, James Morris,
Jan Kara, Jann Horn, Jeff Xu, Jonathan Corbet, Jordan R Abrahams,
Lakshmi Ramasubramanian, Luca Boccassi, Luis Chamberlain,
Madhavan T . Venkataraman, Matt Bobrowski, Matthew Garrett,
Matthew Wilcox, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Scott Shell, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Thibaut Sautereau, Vincent Strubel, Xiaoming Ni,
Yin Fengwei, kernel-hardening, linux-api, linux-fsdevel,
linux-integrity, linux-kernel, linux-security-module
In-Reply-To: <20240704190137.696169-1-mic@digikod.net>
Add a simple tool to set SECBIT_SHOULD_EXEC_CHECK,
SECBIT_SHOULD_EXEC_RESTRICT, and their lock counterparts before
executing a command. This should be useful to easily test against
script interpreters.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20240704190137.696169-6-mic@digikod.net
---
samples/Kconfig | 7 +++
samples/Makefile | 1 +
samples/should-exec/.gitignore | 1 +
samples/should-exec/Makefile | 13 ++++
samples/should-exec/set-should-exec.c | 88 +++++++++++++++++++++++++++
5 files changed, 110 insertions(+)
create mode 100644 samples/should-exec/.gitignore
create mode 100644 samples/should-exec/Makefile
create mode 100644 samples/should-exec/set-should-exec.c
diff --git a/samples/Kconfig b/samples/Kconfig
index b288d9991d27..d8f2639bc830 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -180,6 +180,13 @@ config SAMPLE_SECCOMP
Build samples of seccomp filters using various methods of
BPF filter construction.
+config SAMPLE_SHOULD_EXEC
+ bool "Should-exec secure bits examples"
+ depends on CC_CAN_LINK && HEADERS_INSTALL
+ help
+ Build a tool to easily configure SECBIT_SHOULD_EXEC_CHECK,
+ SECBIT_SHOULD_EXEC_RESTRICT and their lock counterparts.
+
config SAMPLE_TIMER
bool "Timer sample"
depends on CC_CAN_LINK && HEADERS_INSTALL
diff --git a/samples/Makefile b/samples/Makefile
index b85fa64390c5..0e7a97fb222d 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -19,6 +19,7 @@ subdir-$(CONFIG_SAMPLE_PIDFD) += pidfd
obj-$(CONFIG_SAMPLE_QMI_CLIENT) += qmi/
obj-$(CONFIG_SAMPLE_RPMSG_CLIENT) += rpmsg/
subdir-$(CONFIG_SAMPLE_SECCOMP) += seccomp
+subdir-$(CONFIG_SAMPLE_SHOULD_EXEC) += should-exec
subdir-$(CONFIG_SAMPLE_TIMER) += timers
obj-$(CONFIG_SAMPLE_TRACE_EVENTS) += trace_events/
obj-$(CONFIG_SAMPLE_TRACE_CUSTOM_EVENTS) += trace_events/
diff --git a/samples/should-exec/.gitignore b/samples/should-exec/.gitignore
new file mode 100644
index 000000000000..ac46c614ec80
--- /dev/null
+++ b/samples/should-exec/.gitignore
@@ -0,0 +1 @@
+/set-should-exec
diff --git a/samples/should-exec/Makefile b/samples/should-exec/Makefile
new file mode 100644
index 000000000000..c4294278dd07
--- /dev/null
+++ b/samples/should-exec/Makefile
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: BSD-3-Clause
+
+userprogs-always-y := set-should-exec
+
+userccflags += -I usr/include
+
+.PHONY: all clean
+
+all:
+ $(MAKE) -C ../.. samples/should-exec/
+
+clean:
+ $(MAKE) -C ../.. M=samples/should-exec/ clean
diff --git a/samples/should-exec/set-should-exec.c b/samples/should-exec/set-should-exec.c
new file mode 100644
index 000000000000..b3c31106d916
--- /dev/null
+++ b/samples/should-exec/set-should-exec.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Simple tool to set SECBIT_SHOULD_EXEC_CHECK, SECBIT_SHOULD_EXEC_RESTRICT,
+ * and their lock counterparts before executing a command.
+ *
+ * Copyright © 2024 Microsoft Corporation
+ */
+
+#define _GNU_SOURCE
+#define __SANE_USERSPACE_TYPES__
+#include <errno.h>
+#include <linux/prctl.h>
+#include <linux/securebits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/prctl.h>
+#include <unistd.h>
+
+static void print_usage(const char *argv0)
+{
+ fprintf(stderr, "usage: %s -c|-r [-l] -- <cmd> [args]...\n\n", argv0);
+ fprintf(stderr, "Execute a command with\n");
+ fprintf(stderr, "- SECBIT_SHOULD_EXEC_CHECK set: -c\n");
+ fprintf(stderr, "- SECBIT_SHOULD_EXEC_RESTRICT set: -r\n");
+ fprintf(stderr, "- SECBIT_SHOULD_EXEC_*_LOCKED set: -l\n");
+}
+
+int main(const int argc, char *const argv[], char *const *const envp)
+{
+ const char *cmd_path;
+ char *const *cmd_argv;
+ int opt, secbits, err;
+ bool has_policy = false;
+
+ secbits = prctl(PR_GET_SECUREBITS);
+
+ while ((opt = getopt(argc, argv, "crl")) != -1) {
+ switch (opt) {
+ case 'c':
+ secbits |= SECBIT_SHOULD_EXEC_CHECK;
+ has_policy = true;
+ break;
+ case 'r':
+ secbits |= SECBIT_SHOULD_EXEC_RESTRICT;
+ has_policy = true;
+ break;
+ case 'l':
+ secbits |= SECBIT_SHOULD_EXEC_CHECK_LOCKED;
+ secbits |= SECBIT_SHOULD_EXEC_RESTRICT_LOCKED;
+ break;
+ default:
+ print_usage(argv[0]);
+ return 1;
+ }
+ }
+
+ if (!argv[optind] || !has_policy) {
+ print_usage(argv[0]);
+ return 1;
+ }
+
+ err = prctl(PR_SET_SECUREBITS, secbits);
+ if (err) {
+ perror("Failed to set secure bit(s).");
+ fprintf(stderr,
+ "Hint: The running kernel may not support this feature.\n");
+ return 1;
+ }
+
+ fprintf(stderr, "SECBIT_SHOULD_EXEC_CHECK: %d\n",
+ !!(secbits & SECBIT_SHOULD_EXEC_CHECK));
+ fprintf(stderr, "SECBIT_SHOULD_EXEC_CHECK_LOCKED: %d\n",
+ !!(secbits & SECBIT_SHOULD_EXEC_CHECK_LOCKED));
+ fprintf(stderr, "SECBIT_SHOULD_EXEC_RESTRICT: %d\n",
+ !!(secbits & SECBIT_SHOULD_EXEC_RESTRICT));
+ fprintf(stderr, "SECBIT_SHOULD_EXEC_RESTRICT_LOCKED: %d\n",
+ !!(secbits & SECBIT_SHOULD_EXEC_RESTRICT_LOCKED));
+
+ cmd_path = argv[optind];
+ cmd_argv = argv + optind;
+ fprintf(stderr, "Executing command...\n");
+ execvpe(cmd_path, cmd_argv, envp);
+ fprintf(stderr, "Failed to execute \"%s\": %s\n", cmd_path,
+ strerror(errno));
+ return 1;
+}
--
2.45.2
^ permalink raw reply related
* [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)
From: Mickaël Salaün @ 2024-07-04 19:01 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
Theodore Ts'o
Cc: Mickaël Salaün, Alejandro Colomar, Aleksa Sarai,
Andrew Morton, Andy Lutomirski, Arnd Bergmann, Casey Schaufler,
Christian Heimes, Dmitry Vyukov, Eric Biggers, Eric Chiang,
Fan Wu, Florian Weimer, Geert Uytterhoeven, James Morris,
Jan Kara, Jann Horn, Jeff Xu, Jonathan Corbet, Jordan R Abrahams,
Lakshmi Ramasubramanian, Luca Boccassi, Luis Chamberlain,
Madhavan T . Venkataraman, Matt Bobrowski, Matthew Garrett,
Matthew Wilcox, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Scott Shell, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Thibaut Sautereau, Vincent Strubel, Xiaoming Ni,
Yin Fengwei, kernel-hardening, linux-api, linux-fsdevel,
linux-integrity, linux-kernel, linux-security-module
In-Reply-To: <20240704190137.696169-1-mic@digikod.net>
Add a new AT_CHECK flag to execveat(2) to check if a file would be
allowed for execution. The main use case is for script interpreters and
dynamic linkers to check execution permission according to the kernel's
security policy. Another use case is to add context to access logs e.g.,
which script (instead of interpreter) accessed a file. As any
executable code, scripts could also use this check [1].
This is different than faccessat(2) which only checks file access
rights, but not the full context e.g. mount point's noexec, stack limit,
and all potential LSM extra checks (e.g. argv, envp, credentials).
Since the use of AT_CHECK follows the exact kernel semantic as for a
real execution, user space gets the same error codes.
With the information that a script interpreter is about to interpret a
script, an LSM security policy can adjust caller's access rights or log
execution request as for native script execution (e.g. role transition).
This is possible thanks to the call to security_bprm_creds_for_exec().
Because LSMs may only change bprm's credentials, use of AT_CHECK with
current kernel code should not be a security issue (e.g. unexpected role
transition). LSMs willing to update the caller's credential could now
do so when bprm->is_check is set. Of course, such policy change should
be in line with the new user space code.
Because AT_CHECK is dedicated to user space interpreters, it doesn't
make sense for the kernel to parse the checked files, look for
interpreters known to the kernel (e.g. ELF, shebang), and return ENOEXEC
if the format is unknown. Because of that, security_bprm_check() is
never called when AT_CHECK is used.
It should be noted that script interpreters cannot directly use
execveat(2) (without this new AT_CHECK flag) because this could lead to
unexpected behaviors e.g., `python script.sh` could lead to Bash being
executed to interpret the script. Unlike the kernel, script
interpreters may just interpret the shebang as a simple comment, which
should not change for backward compatibility reasons.
Because scripts or libraries files might not currently have the
executable permission set, or because we might want specific users to be
allowed to run arbitrary scripts, the following patch provides a dynamic
configuration mechanism with the SECBIT_SHOULD_EXEC_CHECK and
SECBIT_SHOULD_EXEC_RESTRICT securebits.
This is a redesign of the CLIP OS 4's O_MAYEXEC:
https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
This patch has been used for more than a decade with customized script
interpreters. Some examples can be found here:
https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Link: https://docs.python.org/3/library/io.html#io.open_code [1]
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20240704190137.696169-2-mic@digikod.net
---
New design since v18:
https://lore.kernel.org/r/20220104155024.48023-3-mic@digikod.net
---
fs/exec.c | 5 +++--
include/linux/binfmts.h | 7 ++++++-
include/uapi/linux/fcntl.h | 30 ++++++++++++++++++++++++++++++
kernel/audit.h | 1 +
kernel/auditsc.c | 1 +
5 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 40073142288f..ea2a1867afdc 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -931,7 +931,7 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags)
.lookup_flags = LOOKUP_FOLLOW,
};
- if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+ if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH | AT_CHECK)) != 0)
return ERR_PTR(-EINVAL);
if (flags & AT_SYMLINK_NOFOLLOW)
open_exec_flags.lookup_flags &= ~LOOKUP_FOLLOW;
@@ -1595,6 +1595,7 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int fl
bprm->filename = bprm->fdpath;
}
bprm->interp = bprm->filename;
+ bprm->is_check = !!(flags & AT_CHECK);
retval = bprm_mm_init(bprm);
if (!retval)
@@ -1885,7 +1886,7 @@ static int bprm_execve(struct linux_binprm *bprm)
/* Set the unchanging part of bprm->cred */
retval = security_bprm_creds_for_exec(bprm);
- if (retval)
+ if (retval || bprm->is_check)
goto out;
retval = exec_binprm(bprm);
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 70f97f685bff..8ff9c9e33aed 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -42,7 +42,12 @@ struct linux_binprm {
* Set when errors can no longer be returned to the
* original userspace.
*/
- point_of_no_return:1;
+ point_of_no_return:1,
+ /*
+ * Set by user space to check executability according to the
+ * caller's environment.
+ */
+ is_check:1;
struct file *executable; /* Executable to pass to the interpreter */
struct file *interpreter;
struct file *file;
diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index c0bcc185fa48..bcd05c59b7df 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -118,6 +118,36 @@
#define AT_HANDLE_FID AT_REMOVEDIR /* file handle is needed to
compare object identity and may not
be usable to open_by_handle_at(2) */
+
+/*
+ * AT_CHECK only performs a check on a regular file and returns 0 if execution
+ * of this file would be allowed, ignoring the file format and then the related
+ * interpreter dependencies (e.g. ELF libraries, script's shebang). AT_CHECK
+ * should only be used if SECBIT_SHOULD_EXEC_CHECK is set for the calling
+ * thread. See securebits.h documentation.
+ *
+ * Programs should use this check to apply kernel-level checks against files
+ * that are not directly executed by the kernel but directly passed to a user
+ * space interpreter instead. All files that contain executable code, from the
+ * point of view of the interpreter, should be checked. The main purpose of
+ * this flag is to improve the security and consistency of an execution
+ * environment to ensure that direct file execution (e.g. ./script.sh) and
+ * indirect file execution (e.g. sh script.sh) lead to the same result. For
+ * instance, this can be used to check if a file is trustworthy according to
+ * the caller's environment.
+ *
+ * In a secure environment, libraries and any executable dependencies should
+ * also be checked. For instance dynamic linking should make sure that all
+ * libraries are allowed for execution to avoid trivial bypass (e.g. using
+ * LD_PRELOAD). For such secure execution environment to make sense, only
+ * trusted code should be executable, which also requires integrity guarantees.
+ *
+ * To avoid race conditions leading to time-of-check to time-of-use issues,
+ * AT_CHECK should be used with AT_EMPTY_PATH to check against a file
+ * descriptor instead of a path.
+ */
+#define AT_CHECK 0x10000
+
#if defined(__KERNEL__)
#define AT_GETATTR_NOSEC 0x80000000
#endif
diff --git a/kernel/audit.h b/kernel/audit.h
index a60d2840559e..8ebdabd2ab81 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -197,6 +197,7 @@ struct audit_context {
struct open_how openat2;
struct {
int argc;
+ bool is_check;
} execve;
struct {
char *name;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 6f0d6fb6523f..b6316e284342 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2662,6 +2662,7 @@ void __audit_bprm(struct linux_binprm *bprm)
context->type = AUDIT_EXECVE;
context->execve.argc = bprm->argc;
+ context->execve.is_check = bprm->is_check;
}
--
2.45.2
^ permalink raw reply related
* [RFC PATCH v19 2/5] security: Add new SHOULD_EXEC_CHECK and SHOULD_EXEC_RESTRICT securebits
From: Mickaël Salaün @ 2024-07-04 19:01 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Linus Torvalds, Paul Moore,
Theodore Ts'o
Cc: Mickaël Salaün, Alejandro Colomar, Aleksa Sarai,
Andrew Morton, Andy Lutomirski, Arnd Bergmann, Casey Schaufler,
Christian Heimes, Dmitry Vyukov, Eric Biggers, Eric Chiang,
Fan Wu, Florian Weimer, Geert Uytterhoeven, James Morris,
Jan Kara, Jann Horn, Jeff Xu, Jonathan Corbet, Jordan R Abrahams,
Lakshmi Ramasubramanian, Luca Boccassi, Luis Chamberlain,
Madhavan T . Venkataraman, Matt Bobrowski, Matthew Garrett,
Matthew Wilcox, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Scott Shell, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Thibaut Sautereau, Vincent Strubel, Xiaoming Ni,
Yin Fengwei, kernel-hardening, linux-api, linux-fsdevel,
linux-integrity, linux-kernel, linux-security-module
In-Reply-To: <20240704190137.696169-1-mic@digikod.net>
These new SECBIT_SHOULD_EXEC_CHECK, SECBIT_SHOULD_EXEC_RESTRICT, and
their *_LOCKED counterparts are designed to be set by processes setting
up an execution environment, such as a user session, a container, or a
security sandbox. Like seccomp filters or Landlock domains, the
securebits are inherited across proceses.
When SECBIT_SHOULD_EXEC_CHECK is set, programs interpreting code should
check executable resources with execveat(2) + AT_CHECK (see previous
patch).
When SECBIT_SHOULD_EXEC_RESTRICT is set, a process should only allow
execution of approved resources, if any (see SECBIT_SHOULD_EXEC_CHECK).
For a secure environment, we might also want
SECBIT_SHOULD_EXEC_CHECK_LOCKED and SECBIT_SHOULD_EXEC_RESTRICT_LOCKED
to be set. For a test environment (e.g. testing on a fleet to identify
potential issues), only the SECBIT_SHOULD_EXEC_CHECK* bits can be set to
still be able to identify potential issues (e.g. with interpreters logs
or LSMs audit entries).
It should be noted that unlike other security bits, the
SECBIT_SHOULD_EXEC_CHECK and SECBIT_SHOULD_EXEC_RESTRICT bits are
dedicated to user space willing to restrict itself. Because of that,
they only make sense in the context of a trusted environment (e.g.
sandbox, container, user session, full system) where the process
changing its behavior (according to these bits) and all its parent
processes are trusted. Otherwise, any parent process could just execute
its own malicious code (interpreting a script or not), or even enforce a
seccomp filter to mask these bits.
Such a secure environment can be achieved with an appropriate access
control policy (e.g. mount's noexec option, file access rights, LSM
configuration) and an enlighten ld.so checking that libraries are
allowed for execution e.g., to protect against illegitimate use of
LD_PRELOAD.
Scripts may need some changes to deal with untrusted data (e.g. stdin,
environment variables), but that is outside the scope of the kernel.
The only restriction enforced by the kernel is the right to ptrace
another process. Processes are denied to ptrace less restricted ones,
unless the tracer has CAP_SYS_PTRACE. This is mainly a safeguard to
avoid trivial privilege escalations e.g., by a debugging process being
abused with a confused deputy attack.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20240704190137.696169-3-mic@digikod.net
---
New design since v18:
https://lore.kernel.org/r/20220104155024.48023-3-mic@digikod.net
---
include/uapi/linux/securebits.h | 56 ++++++++++++++++++++++++++++-
security/commoncap.c | 63 ++++++++++++++++++++++++++++-----
2 files changed, 110 insertions(+), 9 deletions(-)
diff --git a/include/uapi/linux/securebits.h b/include/uapi/linux/securebits.h
index d6d98877ff1a..3fdb0382718b 100644
--- a/include/uapi/linux/securebits.h
+++ b/include/uapi/linux/securebits.h
@@ -52,10 +52,64 @@
#define SECBIT_NO_CAP_AMBIENT_RAISE_LOCKED \
(issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE_LOCKED))
+/*
+ * When SECBIT_SHOULD_EXEC_CHECK is set, a process should check all executable
+ * files with execveat(2) + AT_CHECK. However, such check should only be
+ * performed if all to-be-executed code only comes from regular files. For
+ * instance, if a script interpreter is called with both a script snipped as
+ * argument and a regular file, the interpreter should not check any file.
+ * Doing otherwise would mislead the kernel to think that only the script file
+ * is being executed, which could for instance lead to unexpected permission
+ * change and break current use cases.
+ *
+ * This secure bit may be set by user session managers, service managers,
+ * container runtimes, sandboxer tools... Except for test environments, the
+ * related SECBIT_SHOULD_EXEC_CHECK_LOCKED bit should also be set.
+ *
+ * Ptracing another process is deny if the tracer has SECBIT_SHOULD_EXEC_CHECK
+ * but not the tracee. SECBIT_SHOULD_EXEC_CHECK_LOCKED also checked.
+ */
+#define SECURE_SHOULD_EXEC_CHECK 8
+#define SECURE_SHOULD_EXEC_CHECK_LOCKED 9 /* make bit-8 immutable */
+
+#define SECBIT_SHOULD_EXEC_CHECK (issecure_mask(SECURE_SHOULD_EXEC_CHECK))
+#define SECBIT_SHOULD_EXEC_CHECK_LOCKED \
+ (issecure_mask(SECURE_SHOULD_EXEC_CHECK_LOCKED))
+
+/*
+ * When SECBIT_SHOULD_EXEC_RESTRICT is set, a process should only allow
+ * execution of approved files, if any (see SECBIT_SHOULD_EXEC_CHECK). For
+ * instance, script interpreters called with a script snippet as argument
+ * should always deny such execution if SECBIT_SHOULD_EXEC_RESTRICT is set.
+ * However, if a script interpreter is called with both
+ * SECBIT_SHOULD_EXEC_CHECK and SECBIT_SHOULD_EXEC_RESTRICT, they should
+ * interpret the provided script files if no unchecked code is also provided
+ * (e.g. directly as argument).
+ *
+ * This secure bit may be set by user session managers, service managers,
+ * container runtimes, sandboxer tools... Except for test environments, the
+ * related SECBIT_SHOULD_EXEC_RESTRICT_LOCKED bit should also be set.
+ *
+ * Ptracing another process is deny if the tracer has
+ * SECBIT_SHOULD_EXEC_RESTRICT but not the tracee.
+ * SECBIT_SHOULD_EXEC_RESTRICT_LOCKED is also checked.
+ */
+#define SECURE_SHOULD_EXEC_RESTRICT 10
+#define SECURE_SHOULD_EXEC_RESTRICT_LOCKED 11 /* make bit-8 immutable */
+
+#define SECBIT_SHOULD_EXEC_RESTRICT (issecure_mask(SECURE_SHOULD_EXEC_RESTRICT))
+#define SECBIT_SHOULD_EXEC_RESTRICT_LOCKED \
+ (issecure_mask(SECURE_SHOULD_EXEC_RESTRICT_LOCKED))
+
#define SECURE_ALL_BITS (issecure_mask(SECURE_NOROOT) | \
issecure_mask(SECURE_NO_SETUID_FIXUP) | \
issecure_mask(SECURE_KEEP_CAPS) | \
- issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE))
+ issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE) | \
+ issecure_mask(SECURE_SHOULD_EXEC_CHECK) | \
+ issecure_mask(SECURE_SHOULD_EXEC_RESTRICT))
#define SECURE_ALL_LOCKS (SECURE_ALL_BITS << 1)
+#define SECURE_ALL_UNPRIVILEGED (issecure_mask(SECURE_SHOULD_EXEC_CHECK) | \
+ issecure_mask(SECURE_SHOULD_EXEC_RESTRICT))
+
#endif /* _UAPI_LINUX_SECUREBITS_H */
diff --git a/security/commoncap.c b/security/commoncap.c
index 162d96b3a676..34b4493e2a69 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -117,6 +117,33 @@ int cap_settime(const struct timespec64 *ts, const struct timezone *tz)
return 0;
}
+static bool ptrace_secbits_allowed(const struct cred *tracer,
+ const struct cred *tracee)
+{
+ const unsigned long tracer_secbits = SECURE_ALL_UNPRIVILEGED &
+ tracer->securebits;
+ const unsigned long tracee_secbits = SECURE_ALL_UNPRIVILEGED &
+ tracee->securebits;
+ /* Ignores locking of unset secure bits (cf. SECURE_ALL_LOCKS). */
+ const unsigned long tracer_locked = (tracer_secbits << 1) &
+ tracer->securebits;
+ const unsigned long tracee_locked = (tracee_secbits << 1) &
+ tracee->securebits;
+
+ /* The tracee must not have less constraints than the tracer. */
+ if ((tracer_secbits | tracee_secbits) != tracee_secbits)
+ return false;
+
+ /*
+ * Makes sure that the tracer's locks for restrictions are the same for
+ * the tracee.
+ */
+ if ((tracer_locked | tracee_locked) != tracee_locked)
+ return false;
+
+ return true;
+}
+
/**
* cap_ptrace_access_check - Determine whether the current process may access
* another
@@ -146,7 +173,8 @@ int cap_ptrace_access_check(struct task_struct *child, unsigned int mode)
else
caller_caps = &cred->cap_permitted;
if (cred->user_ns == child_cred->user_ns &&
- cap_issubset(child_cred->cap_permitted, *caller_caps))
+ cap_issubset(child_cred->cap_permitted, *caller_caps) &&
+ ptrace_secbits_allowed(cred, child_cred))
goto out;
if (ns_capable(child_cred->user_ns, CAP_SYS_PTRACE))
goto out;
@@ -178,7 +206,8 @@ int cap_ptrace_traceme(struct task_struct *parent)
cred = __task_cred(parent);
child_cred = current_cred();
if (cred->user_ns == child_cred->user_ns &&
- cap_issubset(child_cred->cap_permitted, cred->cap_permitted))
+ cap_issubset(child_cred->cap_permitted, cred->cap_permitted) &&
+ ptrace_secbits_allowed(cred, child_cred))
goto out;
if (has_ns_capability(parent, child_cred->user_ns, CAP_SYS_PTRACE))
goto out;
@@ -1302,21 +1331,39 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
& (old->securebits ^ arg2)) /*[1]*/
|| ((old->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/
|| (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/
- || (cap_capable(current_cred(),
- current_cred()->user_ns,
- CAP_SETPCAP,
- CAP_OPT_NONE) != 0) /*[4]*/
/*
* [1] no changing of bits that are locked
* [2] no unlocking of locks
* [3] no setting of unsupported bits
- * [4] doing anything requires privilege (go read about
- * the "sendmail capabilities bug")
*/
)
/* cannot change a locked bit */
return -EPERM;
+ /*
+ * Doing anything requires privilege (go read about the
+ * "sendmail capabilities bug"), except for unprivileged bits.
+ * Indeed, the SECURE_ALL_UNPRIVILEGED bits are not
+ * restrictions enforced by the kernel but by user space on
+ * itself. The kernel is only in charge of protecting against
+ * privilege escalation with ptrace protections.
+ */
+ if (cap_capable(current_cred(), current_cred()->user_ns,
+ CAP_SETPCAP, CAP_OPT_NONE) != 0) {
+ const unsigned long unpriv_and_locks =
+ SECURE_ALL_UNPRIVILEGED |
+ SECURE_ALL_UNPRIVILEGED << 1;
+ const unsigned long changed = old->securebits ^ arg2;
+
+ /* For legacy reason, denies non-change. */
+ if (!changed)
+ return -EPERM;
+
+ /* Denies privileged changes. */
+ if (changed & ~unpriv_and_locks)
+ return -EPERM;
+ }
+
new = prepare_creds();
if (!new)
return -ENOMEM;
--
2.45.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox