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? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
