From 9426ade562c31bced4ccdb53dc27d2f51d79fb6a Mon Sep 17 00:00:00 2001 From: Brooks Davis Date: Tue, 7 Jan 2025 19:27:39 +0000 Subject: [PATCH] mmap: disallow MAP_STACK into a reservation It doesn't make sense to insert a stack into a reservation so disallow MAP_FIXED|MAP_STACK with a valid capability in addr. It's still allowed to MAP_FIXED|MAP_STACK, but only if there's nothing in that location (and you shouldn't be doing that). --- bin/cheribsdtest/cheribsdtest_vm.c | 15 ++++++++------- lib/libsys/mmap.2 | 8 ++++++++ sys/vm/vm_mmap.c | 2 ++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/bin/cheribsdtest/cheribsdtest_vm.c b/bin/cheribsdtest/cheribsdtest_vm.c index 7dfe96cf6dcf..021a3f79ccd3 100644 --- a/bin/cheribsdtest/cheribsdtest_vm.c +++ b/bin/cheribsdtest/cheribsdtest_vm.c @@ -2851,7 +2851,8 @@ CHERIBSDTEST(cheri_revoke_shm_anon_hoard_closed, * caused a panic. * https://github.com/CTSRD-CHERI/cheribsd/issues/2252 */ -CHERIBSDTEST(mmap_insert_stack, "insert a stack mapping in a reservation") +CHERIBSDTEST(mmap_insert_stack, + "try to insert a stack mapping in a reservation") { void *p; @@ -2860,13 +2861,13 @@ CHERIBSDTEST(mmap_insert_stack, "insert a stack mapping in a reservation") MAP_FIXED | MAP_ANONYMOUS | MAP_PRIVATE, -1, 0)); /* - * This would fail, but leave the map in a broken state due to - * trying to insert a reservation inside an existing one. - * - * We don't check it because it is needed to set up the panic. + * Historically would fail, but leave the map in a broken state + * due to trying to insert a reservation inside an existing one. + * This is now rejected outright. */ - mmap(cheri_setaddress(p, 0x20ffc000), 0x2000, - PROT_WRITE | PROT_READ, MAP_STACK | MAP_FIXED, -1, 0); + CHERIBSDTEST_CHECK_CALL_ERROR(mmap(cheri_setaddress(p, 0x20ffc000), + 0x2000, PROT_WRITE | PROT_READ, MAP_STACK | MAP_FIXED, -1, 0), + ENOMEM); /* * This would trigger a panic by trying to remove an unmapped diff --git a/lib/libsys/mmap.2 b/lib/libsys/mmap.2 index 54c8750f8925..b532302e860e 100644 --- a/lib/libsys/mmap.2 +++ b/lib/libsys/mmap.2 @@ -409,6 +409,14 @@ and .Dv PROT_WRITE . The size of the guard, in pages, is specified by sysctl .Dv security.bsd.stack_guard_page . +.Pp +Under CheriABI, +.Dv MAP_STACK +may not be combined with +.Dv MAP_FIXED +and an +.Fa addr +argument that is a valid pointer. .El .Pp The diff --git a/sys/vm/vm_mmap.c b/sys/vm/vm_mmap.c index 4cbd76031c1e..0104d1bc792a 100644 --- a/sys/vm/vm_mmap.c +++ b/sys/vm/vm_mmap.c @@ -335,6 +335,8 @@ sys_mmap(struct thread *td, struct mmap_args *uap) if (cheri_gettag(uap->addr)) { if ((flags & MAP_FIXED) == 0) return (EPROT); + else if ((flags & MAP_STACK) != 0) + return (ENOMEM); else if ((cheri_getperm(uap->addr) & CHERI_PERM_SW_VMEM)) source_cap = uap->addr; else {