On Mon, 2026-07-20 at 22:06 +0000, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential > issue(s) to consider: > - [High] Unconditional dereference of `wndw->plane.state` which may > be NULL if the initial state allocation fails. > -- > > --- Patch [2]: [PATCH v3 2/3] drm/nouveau/kms/nv50-: Introduce > nv50_wndw_default_state() --- > commit 2d2dcbda94d37e6b985a1775b04d636840fd94c1 > Author: Lyude Paul <[email protected]> > > drm/nouveau/kms/nv50-: Introduce nv50_wndw_default_state() > > This commit introduces nv50_wndw_default_state() to ensure the > initial > atomic state for a window always starts with a supported value in > pixel_blend_mode. It hooks this function into > nv50_display_read_hw_state() > to enforce a valid blend mode value during driver startup. > > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c > > b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > > index 2635458d52acc..dfa1ba45acd99 100644 > > --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c > > +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c > > [ ... ] > > > +/* > > + * Setup defaults for the atomic wndw state > > + */ > > +void > > +nv50_wndw_default_state(struct nv50_wndw *wndw) > > +{ > > + struct nv50_wndw_atom *armw = nv50_wndw_atom(wndw- > > >plane.state); > > + const unsigned int blend_modes = wndw->func->blend_modes; > > + > > + drm_modeset_lock_assert_held(&wndw->plane.mutex); > > + > > + /* Ensure the plane's atomic state didn't default to a > > pixel_blend_mode we don't support */ > > + if (blend_modes && (!(BIT(armw->state.pixel_blend_mode) & > > blend_modes))) { > > [Severity: High] > Could this dereference a NULL pointer if memory allocation fails > during > drm_mode_config_reset()? > > If plane state allocation fails in the plane's reset hook, wndw- > >plane.state > is left as NULL. When nv50_display_read_hw_state() subsequently calls > this > function, armw will evaluate to NULL, leading to a fault when > accessing > armw->state.pixel_blend_mode.
there we go. I got worried when it had been a while since sashiko last hallucinated :). (there is absolutely no way we could be in the function described if that allocation failed, and additionally this function has nothing to do with drm_mode_config_reset()) > > > + if (blend_modes & BIT(DRM_MODE_BLEND_COVERAGE)) > > + armw->state.pixel_blend_mode = > > DRM_MODE_BLEND_COVERAGE; > > + else if (blend_modes & > > BIT(DRM_MODE_BLEND_PREMULTI)) > > + armw->state.pixel_blend_mode = > > DRM_MODE_BLEND_PREMULTI; > > + else if (blend_modes & > > BIT(DRM_MODE_BLEND_PIXEL_NONE)) > > + armw->state.pixel_blend_mode = > > DRM_MODE_BLEND_PIXEL_NONE; > > + } > > +}
