Re: [PATCH v4 3/3] accel/rocket: Propagate job submission errors

[sashiko-bot@kernel.org]

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

New issues:
- [High] The ioctl introduces ordered partial-submit semantics but fails to report how many jobs were successfully submitted, making error recovery impossible for userspace.

Pre-existing issues:
- [High] Unbounded memory allocation due to missing limit on args->job_count.
--

commit 41a2028e803565dfe6bad77987c577fafe68aee4
Author: MoGGuU <Naixumogu@whut.edu.cn>
accel/rocket: Propagate job submission errors

The patch updates rocket_ioctl_submit() to check the return value of each
job submission instead of discarding it. It now returns the first error
encountered and stops submitting the remaining jobs, providing ordered
partial-submit semantics.

> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index 1634c30ede456..cd2ae1def74d1 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -647,8 +647,11 @@ int rocket_ioctl_submit(struct drm_device *dev, void *data, struct drm_file *fil
>  	}
>  
>  

[Severity: High]
This is a pre-existing issue, but is there a missing limit on args->job_count
before the loop?

Looking at rocket_ioctl_submit(), there is an unbounded allocation based on
this user-controlled parameter earlier in the function:

	jobs = kvmalloc_objs(*jobs, args->job_count);
	if (!jobs) {
		drm_dbg(dev, "Failed to allocate incoming job array\n");
		return -ENOMEM;
	}

Could an untrusted user supply a maliciously large args->job_count to cause
system memory exhaustion and invoke the OOM killer?

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

[Severity: High]
Does this code fail to report how many jobs were successfully submitted when
it aborts partially through?

When a job fails partway through the submission loop, the ioctl aborts and
propagates the error, leaving any previously queued jobs in place (ordered
partial-submit semantics). But without returning the progress state (e.g.,
which index failed) to userspace, doesn't this create an unrecoverable
job state ambiguity? If userspace retries, they might duplicate execution
of the already-submitted jobs.

> +	}
>  
>  exit:
>  	kvfree(jobs);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828061916.20405-1-Naixumogu@whut.edu.cn?part=3