Re: [PATCH v2 1/3] drm/msm: Fix barriers accessing ctx vm

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Direct access to uninitialized `ctx->vm` in `adreno_set_param` bypasses validation, allowing `VM_BIND` on global VMs.
--

commit be0b70b2e7a9f55182d48578fb53358469060fcb
Author: Rob Clark <robin.clark@oss.qualcomm.com>

drm/msm: Fix barriers accessing ctx vm

This commit adds load-acquire and store-release barriers when accessing
and lazily initializing ctx->vm in msm_context_vm(). This prevents callers
from seeing a partially initialized virtual memory state.

> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 32d5ebea2596f..ec88155e0ed72 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -224,18 +224,19 @@ struct drm_gpuvm *msm_context_vm(struct drm_device *dev, struct msm_context *ctx
>  {
>  	static DEFINE_MUTEX(init_lock);
>  	struct msm_drm_private *priv = dev->dev_private;
> +	struct drm_gpuvm *vm = smp_load_acquire(&ctx->vm);
>  
>  	/* Once ctx->vm is created it is valid for the lifetime of the context: */
> -	if (ctx->vm)
> -		return ctx->vm;
> +	if (vm)
> +		return vm;

[Severity: High]
This isn't a bug introduced by this patch, but seeing the fixes to ctx->vm
lazy initialization here points to a related problem in adreno_set_param().

Looking at adreno_set_param() in drivers/gpu/drm/msm/adreno/adreno_gpu.c,
there is this check when enabling VM_BIND:

    if (ctx->vm == gpu->vm)
        return UERR(EINVAL, drm, "requires per-process pgtables");

Since ctx->vm starts as NULL and is lazily initialized, couldn't this
check fail to catch global VMs (since NULL != gpu->vm) if msm_context_vm()
hasn't been called yet?

If unprivileged userspace opts into VM_BIND mode before the VM is
initialized, could this bypass the validation check on platforms without
per-process pagetables, potentially allowing arbitrary mapping in the
global GPU address space? 

Should this be using msm_context_vm(dev, ctx) instead to ensure the
VM is instantiated before checking?

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