Re: [PATCH v7 05/17] drm/msm: Move nr_cmds initialization

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Direct access to lazy-initialized `ctx->vm` without ensuring it exists causes a NULL pointer dereference.
- [Critical] The overflow check `sz > SIZE_MAX` after using `struct_size()` is dead code on 64-bit architectures, providing no protection against integer overflows.
--

--- Patch [5]: [PATCH v7 05/17] drm/msm: Move nr_cmds initialization ---
commit cc137d18d60eae340c7d4959cae7bc281443d450
Author: Rob Clark <robin.clark@oss.qualcomm.com>

drm/msm: Move nr_cmds initialization

Previously if we entered an error path between these two points, we
could leak the relocs tables due to submit->nr_cmds still being zero.

In practice, relocs are disallowed on a6xx+, and non-ancient userspace
will not use relocs on earlier gens unless running on an ancient kernel.
But userspace could use this to trigger a memory leak.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>

> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 3c6bc90c3d486..aa48ea4e7f584 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c

[Severity: Critical]
This is a pre-existing issue, but does submit_create() have a dead code
overflow check?

In 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);

On 64-bit architectures, sz (a uint64_t) cannot exceed SIZE_MAX. If
struct_size() overflows, it returns SIZE_MAX, and adding to it wraps
around to a small value.

This bypasses the size check and causes an undersized allocation, which
could lead to a heap buffer overflow since nr_bos and nr_cmds are
user-controlled via the MSM_GEM_SUBMIT ioctl.

[Severity: Critical]
This is a pre-existing issue, but does msm_ioctl_gem_submit() dereference
an uninitialized pointer?

In drivers/gpu/drm/msm/msm_gem_submit.c:msm_ioctl_gem_submit():

	if (to_msm_vm(ctx->vm)->unusable)
		return UERR(EPIPE, dev, "context is unusable");

Since ctx->vm uses lazy initialization, it can be NULL if the DRM context
was just created.

Accessing ctx->vm without calling msm_context_vm() first can lead to a
NULL pointer dereference if a userspace application calls the
MSM_GEM_SUBMIT ioctl immediately after context creation.

[ ... ]

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