Skip to content

Commit

Permalink
Allow disabling quarantine of unmapped reservations
Browse files Browse the repository at this point in the history
Add a tunable (vm.cheri_revoke.quarantine_unmapped_reservations) which
is enabled by default.  When disabled, reservations are destroyed
once completely unmapped.  The remaining infrastructure is idled as no
vm entries ever become quarantined.
  • Loading branch information
brooksdavis committed Feb 14, 2023
1 parent db1b8fc commit 8fd603e
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 22 deletions.
61 changes: 45 additions & 16 deletions bin/cheribsdtest/cheribsdtest_vm.c
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,31 @@ CHERIBSDTEST(vm_reservation_align,
cheribsdtest_success();
}

static bool
reservations_are_quarantined(void)
{
uint8_t quarantine_unmapped_reservations;
size_t quarantine_unmapped_reservations_sz =
sizeof(quarantine_unmapped_reservations);

CHERIBSDTEST_CHECK_SYSCALL(
sysctlbyname("vm.cheri_revoke.quarantine_unmapped_reservations",
&quarantine_unmapped_reservations,
&quarantine_unmapped_reservations_sz, NULL, 0));

return (quarantine_unmapped_reservations != 0);
}


static const char *
skip_need_quarantine_unmapped_reservations(const char *name __unused)
{
if (reservations_are_quarantined())
return (NULL);
else
return ("unmapped reservations are not being quarantined");
}

/*
* Check that after a reservation is unmapped, it is not possible to
* reuse the old capability to create new fixed mappings.
Expand All @@ -842,22 +867,24 @@ CHERIBSDTEST(vm_reservation_mmap_after_free_fixed,
map = mmap(map, PAGE_SIZE, PROT_READ | PROT_WRITE,
MAP_ANON | MAP_FIXED, -1, 0);
CHERIBSDTEST_VERIFY2(map == MAP_FAILED, "mmap after free succeeded");

#ifdef CHERI_REVOKE
/*
* There's nothing to cause the quarantined reservation to be revoked
* between the munmap and mmap calls so we'll get an ENOMEM
* here.
*
* XXX: ideally we'd trigger a revocation of this specific
* reservation before the mmap call to test the same case with
* and without revocation.
*/
CHERIBSDTEST_VERIFY2(errno == ENOMEM,
"mmap after free failed with %d instead of ENOMEM", errno);
#else
CHERIBSDTEST_VERIFY2(errno == EPROT,
"mmap after free failed with %d instead of EPROT", errno);
if (reservations_are_quarantined()) {
/*
* There's nothing to cause the quarantined reservation to be
* revoked between the munmap and mmap calls so we'll get an
* ENOMEM here.
*
* XXX: ideally we'd trigger a revocation of this specific
* reservation before the mmap call to test the same case with
* and without revocation.
*/
CHERIBSDTEST_VERIFY2(errno == ENOMEM,
"mmap after free failed with %d instead of ENOMEM", errno);
} else
#endif
CHERIBSDTEST_VERIFY2(errno == EPROT,
"mmap after free failed with %d instead of EPROT", errno);

cheribsdtest_success();
}
Expand Down Expand Up @@ -2042,7 +2069,8 @@ CHERIBSDTEST(cheri_revoke_lib_fork,
}

CHERIBSDTEST(revoke_largest_quarantined_reservation,
"Verify that the largest quarantined reservation is revoked")
"Verify that the largest quarantined reservation is revoked",
.ct_check_skip = skip_need_quarantine_unmapped_reservations)
{
const size_t res_size = 0x100000000;
void * __capability res;
Expand Down Expand Up @@ -2119,7 +2147,8 @@ CHERIBSDTEST(revoke_largest_quarantined_reservation,

#define NRES 3
CHERIBSDTEST(revoke_merge_quarantined,
"Verify that adjacent non-neighbor reservations are revoked")
"Verify that adjacent non-neighbor reservations are revoked",
.ct_check_skip = skip_need_quarantine_unmapped_reservations)
{
const size_t big_res_size = 0x100000000;
const size_t res_sizes[NRES] =
Expand Down
21 changes: 15 additions & 6 deletions sys/vm/vm_map.c
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,12 @@ static int vm_map_clip_start(vm_map_t map, vm_map_entry_t entry,
}

#ifdef CHERI_CAPREVOKE
SYSCTL_DECL(_vm_cheri_revoke);
static bool quarantine_unmapped_reservations = true;
SYSCTL_BOOL(_vm_cheri_revoke, OID_AUTO, quarantine_unmapped_reservations,
CTLFLAG_RDTUN, &quarantine_unmapped_reservations, true,
"Quarantine and revoke fully unmapped reservations");

/*
* Sort by size and then reservation.
*/
Expand Down Expand Up @@ -3670,11 +3676,13 @@ vm_map_inherit(vm_map_t map, vm_offset_t start, vm_offset_t end,
goto unlock;
}
#ifdef CHERI_CAPREVOKE
for (entry = start_entry; entry->start < end;
prev_entry = entry, entry = vm_map_entry_succ(entry)) {
if (entry->inheritance == VM_INHERIT_QUARANTINE) {
rv = KERN_PROTECTION_FAILURE;
goto unlock;
if (quarantine_unmapped_reservations) {
for (entry = start_entry; entry->start < end;
prev_entry = entry, entry = vm_map_entry_succ(entry)) {
if (entry->inheritance == VM_INHERIT_QUARANTINE) {
rv = KERN_PROTECTION_FAILURE;
goto unlock;
}
}
}
#endif
Expand Down Expand Up @@ -4710,7 +4718,8 @@ vm_map_remove_locked(vm_map_t map, vm_offset_t start, vm_offset_t end)
result = vm_map_delete(map, start, end, true);
if (vm_map_reservation_is_unmapped(map, reservation)) {
#ifdef CHERI_CAPREVOKE
if (start < VM_MAXUSER_ADDRESS) {
if (quarantine_unmapped_reservations &&
start < VM_MAXUSER_ADDRESS) {
/*
* If we're here, [start, end) was the last mapped
* range in reservation and it has transitioned to
Expand Down

0 comments on commit 8fd603e

Please sign in to comment.