Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions arch/riscv/include/asm/csr.h
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,13 @@
#define CSR_STVAL2 0x14b
#define CSR_SATP 0x180

/*
* Page-fault encoding in stval2/mtval2. Note this is an enum, not a bitfield.
*/
#define TVAL2_PF_RISCV 0 /* RISC-V page fault only */
#define TVAL2_PF_CHERI 1 /* CHERI PTE fault only */
#define TVAL2_PF_RISCV_CHERI 2 /* both */

#define CSR_STIMECMP 0x14D
#define CSR_STIMECMPH 0x15D

Expand Down
7 changes: 7 additions & 0 deletions arch/riscv/include/asm/user_ptr.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ user_ptr_perms_t arch_user_ptr_owning_perms_from_prot(int prot, unsigned long vm
if ((prot & PROT_READ) && (vm_flags & VM_READ_CAPS))
perms |= CHERI_PERM_MUTABLE_LOAD;

if (prot & PROT_NO_CAP) {
if (prot & PROT_READ)
perms |= CHERI_PERMS_LOAD_CAP;
if (prot & PROT_WRITE)
perms |= CHERI_PERMS_STORE_CAP;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but this and the related text in the commit message is bogus. I somewhat see why you are doing this but we really shouldn't add additional permissions to the capability with an explicitly given PROT_NO_CAP. Yes, you will get a CHERI fault instead of a capability fault but I don't think we care. If this fails tests adjust the tests.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that it is even more bogus than that if the CW bit is not supported, either because the hardware does not have it or because for some reason the arch decided to not make use of it.


if (prot & PROT_EXEC) {
if (cheri_perms_get(regs->epc) & CHERI_PERM_SYSTEM_REGS)
perms |= CHERI_PERM_SYSTEM_REGS;
Expand Down
49 changes: 32 additions & 17 deletions arch/riscv/mm/fault.c
Original file line number Diff line number Diff line change
Expand Up @@ -246,29 +246,33 @@ static inline void vmalloc_fault(struct pt_regs *regs, int code, unsigned long a
local_flush_tlb_page(addr);
}

static inline bool access_error(unsigned long cause, struct vm_area_struct *vma)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is quite right. First, you are introducing quite a large diff in a function with subtle logic. This is bound to introduce merge errors when the upstream code changes.
But more importantly, you are now saying that a cheri PTE fault on a VMA with VM_CAP_WRITE is ok. That will trigger standard fault handling which is then trying to fix an ordinary page fault and will fail to do so. So I think that all cheri PTE faults should be bad_area_nosemaphore. This also simpifies things because you can just handle those before even calling access_ok().

/*
* Returns 0 if the access is permitted, otherwise the SIGSEGV si_code to
* deliver.
*/
static inline int access_error(unsigned long cause, struct vm_area_struct *vma,
bool cheri_pte_fault)
{
switch (cause) {
case EXC_INST_PAGE_FAULT:
if (!(vma->vm_flags & VM_EXEC)) {
return true;
}
if (!(vma->vm_flags & VM_EXEC))
return SEGV_ACCERR;
break;
case EXC_LOAD_PAGE_FAULT:
/* Write implies read */
if (!(vma->vm_flags & (VM_READ | VM_WRITE))) {
return true;
}
if (!(vma->vm_flags & (VM_READ | VM_WRITE)))
return SEGV_ACCERR;
break;
case EXC_STORE_PAGE_FAULT:
if (!(vma->vm_flags & VM_WRITE)) {
return true;
}
if (!(vma->vm_flags & VM_WRITE))
return SEGV_ACCERR;
if (cheri_pte_fault && !(vma->vm_flags & VM_WRITE_CAPS))
return SEGV_STORETAG;
break;
default:
panic("%s: unhandled cause %lu", __func__, cause);
}
return false;
return 0;
}

/*
Expand All @@ -283,11 +287,22 @@ void handle_page_fault(struct pt_regs *regs)
unsigned long addr, cause;
unsigned int flags = FAULT_FLAG_DEFAULT;
int code = SEGV_MAPERR;
int access_code;
vm_fault_t fault;
bool cheri_pte_fault = false;

cause = regs->cause;
addr = regs->badaddr;

#ifdef CONFIG_RISCV_CHERI
if (cause == EXC_STORE_PAGE_FAULT || cause == EXC_LOAD_PAGE_FAULT) {
unsigned long t2 = csr_read(CSR_TVAL2);

cheri_pte_fault = (t2 == TVAL2_PF_CHERI ||
t2 == TVAL2_PF_RISCV_CHERI);
}
#endif

tsk = current;
mm = tsk->mm;

Expand Down Expand Up @@ -351,11 +366,12 @@ void handle_page_fault(struct pt_regs *regs)
if (!vma)
goto lock_mmap;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above: Handle cheri_pte_faults here and always do an access error.

if (unlikely(access_error(cause, vma))) {
access_code = access_error(cause, vma, cheri_pte_fault);
if (unlikely(access_code)) {
vma_end_read(vma);
count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
tsk->thread.bad_cause = cause;
bad_area_nosemaphore(regs, SEGV_ACCERR, addr);
bad_area_nosemaphore(regs, access_code, addr);
return;
}

Expand Down Expand Up @@ -390,11 +406,10 @@ void handle_page_fault(struct pt_regs *regs)
* Ok, we have a good vm_area for this memory access, so
* we can handle it.
*/
code = SEGV_ACCERR;

if (unlikely(access_error(cause, vma))) {
access_code = access_error(cause, vma, cheri_pte_fault);
if (unlikely(access_code)) {
tsk->thread.bad_cause = cause;
bad_area(regs, mm, code, addr);
bad_area(regs, mm, access_code, addr);
return;
}

Expand Down
2 changes: 2 additions & 0 deletions include/uapi/asm-generic/mman-common.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

/* PCuABI mapping and capability permissions */
#define PROT_CAP_INVOKE 0x2000 /* mmap flag: provide CInvoke capability permission */
#define PROT_CAP 0x4000 /* region may hold valid capability tags (tag stores preserved) */

Check failure on line 22 in include/uapi/asm-generic/mman-common.h

View workflow job for this annotation

GitHub Actions / checkpatch review

WARNING: line length of 106 exceeds 100 columns
#define PROT_NO_CAP 0x8000 /* region may not hold valid capability tags (tag store fails with a signal) */

Check failure on line 23 in include/uapi/asm-generic/mman-common.h

View workflow job for this annotation

GitHub Actions / checkpatch review

WARNING: line length of 119 exceeds 100 columns

#define _PROT_MAX_SHIFT 16
#define PROT_MAX(prot) ((prot) << _PROT_MAX_SHIFT)
Expand Down
3 changes: 2 additions & 1 deletion include/uapi/asm-generic/siginfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,8 @@ typedef struct siginfo {
#define SEGV_CAPBOUNDSERR 12 /* Capability bounds fault */
#define SEGV_CAPPERMERR 13 /* Capability permission fault */
#define SEGV_CAPACCESSERR 14 /* Capability access fault */
#define NSIGSEGV 14
#define SEGV_STORETAG 15 /* Capability tag store fault */
#define NSIGSEGV 15
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the CI run: This breaks build on other archs.


/*
* SIGBUS si_codes
Expand Down
17 changes: 17 additions & 0 deletions mm/mmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -404,9 +404,26 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
* to. we assume access permissions have been handled by the open
* of the memory object, so we don't do any here.
*/
if (IS_ENABLED(CONFIG_CHERI_PURECAP_UABI)) {
if ((prot & PROT_CAP) && (prot & PROT_NO_CAP))
return -EINVAL;
/*
* PROT_CAP is not supported with file-backed MAP_SHARED mapping
*/
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this? Isn't it the whole point to allow just that?

if ((prot & PROT_CAP) && file && (flags & MAP_SHARED))
return -EINVAL;
}

vm_flags |= calc_vm_prot_bits(prot, pkey) | calc_vm_flag_bits(file, flags) |
mm->def_flags | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC;

if (IS_ENABLED(CONFIG_CHERI_PURECAP_UABI)) {
if (prot & PROT_CAP)
vm_flags |= VM_READ_CAPS | VM_WRITE_CAPS;
else if (prot & PROT_NO_CAP)
vm_flags &= ~(VM_READ_CAPS | VM_WRITE_CAPS);
}

/* Obtain the address to map to. we verify (or select) it and ensure
* that it represents a valid section of the address space.
*/
Expand Down
2 changes: 1 addition & 1 deletion tools/testing/selftests/riscv/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
ARCH ?= $(shell uname -m 2>/dev/null || echo not)

ifneq (,$(filter $(ARCH),riscv))
RISCV_SUBTARGETS ?= abi hwprobe mm sigreturn vector cfi
RISCV_SUBTARGETS ?= abi hwprobe mm sigreturn vector cfi cheri
else
RISCV_SUBTARGETS :=
endif
Expand Down
10 changes: 10 additions & 0 deletions tools/testing/selftests/riscv/cheri/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# SPDX-License-Identifier: GPL-2.0

CFLAGS += -std=gnu99 -I.

TEST_GEN_PROGS := mmap_prot_cap

include ../../lib.mk

$(OUTPUT)/mmap_prot_cap: mmap_prot_cap.c
$(CC) -o$@ $(CFLAGS) $(LDFLAGS) $^
Loading
Loading