Re: [PATCH v4 1/6] drm/msm: Fix barriers accessing ctx vm

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Silent fallback to the global GPU VM when private VM allocation fails breaks memory isolation.
- [High] Direct access to ctx->vm before initialization causes a NULL pointer dereference in IOCTL handlers.
- [High] Missing smp_load_acquire() when reading ctx->vm in msm_gem_close() can lead to accessing partially initialized memory.
--

--- Patch [1]: [PATCH v4 1/6] drm/msm: Fix barriers accessing ctx vm ---
commit 5827c45dcfd83b1ca9d1ff0ea3f3c026fba67fb3
Author: Rob Clark <robin.clark@oss.qualcomm.com>

drm/msm: Fix barriers accessing ctx vm

This commit fixes memory barrier issues when accessing the context VM
by introducing smp_load_acquire and smp_store_release in msm_context_vm.
It prevents callers from seeing a partially initialized virtual machine.

> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 79f78e7e16e9d..3c8b5083a1fbe 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -225,18 +225,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;
> +
> +	guard(mutex)(&init_lock);
>  
> -	mutex_lock(&init_lock);
>  	if (!ctx->vm) {
> -		ctx->vm = msm_gpu_create_private_vm(
> +		vm = msm_gpu_create_private_vm(
>  			priv->gpu, current, !ctx->userspace_managed_vm);

[Severity: Critical]
This isn't a bug introduced by this patch, but looking at the VM creation
logic in msm_gpu_create_private_vm() called above, does it safely handle
memory allocation failures?

drivers/gpu/drm/msm/msm_gpu.c:msm_gpu_create_private_vm() {
    ...
    if (gpu->funcs->create_private_vm) {
        vm = gpu->funcs->create_private_vm(gpu, kernel_managed);
        ...
    }

    if (IS_ERR_OR_NULL(vm))
        vm = drm_gpuvm_get(gpu->vm);
    
    return vm;
}

If the private VM allocation fails (returning ERR_PTR(-ENOMEM) due to
system memory exhaustion), this fallback grants the userspace context access
to the shared global GPU VM.

Could an attacker intentionally exhaust memory to force this fallback,
thereby bypassing private VM isolation and gaining unauthorized access to
structures stored in the global VM?

> -
> +		smp_store_release(&ctx->vm, vm);
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but since we are fixing the
barriers for ctx->vm initialization here, does msm_gem_close() also need an
smp_load_acquire() barrier?

Looking at msm_gem_close() in drivers/gpu/drm/msm/msm_gem.c, it reads
ctx->vm using a plain load:

drivers/gpu/drm/msm/msm_gem.c:msm_gem_close() {
    ...
    if (!ctx->vm)
        return;
    ...
}

If msm_gem_close() executes concurrently with VM creation, could it see a
non-NULL pointer without an acquire barrier and access partially
initialized memory?

[Severity: High]
This is a pre-existing issue, but I noticed there are paths that directly
access ctx->vm without calling msm_context_vm() first to ensure it is
created.

For example, in msm_ioctl_gem_submit():

drivers/gpu/drm/msm/msm_gem_submit.c:msm_ioctl_gem_submit() {
    ...
    if (to_msm_vm(ctx->vm)->unusable)
        return -EPIPE;
    ...
}

And in msm_gem_new_handle():

drivers/gpu/drm/msm/msm_gem.c:msm_gem_new_handle() {
    ...
    struct drm_gem_object *r_obj = drm_gpuvm_resv_obj(ctx->vm);
    ...
}

If userspace calls either of these IOCTLs on a fresh context before the
VM is initialized, could ctx->vm be NULL here, leading to a NULL pointer
dereference when computing the struct offset or dereferencing it?

> -	mutex_unlock(&init_lock);
>  
>  	return ctx->vm;
>  }

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