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 <[email protected]> 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 <[email protected]> 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/[email protected]?part=3
