On 05/06/2023 17:46, Luca Vizzarro wrote:
Whenever a GUP call is made, a user address in the form of a 64-bit integer is used to lookup its corresponding page. When working in PCuABI, this means that the metadata of the user capability gets discarded, hence any access made by the GUP is not checked in hardware.
This commit introduces explicit capability checks whenever a call to the current mm through the GUP functions is made.
Signed-off-by: Luca Vizzarro Luca.Vizzarro@arm.com
drivers/usb/core/devio.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index cb37f6d2010a..30bb27a9f072 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1584,9 +1584,11 @@ find_memory_area(struct usb_dev_state *ps, const struct usbdevfs_urb *uurb) { struct usb_memory *usbm = NULL, *iter; unsigned long flags;
- /* TODO [PCuABI] - capability checks for uaccess */ unsigned long uurb_start = user_ptr_addr(uurb->buffer);
- if (!check_user_ptr_rw(uurb->buffer, uurb->buffer_length))
AFAICT the nature of the access (either read or write) is computed as is_in in proc_do_submiturb(). The simplest is probably to move the check there, next to access_ok() would be ideal.
I've also noticed that *uurb is not copied in a tag-preserving way in proc_submiturb(), so the tag check will fail if we don't fix it. copy_from_user_with_ptr() should be used to preserve tags. This should be a separate commit as in some cases uurb->buffer is passed directly to e.g. copy_from_user() so we would already get failures if we had capability-based uaccess.
Kevin
return ERR_PTR(-EFAULT);- spin_lock_irqsave(&ps->lock, flags); list_for_each_entry(iter, &ps->memory_list, memlist) { if (uurb_start >= iter->vm_start &&