Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [Critical] In `pvr_set_uobj_array`, the pointer increments for the source 
(kernel) and destination (userspace) are swapped when `obj_size != 
out->stride`. This allows an out-of-bounds kernel memory read and memory 
corruption in userspace.
--

commit da1445e748ce26b12e26b7056dc2b26080c7fd58
Author: Brajesh Gupta <[email protected]>

drm/imagination: Fix returned size for DRM_IOCTL_PVR_DEV_QUERY

This patch modifies how the driver overrides the returned size for subtypes
of DRM_IOCTL_PVR_DEV_QUERY. It caps the reported size so it doesn't exceed
the query structure size when the arguments specify a smaller size.

> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c 
> b/drivers/gpu/drm/imagination/pvr_drv.c
> index ca3042d14253b..ee86c74df0d7a 100644
> --- a/drivers/gpu/drm/imagination/pvr_drv.c
> +++ b/drivers/gpu/drm/imagination/pvr_drv.c

[ ... ]

> diff --git a/drivers/gpu/drm/imagination/pvr_vm.c 
> b/drivers/gpu/drm/imagination/pvr_vm.c
> index e1ec60f34b6e6..396d349fb6ce4 100644
> --- a/drivers/gpu/drm/imagination/pvr_vm.c
> +++ b/drivers/gpu/drm/imagination/pvr_vm.c
> @@ -1019,7 +1019,8 @@ pvr_static_data_areas_get(const struct pvr_device 
> *pvr_dev,
>       if (err < 0)
>               return err;
>  
> -     args->size = sizeof(query);
> +     if (args->size > sizeof(query))
> +             args->size = sizeof(query);

[Severity: Critical]
This is a pre-existing issue, but I noticed a potential out-of-bounds kernel
memory read when this function calls PVR_UOBJ_SET_ARRAY() earlier.

In drivers/gpu/drm/imagination/pvr_drv.c:pvr_set_uobj_array(), the pointer
increments for the source and destination appear to be swapped when
obj_size != out->stride:

        void __user *out_ptr = u64_to_user_ptr(out->array);
        const void *in_ptr = in;

        for (u32 i = 0; i < out->count; i++) {
                if (copy_to_user(out_ptr, in_ptr, cpy_elem_size))
                        return -EFAULT;

                out_ptr += obj_size;
                in_ptr += out->stride;
        }

Does this incorrectly advance the kernel-space source pointer (in_ptr) by the
userspace-controlled out->stride?

This could allow an unprivileged user to read past the bounds of kernel
arrays by providing a large stride in the ioctl.

Should out_ptr increment by out->stride and in_ptr increment by obj_size?

>       return 0;
>  }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to