On Sat, Aug 15, 2026 at 11:40 AM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
> to consider:
>
> Pre-existing issues:
> - [Critical] The patch fixes a NULL pointer dereference in the `GETPARAM` 
> ioctl when the graphics engine (`gr`) fails to initialize but misses an 
> identical issue in `nouveau_abi16_ioctl_get_zcull_info`.
> --
>
> --- Patch [1]: [PATCH 1/2] drm/nouveau: Fix NULL pointer dereferences in 
> GETPARAM ioctl ---
> commit 6569813c05cdc1cf39cfd469b6745e81e7cad86d
> Author: Jim Cromie <[email protected]>
>
> drm/nouveau: Fix NULL pointer dereferences in GETPARAM ioctl
>
> This patch adds safety checks for the graphics engine and device functions
> in the GETPARAM ioctl handler. It returns -ENODEV to signal missing
> hardware state to userspace, preventing a kernel NULL pointer dereference
> when initialization fails.
>
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c 
> > b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> > index 291203121f0c2..c9270c5b0fac8 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> > @@ -306,7 +306,12 @@ nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
> >               getparam->value = 1;
> >               break;
> >       case NOUVEAU_GETPARAM_GRAPH_UNITS:
> > -             getparam->value = nvkm_gr_units(gr);
> > +             if (gr && gr->func) {
> > +                     getparam->value = nvkm_gr_units(gr);
> > +             } else {
> > +                     NV_ERROR_ONCE(drm, "GETPARAM_GRAPH_UNITS: no gr 
> > engine or func\n");
> > +                     return -ENODEV;
> > +             }
> >               break;
>
> [Severity: Critical]
> This is a pre-existing issue, but while you are fixing the NULL pointer
> dereference of the gr pointer here, is there a similar vulnerability
> in the adjacent nouveau_abi16_ioctl_get_zcull_info() function?
>
> If the graphics engine fails to initialize and gr is NULL, as addressed by
> this patch, it looks like an unprivileged userspace process could still
> trigger an Oops by calling the DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO ioctl,
> which only requires DRM_RENDER_ALLOW.
>
> Looking at the nouveau_abi16_ioctl_get_zcull_info() function in
> drivers/gpu/drm/nouveau/nouveau_abi16.c, the gr pointer is unconditionally
> dereferenced:
>
> int
> nouveau_abi16_ioctl_get_zcull_info(ABI16_IOCTL_ARGS)
> {
>         struct nouveau_drm *drm = nouveau_drm(dev);
>         struct nvkm_gr *gr = nvxx_gr(drm);
>         struct drm_nouveau_get_zcull_info *out = data;
>
>         if (gr->has_zcull_info) {
>                 const struct nvkm_gr_zcull_info *i = &gr->zcull_info;
>                 ...
>
> Would it make sense to add a similar check for gr being NULL before
> dereferencing gr->has_zcull_info?

yes indeed, the next patch does so.
I hit them both, separately, but theyre mergable if you like.


>
> [ ... ]
>
> --
> Sashiko AI review · 
> https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to