Re: [PATCH v3 3/4] drm/msm: Validate lazy VM in GEM_NEW

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] The check `if (!vm)` fails to reject shared VMs because `msm_context_vm()` returns the shared VM on older targets.

Pre-existing issues:
- [Critical] Checking `if (sz > SIZE_MAX)` after assigning `struct_size()` to a u64 variable fails to detect overflow on 64-bit systems, leading to a heap buffer overflow.
--

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

drm/msm: Validate lazy VM in GEM_NEW
    
This commit fixes a potential NULL pointer dereference by validating that
a lazy VM exists before creating a _NO_SHARE buffer object.
    
Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: b58e12a66e47 ("drm/msm: Add _NO_SHARE flag")

> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index 2cb3ab04f1250..b083790dba042 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -1148,7 +1148,14 @@ int msm_gem_new_handle(struct drm_device *dev, struct drm_file *file,
>  
>  	if (flags & MSM_BO_NO_SHARE) {
>  		struct msm_context *ctx = file->driver_priv;
> -		struct drm_gem_object *r_obj = drm_gpuvm_resv_obj(ctx->vm);
> +		struct drm_gpuvm *vm = msm_context_vm(dev, ctx);
> +
> +		if (!vm) {
                ^^^^

[Severity: Medium]
Does this check properly reject shared VMs?

When per-process page tables are unsupported, msm_context_vm() falls
back to returning the global shared VM (gpu->vm). Since the returned
vm is not NULL, this check is bypassed, allowing NO_SHARE buffers to
be created on the shared VM.

Should this check also ensure that the returned VM is not the global
shared VM?

> +			ret = UERR(EINVAL, dev, "not supported with shared VM");
> +			goto out_put;
> +		}
> +
> +		struct drm_gem_object *r_obj = drm_gpuvm_resv_obj(vm);

[ ... ]

While reviewing this, I noticed a pre-existing issue in the msm submit code:

drivers/gpu/drm/msm/msm_gem_submit.c:submit_create() {
	...
	sz = struct_size(submit, bos, nr_bos) +
			((u64)nr_cmds * sizeof(submit->cmd[0]));

	if (sz > SIZE_MAX)
		return ERR_PTR(-ENOMEM);
	...
}

[Severity: Critical]
This is a pre-existing issue, but does this sz check work on 64-bit systems?

Since sz is a u64, sz > SIZE_MAX is impossible on 64-bit platforms where
SIZE_MAX is U64_MAX. If nr_bos is excessively large, struct_size() returns
SIZE_MAX, and adding the cmd size wraps sz around to a small positive
value.

The check is bypassed, leading to a small buffer allocation. Could this
allow out-of-bounds writes in submit_lookup_objects() when it loops up
to nr_bos?

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