| Message ID | 1463065021-18280-5-git-send-email-cpaul@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 12, 2016 at 10:57:01AM -0400, Lyude wrote: > Unfortunately since we don't have Dave's connector refcounting patch > here yet, it's very possible that drm_atomic_state_default_clear() could > get called by intel_display_resume() when > intel_dp_mst_destroy_connector() isn't completely finished destroying an > mst connector, but has already finished setting connector->funcs to > NULL. As such, we need to treat the connector like it's already been > destroyed and just skip it, otherwise we'll end up dereferencing a NULL > pointer. > > This fix is only required for 4.6 and below. David Airlie's patchseries > for 4.7 to add connector reference counting provides a more proper fix > for this. > > Upstream fix: b164d31f50b2923a7a92c2a40cb46973a6ba8c36 > Cc: stable@vger.kernel.org > Signed-off-by: Lyude <cpaul@redhat.com> Not fixing the race at all, bug if it helps a few users in real-world cases while the real bugfix trickles down into shipping kernels (it'll be in 4.7 but just way too big for backporting) I'm ok with this. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> (but for stable kernels only) > --- > drivers/gpu/drm/drm_atomic.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 8ee1db8..d3a5b5c 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -139,7 +139,7 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) > for (i = 0; i < state->num_connector; i++) { > struct drm_connector *connector = state->connectors[i]; > > - if (!connector) > + if (!connector || !connector->funcs) > continue; > > /* > @@ -150,6 +150,7 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) > * case by setting all connector pointers to NULL. > */ > state->connector_states[i]->connector = NULL; > + > connector->funcs->atomic_destroy_state(NULL, > state->connector_states[i]); > state->connectors[i] = NULL; > -- > 2.5.5 >
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 8ee1db8..d3a5b5c 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -139,7 +139,7 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) for (i = 0; i < state->num_connector; i++) { struct drm_connector *connector = state->connectors[i]; - if (!connector) + if (!connector || !connector->funcs) continue; /* @@ -150,6 +150,7 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state) * case by setting all connector pointers to NULL. */ state->connector_states[i]->connector = NULL; + connector->funcs->atomic_destroy_state(NULL, state->connector_states[i]); state->connectors[i] = NULL;
Unfortunately since we don't have Dave's connector refcounting patch here yet, it's very possible that drm_atomic_state_default_clear() could get called by intel_display_resume() when intel_dp_mst_destroy_connector() isn't completely finished destroying an mst connector, but has already finished setting connector->funcs to NULL. As such, we need to treat the connector like it's already been destroyed and just skip it, otherwise we'll end up dereferencing a NULL pointer. This fix is only required for 4.6 and below. David Airlie's patchseries for 4.7 to add connector reference counting provides a more proper fix for this. Upstream fix: b164d31f50b2923a7a92c2a40cb46973a6ba8c36 Cc: stable@vger.kernel.org Signed-off-by: Lyude <cpaul@redhat.com> --- drivers/gpu/drm/drm_atomic.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)