[PATCH] drm/imagination: Fix user array stride in pvr_set_uobj_array()

[PATCH] drm/imagination: Fix user array stride in pvr_set_uobj_array()

[shuvampandey1]

pvr_set_uobj_array() copies an array of kernel objects to a userspace
array whose element size is described by out->stride. When out->stride
is different from the kernel object size, the slow path advances the
userspace pointer by the kernel object size and the kernel pointer by the
userspace stride.

This reverses the intended layout. For larger userspace strides, later
copies read from the wrong kernel addresses. For smaller userspace
strides, later copies are written at the wrong userspace offsets. The
padding clear is also done only for the first element instead of the
padding area for each element.

Advance the userspace pointer by out->stride and the kernel pointer by
obj_size, and clear per-element padding while the current userspace
pointer is still available.

Fixes: f99f5f3ea7ef ("drm/imagination: Add GPU ID parsing and firmware loading")
Cc: stable@vger.kernel.org # v6.8+
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
 drivers/gpu/drm/imagination/pvr_drv.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
index 268900464ab6..0a68a9c32361 100644
--- a/drivers/gpu/drm/imagination/pvr_drv.c
+++ b/drivers/gpu/drm/imagination/pvr_drv.c
@@ -1252,14 +1252,13 @@ pvr_set_uobj_array(const struct drm_pvr_obj_array *out, u32 min_stride, u32 obj_
 			if (copy_to_user(out_ptr, in_ptr, cpy_elem_size))
 				return -EFAULT;
 
-			out_ptr += obj_size;
-			in_ptr += out->stride;
-		}
+			if (out->stride > obj_size &&
+			    clear_user(out_ptr + cpy_elem_size, out->stride - obj_size)) {
+				return -EFAULT;
+			}
 
-		if (out->stride > obj_size &&
-		    clear_user(u64_to_user_ptr(out->array + obj_size),
-			       out->stride - obj_size)) {
-			return -EFAULT;
+			out_ptr += out->stride;
+			in_ptr += obj_size;
 		}
 	}
 

Re: [PATCH] drm/imagination: Fix user array stride in pvr_set_uobj_array()

[sashiko-bot]

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

Pre-existing issues:
- [High] Unbounded unaccounted memory allocations in job submission paths based on user-controlled counts.
--

commit 6abaeaa93e812673bb771d67a15b960baf454dc7
Author: shuvampandey1@gmail.com <shuvampandey1@gmail.com>

drm/imagination: Fix user array stride in pvr_set_uobj_array()

This commit corrects the pointer arithmetic and padding clearing in
pvr_set_uobj_array to ensure correct element copies. It advances the
userspace pointer by the stride and the kernel pointer by the object size.

Cc: stable@vger.kernel.org # v6.8+

> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
> index b20c462bcba0b..58e24f133bec7 100644
> --- a/drivers/gpu/drm/imagination/pvr_drv.c
> +++ b/drivers/gpu/drm/imagination/pvr_drv.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but while reviewing this subsystem I noticed a
potential problem with unbounded allocations in the job submission paths.

In drivers/gpu/drm/imagination/pvr_job.c:pvr_submit_jobs():

    err = PVR_UOBJ_GET_ARRAY(job_args, &args->jobs);
    ...
    job_data = kvmalloc_objs(*job_data, args->jobs.count, GFP_KERNEL | __GFP_ZERO);

And similarly in drivers/gpu/drm/imagination/pvr_job.c:pvr_job_data_init():

    err = PVR_UOBJ_GET_ARRAY(job_data_out[i].sync_ops, &job_args[i].sync_ops);

Since args->jobs.count and sync_ops.count come directly from userspace, can a
malicious local user with access to the DRM render node issue a submit jobs
ioctl with an arbitrarily large count?

Because these allocations use GFP_KERNEL without the __GFP_ACCOUNT flag,
__GFP_NORETRY, or an upper bound validation, could a massive allocation
request bypass memory cgroup limits and trigger the global OOM killer,
leading to a system-wide denial of service?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178155996993.4848.8618351576278880213@gmail.com?part=1