On Wed, May 02, 2018 at 09:32:47PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Clear the old_state and new_state pointers for every object in
> drm_atomic_state_default_clear(). Otherwise
> drm_atomic_get_{new,old}_*_state() will hand out stale pointers to
> anyone who hasn't first confirmed that the object is in fact part of
> the current atomic transcation, if they are called after we've done
> the ww backoff dance while hanging on to the same drm_atomic_state.
>
> For example, handle_conflicting_encoders() looks like it could hit
> this since it iterates the full connector list and just calls
> drm_atomic_get_new_connector_state() for each.
>
> And I believe we have now witnessed this happening at least once in
> i915 check_digital_port_conflicts(). Commit 8b69449d2663 ("drm/i915:
> Remove last references to drm_atomic_get_existing* macros") changed
> the safe drm_atomic_get_existing_connector_state() to the unsafe
> drm_atomic_get_new_connector_state(), which opened the doors for
> this particular bug there as well.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Cc: Abhay Kumar <abhay.kumar@xxxxxxxxx>
> Fixes: 581e49fe6b41 ("drm/atomic: Add new iterators over all state, v3.")
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Uh ... that's some bad oversight :-/
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
btw for stable we might want to split this into 1 patch for core objects
and 1 patch for driver private stuff. Feel free to do so while applying
and keep my r-b. The fixes line for the 2nd patch would be:
Fixes: a4370c777406 ("drm/atomic: Make private objs proper objects")
Cc: <stable@xxxxxxxxxxxxxxx> # v4.14+
Cheers, Daniel
> ---
> drivers/gpu/drm/drm_atomic.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 7d25c42f22db..c825c76edc1d 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -155,6 +155,8 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
> state->connectors[i].state);
> state->connectors[i].ptr = NULL;
> state->connectors[i].state = NULL;
> + state->connectors[i].old_state = NULL;
> + state->connectors[i].new_state = NULL;
> drm_connector_put(connector);
> }
>
> @@ -169,6 +171,8 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
>
> state->crtcs[i].ptr = NULL;
> state->crtcs[i].state = NULL;
> + state->crtcs[i].old_state = NULL;
> + state->crtcs[i].new_state = NULL;
> }
>
> for (i = 0; i < config->num_total_plane; i++) {
> @@ -181,6 +185,8 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
> state->planes[i].state);
> state->planes[i].ptr = NULL;
> state->planes[i].state = NULL;
> + state->planes[i].old_state = NULL;
> + state->planes[i].new_state = NULL;
> }
>
> for (i = 0; i < state->num_private_objs; i++) {
> @@ -190,6 +196,8 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
> state->private_objs[i].state);
> state->private_objs[i].ptr = NULL;
> state->private_objs[i].state = NULL;
> + state->private_objs[i].old_state = NULL;
> + state->private_objs[i].new_state = NULL;
> }
> state->num_private_objs = 0;
>
> --
> 2.16.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
