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

Pre-existing issues:
- [High] NULL pointer dereference from direct access to `ctx->vm` before VM 
initialization.
--

--- Patch [6]: [PATCH v7 06/17] drm/msm: Remove redundant SIZE_MAX check ---
commit 704799112fb8f5dd4e2424f4cac69e1d4b373f9d
Author: Rob Clark <[email protected]>

drm/msm: Remove redundant SIZE_MAX check

This commit removes a redundant SIZE_MAX overflow check in submit_lookup_cmds().
Since kmalloc() will inherently fail and return NULL when passed SIZE_MAX,
the explicit check is unnecessary.

> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c 
> b/drivers/gpu/drm/msm/msm_gem_submit.c
> index aa48ea4e7f584..040354ee16d11 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -250,11 +250,6 @@ static int submit_lookup_cmds(struct msm_gem_submit 
> *submit,
>  
>               sz = array_size(submit_cmd.nr_relocs,
>                               sizeof(struct drm_msm_gem_submit_reloc));
> -             /* check for overflow: */
> -             if (sz == SIZE_MAX) {
> -                     ret = -ENOMEM;
> -                     goto out;
> -             }
>               submit->cmd[i].relocs = kmalloc(sz, GFP_KERNEL | __GFP_NOWARN);
>               if (!submit->cmd[i].relocs) {
>                       ret = -ENOMEM;

[Severity: High]
This is a pre-existing issue, but I noticed it while reviewing this file.
Can this code cause a NULL pointer dereference if a local user opens a DRM
context and immediately calls the GEM submit ioctl before any VM has been
created for that context?

Looking at msm_ioctl_gem_submit() in the same file:

drivers/gpu/drm/msm/msm_gem_submit.c:msm_ioctl_gem_submit() {
    ...
        if (args->pad)
                return -EINVAL;

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

The to_msm_vm() macro performs a container_of() calculation, which on a NULL
ctx->vm yields an invalid pointer offset. Dereferencing ->unusable on this
invalid pointer will cause a page fault.

Should this use msm_context_vm(dev, ctx) instead of accessing ctx->vm
directly to ensure the VM is properly initialized before checking its
unusable state?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=6

Reply via email to