Re: [PATCH v6 1/2] accel/rocket: Check allocations before use

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Silent failure when submitting multiple jobs due to ignored return value.
- [High] Violation of DRM scheduler contract by calling drm_sched_job_cleanup() on an armed job.
--

commit e8a7bda4ea0e8bc90689dd01aad3165b7c5004b2
Author: Triet Hoang <triet.hoang.dev@gmail.com>

accel/rocket: Check allocations before use

Check the result of kmalloc_objs() in rocket_job_open() before using the
allocated buffers. It also updates the error handling when
drm_sched_entity_init() fails by replacing WARN_ON(ret) with a plain if.

> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index f404355058185..ffa0f8db5da65 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -516,6 +516,9 @@ int rocket_job_open(struct rocket_file_priv *rocket_priv)
>  	unsigned int core;
>  	int ret;
>  
> +	if (!scheds)
> +		return -ENOMEM;
> +

[ ... ]

While reviewing this code, I noticed a couple of pre-existing issues in this
file.

[Severity: High]
This is a pre-existing issue, but does calling drm_sched_job_arm() before
rocket_acquire_object_fences() violate the DRM scheduler contract if
resource acquisition fails?

In rocket_job_push():

	scoped_guard(mutex, &rdev->sched_lock) {
		drm_sched_job_arm(&job->base);

		job->inference_done_fence =
			dma_fence_get(&job->base.s_fence->finished);

		ret = rocket_acquire_object_fences(job->in_bos,
						   job->in_bo_count,
						   &job->base, false);
		if (ret)
			goto err_unlock;

If rocket_acquire_object_fences() returns an error (e.g., due to -ENOMEM in
dma_resv_reserve_fences), the error path aborts the armed job by calling
drm_sched_job_cleanup() back in rocket_ioctl_submit_job().

Since drm_sched_job_arm() initializes fences and assigns a sequence number,
aborting it afterward leaks the assigned sequence number and can corrupt the
scheduler's fence ordering, leading to hangs or crashes in the DRM scheduler
core.


[Severity: High]
This is a pre-existing issue, but is it intentional that the return value of
rocket_ioctl_submit_job() is ignored?

In rocket_ioctl_submit():

	for (i = 0; i < args->job_count; i++)
		rocket_ioctl_submit_job(dev, file, &jobs[i]);

exit:
	kvfree(jobs);

	return ret;

If rocket_ioctl_submit_job() fails (e.g., due to an invalid BO handle or
-ENOMEM), the failure is silently swallowed and not propagated.

The ioctl will return success even when jobs were not queued, which can cause
userspace clients waiting on implicit fences for these jobs to hang
indefinitely.

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