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 <[email protected]> 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 <[email protected]> Signed-off-by: Rob Clark <[email protected]> > 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/[email protected]?part=5
