[PATCH -next] drm/i915: fix potential NULL pointer dereference in i915_gem_context_get_hang_stats()

[PATCH -next] drm/i915: fix potential NULL pointer dereference in i915_gem_context_get_hang_stats()

[Wei Yongjun]

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

The dereference should be moved below the NULL test.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index ff47145..f32107e 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -309,7 +309,7 @@ i915_gem_context_get_hang_stats(struct intel_ring_buffer *ring,
 				u32 id)
 {
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
-	struct drm_i915_file_private *file_priv = file->driver_priv;
+	struct drm_i915_file_private *file_priv;
 	struct i915_hw_context *to;
 
 	if (dev_priv->hw_contexts_disabled)
@@ -321,6 +321,7 @@ i915_gem_context_get_hang_stats(struct intel_ring_buffer *ring,
 	if (file == NULL)
 		return ERR_PTR(-EINVAL);
 
+	file_priv = file->driver_priv;
 	if (id == DEFAULT_CONTEXT_ID)
 		return &file_priv->hang_stats;
 

Re: [PATCH -next] drm/i915: fix potential NULL pointer dereference in i915_gem_context_get_hang_stats()

[Damien Lespiau]

On Thu, Jun 20, 2013 at 08:01:47AM +0800, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> The dereference should be moved below the NULL test.
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index ff47145..f32107e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -309,7 +309,7 @@ i915_gem_context_get_hang_stats(struct intel_ring_buffer *ring,
>  				u32 id)
>  {
>  	struct drm_i915_private *dev_priv = ring->dev->dev_private;
> -	struct drm_i915_file_private *file_priv = file->driver_priv;
> +	struct drm_i915_file_private *file_priv;
>  	struct i915_hw_context *to;
>  
>  	if (dev_priv->hw_contexts_disabled)
> @@ -321,6 +321,7 @@ i915_gem_context_get_hang_stats(struct intel_ring_buffer *ring,
>  	if (file == NULL)
>  		return ERR_PTR(-EINVAL);
>  
> +	file_priv = file->driver_priv;
>  	if (id == DEFAULT_CONTEXT_ID)
>  		return &file_priv->hang_stats;

I think we could just not check for file == NULL here as it comes
directly from the ioctl() through i915_gem_execbuffer().

Patch coming...

-- 
Damien

[PATCH] drm/i915: Don't check if a drm_file * is NULL in the ioctl code path

[Damien Lespiau]

Right now code checkers point out that we try to dereference file before
testing it for NULL.

This check doesn't seem necessary as file needs to be valid for the
ioctl() code to run.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index ff47145..6573e47 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -318,9 +318,6 @@ i915_gem_context_get_hang_stats(struct intel_ring_buffer *ring,
 	if (ring->id != RCS)
 		return ERR_PTR(-EINVAL);
 
-	if (file == NULL)
-		return ERR_PTR(-EINVAL);
-
 	if (id == DEFAULT_CONTEXT_ID)
 		return &file_priv->hang_stats;
 
-- 
1.8.3.1

Re: [PATCH] drm/i915: Don't check if a drm_file * is NULL in the ioctl code path

[Chris Wilson]

On Thu, Jul 11, 2013 at 07:31:34PM +0100, Damien Lespiau wrote:
> Right now code checkers point out that we try to dereference file before
> testing it for NULL.
> 
> This check doesn't seem necessary as file needs to be valid for the
> ioctl() code to run.

This whole function was a copy'n'paste mistake with a rewrite on the
mailing list.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre