public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: fix use after free during eDP encoder destroying
@ 2014-12-12 15:57 Imre Deak
  2014-12-12 17:06 ` Ville Syrjälä
  2014-12-14  5:51 ` shuang.he
  0 siblings, 2 replies; 5+ messages in thread
From: Imre Deak @ 2014-12-12 15:57 UTC (permalink / raw)
  To: intel-gfx

After

commit a18c0af171bfb875012da26f23df051004726973
uthor: Thierry Reding <treding@nvidia.com>
Date:   Wed Dec 10 11:38:49 2014 +0100

    drm: Zero out DRM object memory upon cleanup

we will use the eDP encoder during destroying it. Fix this by calling
drm_encoder_cleanup() at a point when the encoder is not used any more.
This caused a NULL pointer dereference in pps_lock(), I can't see that
it caused any other problem.

All the other encoders seem to call drm_encoder_cleanup() at a safe
place.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3fc3296..0a55623 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4310,7 +4310,6 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 
 	drm_dp_aux_unregister(&intel_dp->aux);
 	intel_dp_mst_encoder_cleanup(intel_dig_port);
-	drm_encoder_cleanup(encoder);
 	if (is_edp(intel_dp)) {
 		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
 		/*
@@ -4326,6 +4325,7 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 			intel_dp->edp_notifier.notifier_call = NULL;
 		}
 	}
+	drm_encoder_cleanup(encoder);
 	kfree(intel_dig_port);
 }
 
-- 
1.8.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: fix use after free during eDP encoder destroying
  2014-12-12 15:57 [PATCH] drm/i915: fix use after free during eDP encoder destroying Imre Deak
@ 2014-12-12 17:06 ` Ville Syrjälä
  2014-12-15 14:45   ` Daniel Vetter
  2014-12-14  5:51 ` shuang.he
  1 sibling, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2014-12-12 17:06 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Fri, Dec 12, 2014 at 05:57:38PM +0200, Imre Deak wrote:
> After
> 
> commit a18c0af171bfb875012da26f23df051004726973
> uthor: Thierry Reding <treding@nvidia.com>
> Date:   Wed Dec 10 11:38:49 2014 +0100
> 
>     drm: Zero out DRM object memory upon cleanup
> 
> we will use the eDP encoder during destroying it. Fix this by calling
> drm_encoder_cleanup() at a point when the encoder is not used any more.
> This caused a NULL pointer dereference in pps_lock(), I can't see that
> it caused any other problem.
> 
> All the other encoders seem to call drm_encoder_cleanup() at a safe
> place.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

drm_encoder_cleanup() doesn't appear to have any nasty side effects
so this looks sane.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3fc3296..0a55623 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4310,7 +4310,6 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
>  
>  	drm_dp_aux_unregister(&intel_dp->aux);
>  	intel_dp_mst_encoder_cleanup(intel_dig_port);
> -	drm_encoder_cleanup(encoder);
>  	if (is_edp(intel_dp)) {
>  		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
>  		/*
> @@ -4326,6 +4325,7 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
>  			intel_dp->edp_notifier.notifier_call = NULL;
>  		}
>  	}
> +	drm_encoder_cleanup(encoder);
>  	kfree(intel_dig_port);
>  }
>  
> -- 
> 1.8.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: fix use after free during eDP encoder destroying
  2014-12-12 15:57 [PATCH] drm/i915: fix use after free during eDP encoder destroying Imre Deak
  2014-12-12 17:06 ` Ville Syrjälä
@ 2014-12-14  5:51 ` shuang.he
  1 sibling, 0 replies; 5+ messages in thread
From: shuang.he @ 2014-12-14  5:51 UTC (permalink / raw)
  To: shuang.he, intel-gfx, imre.deak

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  364/364              364/364
ILK              +3-3              362/366              362/366
SNB                                  448/450              448/450
IVB                 -1              497/498              496/498
BYT                                  289/289              289/289
HSW                                  563/564              563/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 ILK  igt_kms_flip_nonexisting-fb      DMESG_WARN(1, M26)PASS(3, M37M26)      PASS(1, M26)
 ILK  igt_kms_flip_rcs-flip-vs-panning-interruptible      DMESG_WARN(1, M26)PASS(3, M37M26)      PASS(1, M26)
 ILK  igt_kms_flip_rcs-wf_vblank-vs-dpms-interruptible      DMESG_WARN(1, M26)PASS(2, M26M37)      PASS(1, M26)
 ILK  igt_kms_flip_busy-flip-interruptible      DMESG_WARN(1, M26)PASS(1, M26)      DMESG_WARN(1, M26)
*ILK  igt_kms_flip_rcs-flip-vs-panning      PASS(2, M26)      DMESG_WARN(1, M26)
*ILK  igt_kms_flip_vblank-vs-hang      PASS(2, M26)      NSPT(1, M26)
*IVB  igt_kms_cursor_crc_cursor-256x256-offscreen      PASS(2, M4M21)      DMESG_WARN(1, M21)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: fix use after free during eDP encoder destroying
  2014-12-12 17:06 ` Ville Syrjälä
@ 2014-12-15 14:45   ` Daniel Vetter
  2014-12-16  8:30     ` Thierry Reding
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Vetter @ 2014-12-15 14:45 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Thierry Reding, intel-gfx

On Fri, Dec 12, 2014 at 07:06:24PM +0200, Ville Syrjälä wrote:
> On Fri, Dec 12, 2014 at 05:57:38PM +0200, Imre Deak wrote:
> > After
> > 
> > commit a18c0af171bfb875012da26f23df051004726973
> > uthor: Thierry Reding <treding@nvidia.com>
> > Date:   Wed Dec 10 11:38:49 2014 +0100
> > 
> >     drm: Zero out DRM object memory upon cleanup

Please cc Thierry next time around. I've added him.

> > 
> > we will use the eDP encoder during destroying it. Fix this by calling
> > drm_encoder_cleanup() at a point when the encoder is not used any more.
> > This caused a NULL pointer dereference in pps_lock(), I can't see that
> > it caused any other problem.
> > 
> > All the other encoders seem to call drm_encoder_cleanup() at a safe
> > place.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> drm_encoder_cleanup() doesn't appear to have any nasty side effects
> so this looks sane.
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/i915: fix use after free during eDP encoder destroying
  2014-12-15 14:45   ` Daniel Vetter
@ 2014-12-16  8:30     ` Thierry Reding
  0 siblings, 0 replies; 5+ messages in thread
From: Thierry Reding @ 2014-12-16  8:30 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 803 bytes --]

On Mon, Dec 15, 2014 at 03:45:34PM +0100, Daniel Vetter wrote:
> On Fri, Dec 12, 2014 at 07:06:24PM +0200, Ville Syrjälä wrote:
> > On Fri, Dec 12, 2014 at 05:57:38PM +0200, Imre Deak wrote:
> > > After
> > > 
> > > commit a18c0af171bfb875012da26f23df051004726973
> > > uthor: Thierry Reding <treding@nvidia.com>
> > > Date:   Wed Dec 10 11:38:49 2014 +0100
> > > 
> > >     drm: Zero out DRM object memory upon cleanup
> 
> Please cc Thierry next time around. I've added him.

I don't have a copy of the original patch in any of my inboxes. It'd be
nice to Cc dri-devel@lists.freedesktop.org on patches of this nature.

I had to go dig for the patch in the intel-gfx archives and it looks
good to me, so if it's not too late yet:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-12-16  8:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-12 15:57 [PATCH] drm/i915: fix use after free during eDP encoder destroying Imre Deak
2014-12-12 17:06 ` Ville Syrjälä
2014-12-15 14:45   ` Daniel Vetter
2014-12-16  8:30     ` Thierry Reding
2014-12-14  5:51 ` shuang.he

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox