Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Arbitrary kernel memory leak and local denial of service via swapped pointer increments in `pvr_set_uobj_array`. -- commit 6b5a278856eb4ef8ab480c543efc90923416bfbf Author: Brajesh Gupta <[email protected]> drm/imagination: Fix returned size for DRM_IOCTL_PVR_DEV_QUERY This commit updates DRM_IOCTL_PVR_DEV_QUERY to ensure the returned size does not exceed the size of the query structure. It prevents the driver from unconditionally overriding the size reported to userspace. > 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 since this function uses PVR_UOBJ_SET_ARRAY() to copy static_data_areas to userspace, could this lead to an out-of-bounds kernel memory leak? Looking at the underlying pvr_set_uobj_array() function in drivers/gpu/drm/imagination/pvr_drv.c, the pointer increments in the copy loop appear to be swapped: drivers/gpu/drm/imagination/pvr_drv.c:pvr_set_uobj_array() { ... 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; } ... } Here in_ptr (the kernel source) is incremented by out->stride (which is controlled by userspace), while out_ptr (the userspace destination) is incremented by obj_size (the kernel's structure size). If userspace provides an arbitrarily large stride via DRM_IOCTL_PVR_DEV_QUERY, would this cause in_ptr to read unmapped kernel memory or leak arbitrary kernel memory to userspace? Additionally, the padding clearance in that same function seems to be misplaced outside the loop: drivers/gpu/drm/imagination/pvr_drv.c:pvr_set_uobj_array() { ... if (out->stride > obj_size && clear_user(u64_to_user_ptr(out->array + obj_size), out->stride - obj_size)) { return -EFAULT; } ... } Does this leave the padding for subsequent array elements uncleared since it only runs once at the end? > return 0; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
