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

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

[Muhammad Bilal]

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

Claude review: accel/rocket: fix NULL dereference and integer overflow in rocket_job_push()

[Claude Code Review Bot]

Overall Series Review

Subject: accel/rocket: fix NULL dereference and integer overflow in rocket_job_push()
Author: Muhammad Bilal <meatuni001@gmail.com>
Patches: 1
Reviewed: 2026-05-25T17:04:16.545445

---

This is a single-patch fix for the `accel/rocket` driver addressing two real bugs in `rocket_job_push()`: a missing NULL check after `kvmalloc_array()` and a potential u32 overflow when summing userspace-controlled `in_bo_count` and `out_bo_count`. Both bugs are legitimate — `in_bo_count` and `out_bo_count` are directly assigned from userspace IOCTL fields (`job->in_bo_handle_count`, `job->out_bo_handle_count`) at `rocket_job.c:573` and `:580` with no bounds checking, so the overflow path is reachable.

The fix is correct, minimal, and well-targeted. The commit message is clear and accurately describes both issues. The `Fixes:` tag and `Cc: stable` are appropriate.

**Verdict: Patch looks good.** One minor nit below, but nothing blocking.

---
Generated by Claude Code Patch Reviewer

Claude review: accel/rocket: fix NULL dereference and integer overflow in rocket_job_push()

[Claude Code Review Bot]

Patch Review

**Overflow check — correct:**

```c
+	if (check_add_overflow(job->in_bo_count, job->out_bo_count, &bo_count))
+		return -EINVAL;
```

All three operands are `u32`, so `check_add_overflow()` will correctly detect wrap. Returning `-EINVAL` is appropriate for invalid userspace input.

**NULL check — correct:**

```c
+	bos = kvmalloc_array(bo_count, sizeof(*bos), GFP_KERNEL);
+	if (!bos)
+		return -ENOMEM;
```

This is the standard pattern. Also note the `sizeof(void *)` → `sizeof(*bos)` change is a good improvement (type-safe, follows kernel style).

**Sum replacement in callers — correct:**

The raw `job->in_bo_count + job->out_bo_count` expressions in `drm_gem_lock_reservations()` and `drm_gem_unlock_reservations()` are replaced with the pre-validated `bo_count`. This is both safer and cleaner.

**Minor nit (non-blocking):** The `memcpy` calls on lines 201–202 still use `sizeof(void *)` instead of `sizeof(*bos)`:

```c
	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 *));
```

Since the allocation was changed to `sizeof(*bos)`, it would be more consistent to use `sizeof(*bos)` in the `memcpy` sizes too. Both are equivalent (`bos` is `struct drm_gem_object **`, so `*bos` is a pointer, same size as `void *`), so this is purely cosmetic and not a correctness issue.

**Overall:** Clean, correct fix for two real bugs. Reviewed-by worthy.

---
Generated by Claude Code Patch Reviewer