Skip to contentSkip to conversation

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

PATCH In review Dan Carpenter 2 participants 3 messages Nov 10, 2013 – Nov 11, 2013 2 messages not in the archive

Subsystems (4)
DRM drivers Kernel janitors Tegra architecture support DRM drivers for NVIDIA Tegra
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

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

If we don't have enough memory for ->planes then we leak "fb".

Signed-off-by Dan Carpenter <[email protected]>
drivers/gpu/drm/tegra/fb.c +12 -5
index 490f771..1362d78 100644
@@ -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,

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw
Quoted 3 lines · Dan Carpenter

If we don't have enough memory for ->planes then we leak "fb".

Signed-off-by Dan Carpenter <[email protected]>

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.

Quoted 26 lines
drivers/gpu/drm/tegra/fb.c +5 -5
index 490f771..1362d78 100644
@@ -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;
 	}

drivers/gpu/drm/tegra/fb.c static struct tegra_fb *tegra_fb_alloc(struct drm_device *drm, · line 128

Quoted 10 lines
drivers/gpu/drm/tegra/fb.c +7 -0
 
 	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

A message that is not in the archive (sent off-list or not yet received): 20131111090348.GD3884-AwZRO8vwLAwmlAP/[email protected]