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. > Best regards, > - Maíra > Regards, Jeongjun Park
