On 09/08/2023 12:07, Luca Vizzarro wrote:
With PCuABI we want to make sure that any userspace capability is correctly validated.
In this commit the fault_in_safe_writeable function of the GUP is
I'd remove "of the GUP", it's not clear what it means (this is not really a get_user_pages kind of function).
attempting to fault in the pointed user page for writing purposes. In order to do so pointer arithmetic is performed, discarding of
Pointer arithmetic is not a problem. The fact that fixup_user_fault() only considers the address and ignores the rest is.
any capability metadata. Therefore, it is necessary to perform an explicit capability check to make sure that the user page we want to fault in should have write permissions and it falls between the
We need to be very careful not to confuse MMU permissions and capability permissions. The check we are adding is unrelated to whether the page is writeable via the user mapping or not. Rather we are checking that the pointer the caller provided has appropriate bounds and permissions to write to that address range.
expected bounds.
This also requires to change the signature of the function by dropping the const qualifier to the pointer argument. This is
It's not so obvious why, we should say check_user_ptr_write() expects a non-const pointer.
Kevin
acceptable given that the expectation is for the pointed region of memory to be writeable.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com
include/linux/pagemap.h | 2 +- mm/gup.c | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index a56308a9d1a4..a77203ae3686 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -1094,7 +1094,7 @@ void folio_add_wait_queue(struct folio *folio, wait_queue_entry_t *waiter); */ size_t fault_in_writeable(char __user *uaddr, size_t size); size_t fault_in_subpage_writeable(char __user *uaddr, size_t size); -size_t fault_in_safe_writeable(const char __user *uaddr, size_t size); +size_t fault_in_safe_writeable(char __user *uaddr, size_t size); size_t fault_in_readable(const char __user *uaddr, size_t size); int add_to_page_cache_lru(struct page *page, struct address_space *mapping, diff --git a/mm/gup.c b/mm/gup.c index 8dc517c42ea8..dcfe02390eea 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -1809,13 +1809,15 @@ EXPORT_SYMBOL(fault_in_subpage_writeable);
- Returns the number of bytes not faulted in, like copy_to_user() and
- copy_from_user().
*/ -size_t fault_in_safe_writeable(const char __user *uaddr, size_t size) +size_t fault_in_safe_writeable(char __user *uaddr, size_t size) {
- /* TODO [PCuABI] - capability checks for uaccess */ unsigned long start = user_ptr_addr(uaddr), end; struct mm_struct *mm = current->mm; bool unlocked = false;
- if (unlikely(!check_user_ptr_write(uaddr, size)))
return 0;- if (unlikely(size == 0)) return 0; end = PAGE_ALIGN(start + size);