The drm_atomic_commit structure is freed through the drm_atomic_commit_put() function, that eventually calls drm_atomic_commit_default_clear() by default when there's no active users of that state.
It then iterates over all objects with a state, and will call the atomic_destroy_state callback on the state pointer. The state pointer is mostly used these days to point to which of the old or new state needs to be freed, depending on whether the state was committed or not. So it all makes sense. However, with the hardware state readout support approaching, we might have a state, with multiple objects in it, but no state to free because we want them to persist. In such a case, the state pointer is going to be NULL, and thus we'll end up with NULL pointer dereference. Test if the state pointer is non-NULL before calling atomic_destroy_state on it. Reviewed-by: Laurent Pinchart <[email protected]> Reviewed-by: Thomas Zimmermann <[email protected]> Signed-off-by: Maxime Ripard <[email protected]> --- drivers/gpu/drm/drm_atomic.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 6bae998dd36a..d43ea69c2c9c 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -291,12 +291,14 @@ void drm_atomic_commit_default_clear(struct drm_atomic_commit *state) struct drm_connector *connector = state->connectors[i].ptr; if (!connector) continue; - connector->funcs->atomic_destroy_state(connector, - state->connectors[i].state_to_destroy); + if (state->connectors[i].state_to_destroy) + connector->funcs->atomic_destroy_state(connector, + state->connectors[i].state_to_destroy); + state->connectors[i].ptr = NULL; state->connectors[i].state_to_destroy = NULL; state->connectors[i].old_state = NULL; state->connectors[i].new_state = NULL; drm_connector_put(connector); @@ -306,12 +308,13 @@ void drm_atomic_commit_default_clear(struct drm_atomic_commit *state) struct drm_crtc *crtc = state->crtcs[i].ptr; if (!crtc) continue; - crtc->funcs->atomic_destroy_state(crtc, - state->crtcs[i].state_to_destroy); + if (state->crtcs[i].state_to_destroy) + crtc->funcs->atomic_destroy_state(crtc, + state->crtcs[i].state_to_destroy); state->crtcs[i].ptr = NULL; state->crtcs[i].state_to_destroy = NULL; state->crtcs[i].old_state = NULL; state->crtcs[i].new_state = NULL; @@ -326,12 +329,14 @@ void drm_atomic_commit_default_clear(struct drm_atomic_commit *state) struct drm_plane *plane = state->planes[i].ptr; if (!plane) continue; - plane->funcs->atomic_destroy_state(plane, - state->planes[i].state_to_destroy); + if (state->planes[i].state_to_destroy) + plane->funcs->atomic_destroy_state(plane, + state->planes[i].state_to_destroy); + state->planes[i].ptr = NULL; state->planes[i].state_to_destroy = NULL; state->planes[i].old_state = NULL; state->planes[i].new_state = NULL; } @@ -354,12 +359,14 @@ void drm_atomic_commit_default_clear(struct drm_atomic_commit *state) struct drm_private_obj *obj = state->private_objs[i].ptr; if (!obj) continue; - obj->funcs->atomic_destroy_state(obj, - state->private_objs[i].state_to_destroy); + if (state->private_objs[i].state_to_destroy) + obj->funcs->atomic_destroy_state(obj, + state->private_objs[i].state_to_destroy); + state->private_objs[i].ptr = NULL; state->private_objs[i].state_to_destroy = NULL; state->private_objs[i].old_state = NULL; state->private_objs[i].new_state = NULL; } -- 2.54.0
