diff mbox

drm/tegra: small leak on error in tegra_fb_alloc()

Message ID 20131110053406.GB13643@elgon.mountain (mailing list archive)
State New, archived
Headers show

Commit Message

Dan Carpenter Nov. 10, 2013, 5:34 a.m. UTC
If we don't have enough memory for ->planes then we leak "fb".

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Comments

Thierry Reding Nov. 11, 2013, 9:03 a.m. UTC | #1
On Sat, Nov 09, 2013 at 09:34:06PM -0800, Dan Carpenter wrote:
> If we don't have enough memory for ->planes then we leak "fb".
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Hi Dan,

Thanks for catching this. Perhaps tweak the commit subject to:

	drm/tegra: fix small leak on error in tegra_fb_alloc()

?

One additional comment below.

> diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
> index 490f771..1362d78 100644
> --- a/drivers/gpu/drm/tegra/fb.c
> +++ b/drivers/gpu/drm/tegra/fb.c
> @@ -98,8 +98,10 @@ static struct tegra_fb *tegra_fb_alloc(struct drm_device *drm,
>  		return ERR_PTR(-ENOMEM);
>  
>  	fb->planes = kzalloc(num_planes * sizeof(*planes), GFP_KERNEL);
> -	if (!fb->planes)
> -		return ERR_PTR(-ENOMEM);
> +	if (!fb->planes) {
> +		err = -ENOMEM;
> +		goto free_fb;
> +	}
>  
>  	fb->num_planes = num_planes;
>  
> @@ -112,12 +114,17 @@ static struct tegra_fb *tegra_fb_alloc(struct drm_device *drm,
>  	if (err < 0) {
>  		dev_err(drm->dev, "failed to initialize framebuffer: %d\n",
>  			err);
> -		kfree(fb->planes);
> -		kfree(fb);
> -		return ERR_PTR(err);
> +		goto free_planes;
>  	}
>  
>  	return fb;
> +
> +free_planes:
> +	kfree(fb->planes);
> +free_fb:
> +	kfree(fb);
> +
> +	return ERR_PTR(err);
>  }

I think in this case I'd actually prefer for the kfree(fb) to be
duplicated and not have this error unwind. It's actually shorter
to do it that way in this case.

What I mean is:

	if (!fb->planes) {
		kfree(fb);
		return ERR_PTR(-ENOMEM);
	}

Thierry
Dan Carpenter Nov. 11, 2013, 9:42 a.m. UTC | #2
Sure.  I will resend.

regards,
dan carpenter
diff mbox

Patch

diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index 490f771..1362d78 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -98,8 +98,10 @@  static struct tegra_fb *tegra_fb_alloc(struct drm_device *drm,
 		return ERR_PTR(-ENOMEM);
 
 	fb->planes = kzalloc(num_planes * sizeof(*planes), GFP_KERNEL);
-	if (!fb->planes)
-		return ERR_PTR(-ENOMEM);
+	if (!fb->planes) {
+		err = -ENOMEM;
+		goto free_fb;
+	}
 
 	fb->num_planes = num_planes;
 
@@ -112,12 +114,17 @@  static struct tegra_fb *tegra_fb_alloc(struct drm_device *drm,
 	if (err < 0) {
 		dev_err(drm->dev, "failed to initialize framebuffer: %d\n",
 			err);
-		kfree(fb->planes);
-		kfree(fb);
-		return ERR_PTR(err);
+		goto free_planes;
 	}
 
 	return fb;
+
+free_planes:
+	kfree(fb->planes);
+free_fb:
+	kfree(fb);
+
+	return ERR_PTR(err);
 }
 
 static struct drm_framebuffer *tegra_fb_create(struct drm_device *drm,