[4/4] drm/atomic: Verify connector->funcs != NULL when clearing states

Submitted by cpaul@redhat.com on May 12, 2016, 2:57 p.m.

Details

Message ID 1463065021-18280-5-git-send-email-cpaul@redhat.com
State New
Headers show
Series "Important MST fixes for 4.6" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

cpaul@redhat.com May 12, 2016, 2:57 p.m.
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(-)

Patch hide | download patch | download mbox

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;

Comments

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
>