diff mbox series

drm/i915/fbc: __intel_fbc_cleanup_cfb() may be called multiple times

Message ID 20200130135136.1878646-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series drm/i915/fbc: __intel_fbc_cleanup_cfb() may be called multiple times | expand

Commit Message

Chris Wilson Jan. 30, 2020, 1:51 p.m. UTC
Avoid releasing the same stolen nodes causing a use-after-free and/or
explosions as the self-checks fail, as __intel_fbc_cleanup_cfb() may be
called multiple times during module unload.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Ville Syrjälä Jan. 30, 2020, 2:05 p.m. UTC | #1
On Thu, Jan 30, 2020 at 01:51:36PM +0000, Chris Wilson wrote:
> Avoid releasing the same stolen nodes causing a use-after-free and/or
> explosions as the self-checks fail, as __intel_fbc_cleanup_cfb() may be
> called multiple times during module unload.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 2a3f1333c8ff..ab676c6756af 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -537,13 +537,15 @@ static void __intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_fbc *fbc = &dev_priv->fbc;
>  
> -	if (drm_mm_node_allocated(&fbc->compressed_fb))
> -		i915_gem_stolen_remove_node(dev_priv, &fbc->compressed_fb);
> +	if (!drm_mm_node_allocated(&fbc->compressed_fb))
> +		return;
>  
>  	if (fbc->compressed_llb) {
>  		i915_gem_stolen_remove_node(dev_priv, fbc->compressed_llb);
>  		kfree(fbc->compressed_llb);
>  	}

Had to double check that we don't have some funny cases where we
have the llb allocated without the cfb. And leaving the stale
pointer behind doesn't seem like it can cause any other issues.

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

> +
> +	i915_gem_stolen_remove_node(dev_priv, &fbc->compressed_fb);
>  }
>  
>  void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv)
> -- 
> 2.25.0
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 2a3f1333c8ff..ab676c6756af 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -537,13 +537,15 @@  static void __intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv)
 {
 	struct intel_fbc *fbc = &dev_priv->fbc;
 
-	if (drm_mm_node_allocated(&fbc->compressed_fb))
-		i915_gem_stolen_remove_node(dev_priv, &fbc->compressed_fb);
+	if (!drm_mm_node_allocated(&fbc->compressed_fb))
+		return;
 
 	if (fbc->compressed_llb) {
 		i915_gem_stolen_remove_node(dev_priv, fbc->compressed_llb);
 		kfree(fbc->compressed_llb);
 	}
+
+	i915_gem_stolen_remove_node(dev_priv, &fbc->compressed_fb);
 }
 
 void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv)