[PATCH] accel/rocket: fix NULL dereference and integer overflow in rocket_job_push()

Muhammad Bilal posted 1 patch 48 minutes ago
drivers/accel/rocket/rocket_job.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
[PATCH] accel/rocket: fix NULL dereference and integer overflow in rocket_job_push()
Posted by Muhammad Bilal 48 minutes ago
rocket_job_push() allocates a temporary array to hold all input and
output GEM object pointers:

    bos = kvmalloc_array(job->in_bo_count + job->out_bo_count,
                         sizeof(void *), GFP_KERNEL);
    memcpy(bos, job->in_bos, job->in_bo_count * sizeof(void *));
    memcpy(&bos[job->in_bo_count], job->out_bos, ...);

Two bugs exist:

1. Missing NULL check: if kvmalloc_array() fails, bos is NULL and
   the subsequent memcpy() dereferences it, causing a kernel NULL
   pointer dereference.

2. Integer overflow: in_bo_count and out_bo_count are both u32, set
   directly from userspace-supplied in_bo_handle_count and
   out_bo_handle_count with no prior validation. Their sum is computed
   in u32 arithmetic and can wrap to a smaller value, causing the
   allocation count passed to kvmalloc_array() to be smaller than
   intended. Subsequent uses still operate on the original counts when
   copying and locking objects, which may lead to out-of-bounds accesses
   on the temporary array.

Fix by using check_add_overflow() to detect count overflow before the
allocation, and adding a NULL check on the allocation result.

Fixes: 0810d5ad88a1 ("accel/rocket: Add job submission IOCTL")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/accel/rocket/rocket_job.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index ac51bff39833..71f64bf2bb7f 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -8,6 +8,7 @@
 #include <drm/drm_gem.h>
 #include <drm/rocket_accel.h>
 #include <linux/interrupt.h>
+#include <linux/overflow.h>
 #include <linux/iommu.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -188,14 +189,19 @@ static int rocket_job_push(struct rocket_job *job)
 	struct rocket_device *rdev = job->rdev;
 	struct drm_gem_object **bos;
 	struct ww_acquire_ctx acquire_ctx;
+	u32 bo_count;
 	int ret = 0;
 
-	bos = kvmalloc_array(job->in_bo_count + job->out_bo_count, sizeof(void *),
-			     GFP_KERNEL);
+	if (check_add_overflow(job->in_bo_count, job->out_bo_count, &bo_count))
+		return -EINVAL;
+
+	bos = kvmalloc_array(bo_count, sizeof(*bos), GFP_KERNEL);
+	if (!bos)
+		return -ENOMEM;
 	memcpy(bos, job->in_bos, job->in_bo_count * sizeof(void *));
 	memcpy(&bos[job->in_bo_count], job->out_bos, job->out_bo_count * sizeof(void *));
 
-	ret = drm_gem_lock_reservations(bos, job->in_bo_count + job->out_bo_count, &acquire_ctx);
+	ret = drm_gem_lock_reservations(bos, bo_count, &acquire_ctx);
 	if (ret)
 		goto err;
 
@@ -220,7 +226,7 @@ static int rocket_job_push(struct rocket_job *job)
 	rocket_attach_object_fences(job->out_bos, job->out_bo_count, job->inference_done_fence);
 
 err_unlock:
-	drm_gem_unlock_reservations(bos, job->in_bo_count + job->out_bo_count, &acquire_ctx);
+	drm_gem_unlock_reservations(bos, bo_count, &acquire_ctx);
 err:
 	kvfree(bos);
 
-- 
2.53.0