Skip to contentSkip to conversation

Leak in vesafb

PATCH In review Felipe W Damasio 1 participant 2 messages Oct 6, 2003 Framebuffer layer

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Leak in vesafb

Hi Andrew,

Patch against 2.6.0-test6.

Releases a previous request'ed_mem_region. Found by smatch.

Since it didn't checked the return value of request_region, I'm not
sure we should free it here...since (as it says on the driver), "vgacon probably has this region already".

Andrew, I'd appreciate you could review this..

Thanks.

Felipe

drivers/video/vesafb.c renamed from drivers/video/vesafb.c.orig +3 -1
@@ -366,8 +366,10 @@
 
 	fb_alloc_cmap(&fb_info.cmap, video_cmap_len, 0);
 
-	if (register_framebuffer(&fb_info)<0)
+	if (register_framebuffer(&fb_info)<0) {
+		release_mem_region(vesafb_fix.smem_start, vesafb_fix.smem_len);
 		return -EINVAL;
+	}
 
 	printk(KERN_INFO "fb%d: %s frame buffer device\n",
 	       fb_info.node, fb_info.fix.id);
#1 lore raw
Quoted 10 lines · Felipe W Damasio

Hi Andrew,

Patch against 2.6.0-test6.

Releases a previous request'ed_mem_region. Found by smatch.

Since it didn't checked the return value of request_region, I'm not
sure we should free it here...since (as it says on the driver), "vgacon probably has this region already".

Andrew, I'd appreciate you could review this..

Also, this updated patch also iounmap fb_info.screen_base, since
we're about to return -EINVAL on vesa_init.

Cheers,

Felipe

drivers/video/vesafb.c renamed from drivers/video/vesafb.c.orig +4 -1
@@ -366,8 +366,11 @@
 
 	fb_alloc_cmap(&fb_info.cmap, video_cmap_len, 0);
 
-	if (register_framebuffer(&fb_info)<0)
+	if (register_framebuffer(&fb_info)<0) {
+		release_mem_region(vesafb_fix.smem_start, vesafb_fix.smem_len);
+		iounmap(fb_info.screen_base);
 		return -EINVAL;
+	}
 
 	printk(KERN_INFO "fb%d: %s frame buffer device\n",
 	       fb_info.node, fb_info.fix.id);