Hi On Wed, 2025-12-31 at 13:50 +0800, Shenghao Yang wrote: > On disconnect drm_atomic_helper_disable_all() is called which > sets both the fb and crtc for a plane to NULL before invoking a commit. > > This causes a kernel oops on every display disconnect. > > Add guards for those dereferences. > > Cc: <[email protected]> # 6.18.x > Signed-off-by: Shenghao Yang <[email protected]> > --- > drivers/gpu/drm/gud/gud_pipe.c | 20 ++++++++------------ > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c > index 76d77a736d84..4b77be94348d 100644 > --- a/drivers/gpu/drm/gud/gud_pipe.c > +++ b/drivers/gpu/drm/gud/gud_pipe.c > @@ -457,27 +457,20 @@ int gud_plane_atomic_check(struct drm_plane *plane, > struct drm_plane_state *old_plane_state = > drm_atomic_get_old_plane_state(state, plane); > struct drm_plane_state *new_plane_state = > drm_atomic_get_new_plane_state(state, plane); > struct drm_crtc *crtc = new_plane_state->crtc; > - struct drm_crtc_state *crtc_state; > + struct drm_crtc_state *crtc_state = NULL; > const struct drm_display_mode *mode; > struct drm_framebuffer *old_fb = old_plane_state->fb; > struct drm_connector_state *connector_state = NULL; > struct drm_framebuffer *fb = new_plane_state->fb; > - const struct drm_format_info *format = fb->format; > + const struct drm_format_info *format; > struct drm_connector *connector; > unsigned int i, num_properties; > struct gud_state_req *req; > int idx, ret; > size_t len; > > - if (drm_WARN_ON_ONCE(plane->dev, !fb)) > - return -EINVAL; > - > - if (drm_WARN_ON_ONCE(plane->dev, !crtc)) > - return -EINVAL;
With the elimination of these two WARN_ON_ONCEs, it's possible that
crtc_state may not be assigned below, and therefore may be read/passed
to functions when it is NULL (e.g. line 488). Either protection for a
null crtc_state should be added to the rest of the function, or the
function shouldn't continue if crtc is NULL.
Ruben
> - crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> -
> - mode = &crtc_state->mode;
> + if (crtc)
> + crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>
> ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
> DRM_PLANE_NO_SCALING,
> @@ -492,6 +485,9 @@ int gud_plane_atomic_check(struct drm_plane *plane,
> if (old_plane_state->rotation != new_plane_state->rotation)
> crtc_state->mode_changed = true;
>
> + mode = &crtc_state->mode;
> + format = fb->format;
> +
> if (old_fb && old_fb->format != format)
> crtc_state->mode_changed = true;
>
> @@ -598,7 +594,7 @@ void gud_plane_atomic_update(struct drm_plane *plane,
> struct drm_atomic_helper_damage_iter iter;
> int ret, idx;
>
> - if (crtc->state->mode_changed || !crtc->state->enable) {
> + if (!crtc || crtc->state->mode_changed || !crtc->state->enable) {
> cancel_work_sync(&gdrm->work);
> mutex_lock(&gdrm->damage_lock);
> if (gdrm->fb) {
signature.asc
Description: This is a digitally signed message part
