Re: [PATCH 6/6] drm/msm: Fix task_struct reference leak in recover_worker

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Direct access to ctx->vm without lazy initialization via msm_context_vm() causes a NULL pointer dereference if ioctls are called before the VM is created.
--

commit 9d77c7114e9006cacfe1802a7ff8d83c9c1a6bac
Author: Jie Zhang <jie.zhang@oss.qualcomm.com>

drm/msm: Fix task_struct reference leak in recover_worker

This commit fixes a missing put_task_struct() in the else branch of
recover_worker. This resolves a task struct reference leak that occurred
on every GPU hang recovery.

> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 48ac51f4119be..03c0578560658 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
[ ... ]
> @@ -505,6 +505,8 @@ static void recover_worker(struct kthread_work *work)
>  		 */
>  		if (!vm->managed)
>  			msm_gem_vm_unusable(submit->vm);
> +
> +		put_task_struct(task);
>  	}
>  
>  	noreclaim_flag = memalloc_noreclaim_save();

[Severity: Critical]
This is a pre-existing issue, but while reviewing context VM usage in this
driver, I noticed that direct access to ctx->vm without lazy initialization
occurs in a few places.

In drivers/gpu/drm/msm/msm_gem_submit.c:msm_ioctl_gem_submit():

	if (to_msm_vm(ctx->vm)->unusable)
		return UERR(EPIPE, dev, "context is unusable");

If a userspace process opens the DRM device and immediately calls the
submit ioctl before the VM is lazily created, won't ctx->vm be NULL here?
Since base is the first element of struct msm_gem_vm, to_msm_vm(NULL)
evaluates to NULL, which would cause an immediate NULL pointer dereference
when accessing ->unusable.

Similarly, in drivers/gpu/drm/msm/msm_gem.c:msm_gem_new_handle():

		struct drm_gem_object *r_obj = drm_gpuvm_resv_obj(ctx->vm);

Could this also trigger a crash if ctx->vm is NULL, since passing NULL to
drm_gpuvm_resv_obj() expands to dereference ctx->vm directly? Should these
paths be using msm_context_vm() to ensure the VM is initialized first?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605-assorted-fixes-june-v1-0-2caa04f7287c@oss.qualcomm.com?part=6