vm_map reservations interface #913

qwattash · 2021-01-27T19:54:52Z

Introduce purecap-kernel reservation changes.

brooksdavis

incomplete review, but need to quit for the day. Will pick up at superpages tomorrow.

sys/vm/vm_map.c

sys/vm/vm_map.h

sys/vm/vm_mmap.c

sys/vm/vm_unix.c

sys/kern/kern_exec.c

sys/kern/imgact_elf.c

sys/vm/vm_kern.c

jrtc27 · 2021-01-28T14:04:46Z

sys/kern/sysv_shm.c

@@ -561,22 +565,29 @@ kern_shmat_locked(struct thread *td, int shmid,
 	max_va = 0;
 #endif
 	vm_object_reference(shmseg->object);
-	rv = vm_map_find(&p->p_vmspace->vm_map, shmseg->object, 0, &attach_va,
+	attach_addr = attach_va;


I don't understand this diff, attach_va isn't used again so why do we need to introduce a copy?

It was done to differentiate the variable holding the hint and the one holding the actual capability, it doesn't add anything to the logic though, I'll take it out and reduce the diff

shm requires some more work for reservations anyway, but this is probably not the PR for all of it.

sys/kern/sysv_shm.c

jrtc27 · 2021-01-28T14:07:32Z

sys/kern/uipc_shm.c

-			goto fail;
-		next_entry = vm_map_entry_succ(prev_entry);
-		if (next_entry->start < *addr + size)
+		/* MAP_FIXED */


The loss of a separate MAP_EXCL case concerns me; do we have kyua tests to cover the upstream behaviour that we can run to verify this doesn't break things?

This has now changed. I found kyua tests that were not being ran that check this behaviour and fixed them. There is one that seems to be failing in dev as well and I have not debugged it thoroughly. The rest has been checked on amd64, I need to check on riscv as it has superpages but seemingly support for non-transparent large pages is disabled.
Edit: RISCV pmap is missing handling of the PMAP_ENTER_LARGEPAGE flag for pmap_enter() from vm_fault. I believe this is the reason we do not have full non-transparent large page support there.

jrtc27 · 2021-01-28T14:08:35Z

sys/sys/mman.h

@@ -104,11 +104,11 @@
 #define	MAP_EXCL	 0x00004000 /* for MAP_FIXED, fail if address is used */
 #define	MAP_NOCORE	 0x00020000 /* dont include these pages in a coredump */
 #define	MAP_PREFAULT_READ 0x00040000 /* prefault mapping for reading */
-#ifdef __LP64__
+#if defined(__LP64__) || defined(__CHERI_PURE_CAPABILITY__)


Unrelated. But also wrong for RV32.

sys/vm/vm_kern.c

sys/vm/vm_map.c

brooksdavis · 2021-01-28T19:24:02Z

sys/vm/vm_map.c

+	 * XXX-AM: we need to recreate the capability for the
+	 * stack reservation. This is hacky, we should have
+	 * a way to recover it. Maybe save it as a capability
+	 * in the map entry?


I wonder if @trasz has any thoughts on this. Right now, a "main" stack is extremely integral to maps that apply to single processes, but it's certainly more complicated in colocated processes.

I think that we may end up having a more complex structure to represent a reservation, instead of a plain vaddr of the start of the reservation. This will allow us to have extra metadata in there, which we also would need to record the original reservation maxprot, depending on how we handle things in vm_map_delete().

sys/kern/kern_exec.c

sys/kern/imgact_elf.c

sys/kern/kern_exec.c

Switch to using a local scratch variable instead of assiging temporary values to the output parameter.

- Change the vm_map_delete() interface, this is now for internal use only and vm_map_remove() should be used instead. - Add vm_map_clear() to explicitly discard all mappings in a given map. - Add vm_map_reservation_create, vm_map_reservation_delete and their _locked variants, these enforce reservation representability. - Drop the MAP_CREATE_UNMAPPED vm_map flag. - Export vm_map_alignspace(). - Move vm_map_prot2perms() from vm_mmap to vm_map. - Update mmap syscall handlers to use new reservations interface.

This is currently required as vm_map is exported via user.h

Create reservations for the shared page and stack.

sys/kern/imgact_elf.c

brooksdavis

I think this is basically ready to land. I did point out one style issue (that there are a couple instances of), but I'm not a stickler for it and don't feel it's a blocker.

It looks like we've drained all substantive kernel changes out of morello-dev so I think it's ok to land this.

sys/kern/uipc_shm.c

arichardson · 2021-02-10T10:03:39Z

sys/kern/imgact_elf.c

 	if (error != 0)
 		goto ret;

+	/* Round star/end addresses to representability */
+	imgp->start_addr = CHERI_REPRESENTABLE_BASE(representable_start,


I wonder if we should just use the reservation capability here? We can then pass that directly to userspace after an appropriate candperm. Also means we no longer need the local diff that adds both start+end addr and insteadd we just store the mapping cap.

But that can obviously be done as a follow-up cleanup and doesn't need to be part of this PR.

I have thought about doing that, but have not attempted it. Probably we should land this first as you say. We should do the same for interp_start

sys/kern/imgact_elf.c

arichardson

imgact changes LGTM now. I've only skimmed the other bits but what I've looked at seems reasonable.

- Add capability for the image reservation in the image_params struct. - Update aout imgact to create a reservation for the whole image. - Update ELF imgact to create a reservation for the whole image and then map sections within it. - Replace all uses of vaddr_t with ptraddr_t. - Properly update rbase in ELF imgact to match the start of the reservation. - Remove the representable_start/end parameters from ELF imgact load_sections and related functions as the values can now safely be derived by rounding to representable boundaries.

This will enforce representability in the kernel allocations.

…m_mmap. Introduce vm_map_reservation_get helper to fetch the reservation ID of the reservation spanning spanning a requested range, if one and only one exists.

sys/kern/kern_exec.c

- Add root capability for each vm_map. All reservations will be created from this capability. - Allow software permission bits on root map capability so that those can be propagated down to mmap(). - Use vm_pointer_t where vm_offset_t represents a pointer. - Add assertions to check for capability invariants. This allows to propagate sowftware permission bits from the userspace capability down through mmap().

- Use vm_pointer_t for pointers instead of vm_offset_t - Ensure that the purecap kernel vm_map_fixed() will always be given a valid capability, both for purecap and hybrid userland.

… kernel. Cast to uintptr_t via uintcap_t instead.

- Use vm_pointer_t where vm_offset_t is used to hold a pointer. - Properly setup the stack capability for purecap.

… kmem

…in shm

qwattash force-pushed the dev-purecap-kernel-reservations branch from a713983 to c674803 Compare January 27, 2021 23:43

brooksdavis reviewed Jan 28, 2021

View reviewed changes

qwattash added the purecap-kernel label Jan 28, 2021

qwattash force-pushed the dev-purecap-kernel-reservations branch from c674803 to 813eead Compare January 28, 2021 13:48

jrtc27 reviewed Jan 28, 2021

View reviewed changes

sys/kern/sysv_shm.c Outdated Show resolved Hide resolved

jrtc27 reviewed Jan 28, 2021

View reviewed changes

sys/vm/vm_kern.c Show resolved Hide resolved

jrtc27 reviewed Jan 28, 2021

View reviewed changes

sys/vm/vm_map.c Outdated Show resolved Hide resolved

jrtc27 reviewed Jan 28, 2021

View reviewed changes

sys/vm/vm_map.c Show resolved Hide resolved

brooksdavis reviewed Jan 28, 2021

View reviewed changes

sys/vm/vm_map.c Outdated Show resolved Hide resolved

brooksdavis reviewed Jan 28, 2021

View reviewed changes

arichardson reviewed Feb 1, 2021

View reviewed changes

sys/kern/kern_exec.c Outdated Show resolved Hide resolved

sys/kern/imgact_elf.c Show resolved Hide resolved

sys/kern/kern_exec.c Outdated Show resolved Hide resolved

qwattash force-pushed the dev-purecap-kernel-reservations branch 4 times, most recently from 124fde1 to b4dedc2 Compare February 6, 2021 14:57

qwattash added 4 commits February 8, 2021 01:18

Cleanup use of of output addr parameter in vm_map_find.

584de98

Switch to using a local scratch variable instead of assiging temporary values to the output parameter.

Expose vm_pointer_t via user.h

d285720

This is currently required as vm_map is exported via user.h

Use new reservations KPI for exec_new_vmspace().

48f7d9e

Create reservations for the shared page and stack.

qwattash force-pushed the dev-purecap-kernel-reservations branch from b4dedc2 to 8b9a8cf Compare February 9, 2021 00:12

qwattash commented Feb 9, 2021

View reviewed changes

sys/kern/imgact_elf.c Outdated Show resolved Hide resolved

qwattash force-pushed the dev-purecap-kernel-reservations branch from 8b9a8cf to 9e95fc4 Compare February 9, 2021 01:12

arichardson reviewed Feb 9, 2021

View reviewed changes

sys/kern/imgact_elf.c Show resolved Hide resolved

arichardson reviewed Feb 9, 2021

View reviewed changes

sys/kern/imgact_elf.c Outdated Show resolved Hide resolved

qwattash force-pushed the dev-purecap-kernel-reservations branch from 9e95fc4 to 4b21ef6 Compare February 9, 2021 17:50

brooksdavis approved these changes Feb 9, 2021

View reviewed changes

sys/kern/uipc_shm.c Outdated Show resolved Hide resolved

arichardson reviewed Feb 10, 2021

View reviewed changes

sys/kern/imgact_elf.c Outdated Show resolved Hide resolved

arichardson reviewed Feb 10, 2021

View reviewed changes

qwattash added 5 commits February 10, 2021 16:29

Update kmap allocation functions to use reservations.

209a592

This will enforce representability in the kernel allocations.

Create a reservation for the UMA bootmem.

5d5ad9c

Use new reservation KPI in shmat.

0232bde

Use new reservation KPI for userspace non-trasparent superpages in sh…

d529ea3

…m_mmap. Introduce vm_map_reservation_get helper to fetch the reservation ID of the reservation spanning spanning a requested range, if one and only one exists.

qwattash force-pushed the dev-purecap-kernel-reservations branch from 4b21ef6 to 1f1974f Compare February 10, 2021 16:35

arichardson reviewed Feb 10, 2021

View reviewed changes

sys/kern/kern_exec.c Outdated Show resolved Hide resolved

qwattash added 8 commits February 11, 2021 22:49

purecap-kernel: Update mmap() for purecap kernel.

d593255

- Use vm_pointer_t for pointers instead of vm_offset_t - Ensure that the purecap kernel vm_map_fixed() will always be given a valid capability, both for purecap and hybrid userland.

purecap-kernel: Drop __cheri_addr casts incompatible with the purecap…

5b9f208

… kernel. Cast to uintptr_t via uintcap_t instead.

purecap-kernel: Update kernel exec

be33908

- Use vm_pointer_t where vm_offset_t is used to hold a pointer. - Properly setup the stack capability for purecap.

purecap-kernel: Create reservation for kernel image when initializing…

1fc4873

… kmem

purecap-kernel: Add assertion to make sure capabilities are sensible …

ca58a5a

…in shm

purecap-kernel: Use vm_pointer_t for shm_mmap

768c35d

Set CHERIABI_VMMAP permission bit on the MIPS root_userspace_cap.

29e4600

qwattash force-pushed the dev-purecap-kernel-reservations branch from 1f1974f to 29e4600 Compare February 11, 2021 22:54

qwattash merged commit ec1e66a into dev Feb 12, 2021

qwattash deleted the dev-purecap-kernel-reservations branch February 12, 2021 12:06

jrtc27 mentioned this pull request May 20, 2023

Can not run clang on cheribsd-riscv64-purecap #1700

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vm_map reservations interface #913

vm_map reservations interface #913

qwattash commented Jan 27, 2021

brooksdavis left a comment

jrtc27 Jan 28, 2021

qwattash Feb 1, 2021

qwattash Feb 1, 2021

jrtc27 Jan 28, 2021

qwattash Feb 5, 2021 •

edited

Loading

jrtc27 Jan 28, 2021

brooksdavis Jan 28, 2021

qwattash Feb 2, 2021

brooksdavis left a comment

arichardson Feb 10, 2021

qwattash Feb 10, 2021 •

edited

Loading

arichardson left a comment

vm_map reservations interface #913

vm_map reservations interface #913

Conversation

qwattash commented Jan 27, 2021

brooksdavis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qwattash Feb 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brooksdavis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qwattash Feb 10, 2021 • edited Loading

Choose a reason for hiding this comment

arichardson left a comment

Choose a reason for hiding this comment

qwattash Feb 5, 2021 •

edited

Loading

qwattash Feb 10, 2021 •

edited

Loading