On 11/03/2024 10:28, Amit Daniel Kachhap wrote:
Use the recently introduced PCuABI reservation interfaces to add different parameter constraints for mmap/munmap syscall. The capability returned by mmap syscalls is now bounded and is same as the reservation range. These reservation checks do not affect the compat64 code path.
Signed-off-by: Amit Daniel Kachhap amitdaniel.kachhap@arm.com
mm/mmap.c | 41 ++++++++++++++++++++++++++++++++++++++--- mm/util.c | 3 --- 2 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/mm/mmap.c b/mm/mmap.c index 3b072e822f99..fd25fa7c9cda 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1401,6 +1401,13 @@ user_uintptr_t do_mmap(struct file *file, user_uintptr_t user_ptr, ((vm_flags & VM_LOCKED) || (flags & (MAP_POPULATE | MAP_NONBLOCK)) == MAP_POPULATE)) *populate = len;
- if (!IS_ERR_VALUE(addr)) {
if (new_reserv && reserv_is_supported(mm))user_ptr = make_user_ptr_owning(addr, len,user_ptr_perms_from_prot(prot,(flags & MAP_SHARED) ? false : true));return user_ptr;- }
return addr; } @@ -1410,7 +1417,9 @@ user_uintptr_t ksys_mmap_pgoff(user_uintptr_t user_ptr, unsigned long len, unsigned long fd, unsigned long pgoff) { struct file *file = NULL;
- user_uintptr_t retval;
- user_uintptr_t retval = -EINVAL;
- ptraddr_t addr = (ptraddr_t)user_ptr;
- VMA_ITERATOR(vmi, current->mm, addr);
if (!(flags & MAP_ANONYMOUS)) { audit_mmap_fd(fd, flags); @@ -1442,6 +1451,26 @@ user_uintptr_t ksys_mmap_pgoff(user_uintptr_t user_ptr, unsigned long len, if (IS_ERR(file)) return PTR_ERR(file); }
- if (!reserv_is_supported(current->mm))
goto skip_pcuabi_checks;- if (user_ptr_is_valid((const void __user *)user_ptr)) {
We don't really need a separate tag check. We can directly use check_user_ptr_owning() as a "selector" (if it returns false, then we check that the capability is null-derived, like we do now in the else). This works because the error code is always -EINVAL when we don't like the capability metadata.
if (!(flags & MAP_FIXED) || !check_user_ptr_owning(user_ptr, addr, len))goto out_fput;if (!reserv_vmi_cap_within_reserv(&vmi, user_ptr, false)) {retval = -ERESERVATION;goto out_fput;}if (!reserv_vmi_range_mapped(&vmi, addr, len, false)) {retval = -ENOMEM;goto out_fput;}- } else {
Nit: could be an else if.
if (!user_ptr_is_same((const void __user *)user_ptr,(const void __user *)(user_uintptr_t)addr))goto out_fput;- }
+skip_pcuabi_checks:
Maybe all this could be a separate function to avoid the extra goto? In fact, I suspect that function could be used by mremap too, as memap is functionally equivalent to mmap in terms of those capability/reservation checks.
Kevin
retval = vm_mmap_pgoff(file, user_ptr, len, prot, flags, pgoff); out_fput: @@ -3120,9 +3149,15 @@ int vm_munmap(unsigned long start, size_t len) } EXPORT_SYMBOL(vm_munmap); -SYSCALL_DEFINE2(munmap, user_uintptr_t, addr, size_t, len) +SYSCALL_DEFINE2(munmap, user_uintptr_t, user_ptr, size_t, len) {
- addr = untagged_addr(addr);
- ptraddr_t addr = untagged_addr((ptraddr_t)user_ptr);
- VMA_ITERATOR(vmi, current->mm, addr);
- if (!check_user_ptr_owning(user_ptr, addr, len))
return -EINVAL;- if (!reserv_vmi_cap_within_reserv(&vmi, user_ptr, false))
return -ERESERVATION;} diff --git a/mm/util.c b/mm/util.c index 077c9a2592a9..b3e8175fefc2 100644 --- a/mm/util.c +++ b/mm/util.c @@ -559,9 +559,6 @@ user_uintptr_t vm_mmap_pgoff(struct file *file, user_uintptr_t usrptr, userfaultfd_unmap_complete(mm, &uf); if (populate) mm_populate(ret, populate);
/* TODO [PCuABI] - derive proper capability */if (!IS_ERR_VALUE(ret))ret = (user_uintptr_t)uaddr_to_user_ptr_safe((ptraddr_t)ret);}