Hi Jeongjun,
On 02/06/26 12:41, Jeongjun Park wrote:
Hi Maíra,
Maíra Canal <[email protected]> wrote:
Hi Jeongjun,
On 25/05/26 11:04, Jeongjun Park wrote:
The CPU submit ioctl checks cpu_job->job_type before the CPU job has been
initialized with v3d_job_init(). When no CPU job extension is supplied,
the check fails and the ioctl goes to the common error path.
That path calls v3d_job_cleanup(), which expects a fully initialized
v3d_job. However at this point the CPU job has only been allocated,
so the embedded DRM scheduler job state has not been initialized yet.
Initialize the CPU job after parsing extensions, but before validating
the CPU job type and BO count. Keep pre-initialization failures on a
Considering that this is a minimal fix for stable, could you explain me
why do you believe moving v3d_job_init() is the best solution?
Because according to the implementation of v3d_submit_cpu_ioctl(),
v3d_job_cleanup() should never be called when v3d_job_init() fails to
execute successfully, but it is currently being called.
Therefore, I called the init function before the job_type check and added
the missing v3d_job_deallocate(), which should be called when
v3d_get_extensions() fails. However, upon re-analysis, it appears that
adding the fail_init label is unnecessary now, and I have discovered
another serious issue.
If v3d_job_init() fails after v3d_get_extensions() succeeds, a memory leak
may occur in the memory allocated by v3d_get_extensions(). In such cases,
v3d_job_cleanup() cannot be called, and consequently, v3d_cpu_job_free()
cannot be used either. Therefore, it seems necessary to add a separate
helper function to handle this case.
I will quickly write a v2 patch reflecting these changes.
I wouldn't worry a lot about this particular issue, as I already
addressed it in [1].
[1]
https://lore.kernel.org/dri-devel/[email protected]/T/
Best regards,
- Maíra
Best regards,
- Maíra
Regards,
Jeongjun Park
