Re: [PATCH v5 04/10] drm/msm/a6xx: Access VM directly in submit path

[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 uninitialized `ctx->vm` during early ioctl validation causes a NULL pointer dereference.
--

--- Patch [4]: [PATCH v5 04/10] drm/msm/a6xx: Access VM directly in submit path ---
commit 006ce0b84643f1999004dcef9a0a6a167c16efae
Author: Rob Clark <robin.clark@oss.qualcomm.com>

drm/msm/a6xx: Access VM directly in submit path

This commit changes the a6xx_set_pagetable() function to access the
context VM directly (ctx->vm) instead of calling msm_context_vm(). It
relies on the assumption that the GEM_SUBMIT ioctl has already ensured
the VM is created by the time this code is reached.

> drm/msm/a6xx: Access VM directly in submit path
> 
> The GEM_SUBMIT ioctl has already ensured that the VM is created, so we
> aren't expecting to lazily create the VM this deep into the ioctl.

[Severity: Critical]
This is a pre-existing issue, but does the GEM_SUBMIT ioctl actually ensure
the VM is created?

Looking at msm_ioctl_gem_submit() in drivers/gpu/drm/msm/msm_gem_submit.c,
the early validation path accesses the VM directly without calling
msm_context_vm() to ensure it has been created:

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");
    ...
}

Can unprivileged userspace trigger a NULL pointer dereference by calling the
MSM_GEM_SUBMIT ioctl on a fresh DRM device descriptor where the context VM
has not been lazily initialized yet?

> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 8b3bb2fd433ba..7f1673163f671 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -229,7 +229,7 @@ static void a6xx_set_pagetable(struct a6xx_gpu *a6xx_gpu,
>  {
>  	bool sysprof = msm_gpu_sysprof_no_perfcntr_zap(&a6xx_gpu->base.base);
>  	struct msm_context *ctx = submit->queue->ctx;
> -	struct drm_gpuvm *vm = msm_context_vm(submit->dev, ctx);
> +	struct drm_gpuvm *vm = ctx->vm;
>  	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
>  	phys_addr_t ttbr;
>  	u32 asid;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716201538.365824-1-robin.clark@oss.qualcomm.com?part=4