Re: [PATCH] drm/i915: Fix null pointer dereference in ring cleanup code

[Damien Lespiau <damien.lespiau@intel.com>]

On Fri, Oct 31, 2014 at 12:00:26PM +0000, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> If a ring failed to initialise for any reason then the error path would try to
> clean up all rings including those that had not yet been allocated. The ring
> clean up code did a check that the ring was valid before starting its work.
> Unfortunately, that was after it had already dereferenced the ring to obtain a
> dev_private pointer.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>

This looks good to me.

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>


-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_lrc.c        |    4 +++-
>  drivers/gpu/drm/i915/intel_ringbuffer.c |    7 +++++--
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index cd74e5c..76776fa 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1214,11 +1214,13 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf)
>   */
>  void intel_logical_ring_cleanup(struct intel_engine_cs *ring)
>  {
> -	struct drm_i915_private *dev_priv = ring->dev->dev_private;
> +	struct drm_i915_private *dev_priv;
>  
>  	if (!intel_ring_initialized(ring))
>  		return;
>  
> +	dev_priv = ring->dev->dev_private;
> +
>  	intel_logical_ring_stop(ring);
>  	WARN_ON((I915_READ_MODE(ring) & MODE_IDLE) == 0);
>  	ring->preallocated_lazy_request = NULL;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index a8f72e8..f457146 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1845,12 +1845,15 @@ error:
>  
>  void intel_cleanup_ring_buffer(struct intel_engine_cs *ring)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(ring->dev);
> -	struct intel_ringbuffer *ringbuf = ring->buffer;
> +	struct drm_i915_private *dev_priv;
> +	struct intel_ringbuffer *ringbuf;
>  
>  	if (!intel_ring_initialized(ring))
>  		return;
>  
> +	dev_priv = to_i915(ring->dev);
> +	ringbuf = ring->buffer;
> +
>  	intel_stop_ring_buffer(ring);
>  	WARN_ON(!IS_GEN2(ring->dev) && (I915_READ_MODE(ring) & MODE_IDLE) == 0);
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx