Re: [PATCH v4 2/6] drm/msm: Block VM_BIND for shared VMs

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Incomplete fix: Falling back to the shared VM on private VM allocation failure allows EXECBUF isolation bypass, arbitrary kernel memory access, and information leaks.
- [High] Direct access to `ctx->vm` in `msm_ioctl_gem_submit()` before the VM is guaranteed to be initialized causes a NULL pointer dereference.
--

--- Patch [2]: [PATCH v4 2/6] drm/msm: Block VM_BIND for shared VMs ---
commit 14b0c06028a80fcfb812d65ea727ff988063c94e
Author: Rob Clark <robin.clark@oss.qualcomm.com>

drm/msm: Block VM_BIND for shared VMs

We already block MSM_PARAM_EN_VM_BIND where per-process VMs are not
supported.  But in error paths when we actually try to create the VM
(allocation failures, etc) we could still end up with a shared-VM.

Since we need to create the VM lazily, for backwards compat, there
isn't really a better place to return an error.  But this isn't to
happen in practice.  Just block VM_BIND ioctls in this case so we
aren't giving userspace a way to manage the shared VM.

> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> index 1a952b171ed7f..c88b4352ad751 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
[ ... ]
> @@ -1449,7 +1450,7 @@ msm_ioctl_vm_bind(struct drm_device *dev, void *data, struct drm_file *file)
>  	 * Maybe we could allow just UNMAP ops?  OTOH userspace should just
>  	 * immediately close the device file and all will be torn down.
>  	 */
> -	if (to_msm_vm(msm_context_vm(dev, ctx))->unusable)
> +	if (to_msm_vm(vm)->unusable)
>  		return UERR(EPIPE, dev, "context is unusable");
>  
>  	/*
> @@ -1457,7 +1458,7 @@ msm_ioctl_vm_bind(struct drm_device *dev, void *data, struct drm_file *file)
>  	 * place, if you haven't opted in to VM_BIND context.  But it is
>  	 * cleaner / less confusing, to check this case directly.
>  	 */
> -	if (!msm_context_is_vmbind(ctx))
> +	if (to_msm_vm(vm)->managed)

[Severity: Critical]
This is a pre-existing issue, but does falling back to the shared VM on private
VM allocation failure allow an execbuf isolation bypass?

While this patch blocks vm_bind ioctls for shared VMs, an attacker could still
opt into vm_bind (setting ctx->userspace_managed_vm = true) and intentionally
cause private VM allocation to fail. In this case, msm_gpu_create_private_vm()
falls back to the global kernel VM:

drivers/gpu/drm/msm/msm_gpu.c:msm_gpu_create_private_vm() {
	...
	vm = drm_gpuvm_get(gpu->vm);
	...
}

If the attacker then creates a normal submitqueue and calls
msm_ioctl_gem_submit(), command validation is skipped because
msm_context_is_vmbind(ctx) still returns true based on user intent rather
than actual VM state:

drivers/gpu/drm/msm/msm_gem_submit.c:msm_ioctl_gem_submit() {
	...
	cmds_to_parse = msm_context_is_vmbind(ctx) ? 0 : args->nr_cmds;
	...
}

This allows the unvalidated user-provided IOVA to be mapped and executed
within the global kernel VM:

drivers/gpu/drm/msm/msm_gem_submit.c:submit_lookup_cmds() {
	...
	submit->cmd[i].iova = submit_cmd.iova;
	...
}

Could this lead to arbitrary kernel memory access, or leak other processes'
data into the devcoredump if a job hangs?

[Severity: High]
This isn't a bug introduced by this patch, but does direct access to ctx->vm
in msm_ioctl_gem_submit() cause a null pointer dereference?

If a user directly calls msm_ioctl_gem_submit() on a fresh context without
performing any prior operation (like msm_ioctl_vm_bind) that initializes the
VM, ctx->vm remains null due to lazy initialization.

Does this check in msm_ioctl_gem_submit() cause an immediate crash when
dereferencing the offset to unusable?

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

>  		return UERR(EINVAL, dev, "context does not support vmbind");
>  
>  	if (args->flags & ~MSM_VM_BIND_FLAGS)

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