From c353103de8795358af1584088aa471571decb307 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Sat, 13 Nov 2010 13:06:38 +0300 Subject: fbcmap: cleanup white space in fb_alloc_cmap() checkpatch.pl and Andrew Morton both complained about the indenting in fb_alloc_cmap() Signed-off-by: Dan Carpenter Signed-off-by: Paul Mundt --- drivers/video/fbcmap.c | 54 ++++++++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 24 deletions(-) (limited to 'drivers/video/fbcmap.c') diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c index f53b9f1..a79b976 100644 --- a/drivers/video/fbcmap.c +++ b/drivers/video/fbcmap.c @@ -90,32 +90,38 @@ static const struct fb_cmap default_16_colors = { int fb_alloc_cmap(struct fb_cmap *cmap, int len, int transp) { - int size = len*sizeof(u16); - - if (cmap->len != len) { - fb_dealloc_cmap(cmap); - if (!len) - return 0; - if (!(cmap->red = kmalloc(size, GFP_ATOMIC))) - goto fail; - if (!(cmap->green = kmalloc(size, GFP_ATOMIC))) - goto fail; - if (!(cmap->blue = kmalloc(size, GFP_ATOMIC))) - goto fail; - if (transp) { - if (!(cmap->transp = kmalloc(size, GFP_ATOMIC))) - goto fail; - } else - cmap->transp = NULL; - } - cmap->start = 0; - cmap->len = len; - fb_copy_cmap(fb_default_cmap(len), cmap); - return 0; + int size = len * sizeof(u16); + + if (cmap->len != len) { + fb_dealloc_cmap(cmap); + if (!len) + return 0; + + cmap->red = kmalloc(size, GFP_ATOMIC); + if (!cmap->red) + goto fail; + cmap->green = kmalloc(size, GFP_ATOMIC); + if (!cmap->green) + goto fail; + cmap->blue = kmalloc(size, GFP_ATOMIC); + if (!cmap->blue) + goto fail; + if (transp) { + cmap->transp = kmalloc(size, GFP_ATOMIC); + if (!cmap->transp) + goto fail; + } else { + cmap->transp = NULL; + } + } + cmap->start = 0; + cmap->len = len; + fb_copy_cmap(fb_default_cmap(len), cmap); + return 0; fail: - fb_dealloc_cmap(cmap); - return -ENOMEM; + fb_dealloc_cmap(cmap); + return -ENOMEM; } /** -- cgit v1.1 From 1e7c7804884fc5751e3872f13498fd533325f8b2 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 16 Nov 2010 12:11:02 +0300 Subject: fbcmap: integer overflow bug There is an integer overflow in fb_set_user_cmap() because cmap->len * 2 can wrap. It's basically harmless. Your terminal will be messed up until you type reset. This patch does three things to fix the bug. First, it checks the return value of fb_copy_cmap() in fb_alloc_cmap(). That is enough to fix address the overflow. Second it checks for the integer overflow in fb_set_user_cmap(). Lastly I wanted to cap "cmap->len" in fb_set_user_cmap() much lower because it gets used to determine the size of allocation. Unfortunately no one knows what the limit should be. Instead what this patch does is makes the allocation happen with GFP_KERNEL instead of GFP_ATOMIC and lets the kmalloc() decide what values of cmap->len are reasonable. To do this, the patch introduces a function called fb_alloc_cmap_gfp() which is like fb_alloc_cmap() except that it takes a GFP flag. Signed-off-by: Dan Carpenter Signed-off-by: Paul Mundt --- drivers/video/fbcmap.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) (limited to 'drivers/video/fbcmap.c') diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c index a79b976..affdf3e 100644 --- a/drivers/video/fbcmap.c +++ b/drivers/video/fbcmap.c @@ -88,26 +88,27 @@ static const struct fb_cmap default_16_colors = { * */ -int fb_alloc_cmap(struct fb_cmap *cmap, int len, int transp) +int fb_alloc_cmap_gfp(struct fb_cmap *cmap, int len, int transp, gfp_t flags) { int size = len * sizeof(u16); + int ret = -ENOMEM; if (cmap->len != len) { fb_dealloc_cmap(cmap); if (!len) return 0; - cmap->red = kmalloc(size, GFP_ATOMIC); + cmap->red = kmalloc(size, flags); if (!cmap->red) goto fail; - cmap->green = kmalloc(size, GFP_ATOMIC); + cmap->green = kmalloc(size, flags); if (!cmap->green) goto fail; - cmap->blue = kmalloc(size, GFP_ATOMIC); + cmap->blue = kmalloc(size, flags); if (!cmap->blue) goto fail; if (transp) { - cmap->transp = kmalloc(size, GFP_ATOMIC); + cmap->transp = kmalloc(size, flags); if (!cmap->transp) goto fail; } else { @@ -116,12 +117,19 @@ int fb_alloc_cmap(struct fb_cmap *cmap, int len, int transp) } cmap->start = 0; cmap->len = len; - fb_copy_cmap(fb_default_cmap(len), cmap); + ret = fb_copy_cmap(fb_default_cmap(len), cmap); + if (ret) + goto fail; return 0; fail: fb_dealloc_cmap(cmap); - return -ENOMEM; + return ret; +} + +int fb_alloc_cmap(struct fb_cmap *cmap, int len, int transp) +{ + return fb_alloc_cmap_gfp(cmap, len, transp, GFP_ATOMIC); } /** @@ -256,8 +264,12 @@ int fb_set_user_cmap(struct fb_cmap_user *cmap, struct fb_info *info) int rc, size = cmap->len * sizeof(u16); struct fb_cmap umap; + if (size < 0 || size < cmap->len) + return -E2BIG; + memset(&umap, 0, sizeof(struct fb_cmap)); - rc = fb_alloc_cmap(&umap, cmap->len, cmap->transp != NULL); + rc = fb_alloc_cmap_gfp(&umap, cmap->len, cmap->transp != NULL, + GFP_KERNEL); if (rc) return rc; if (copy_from_user(umap.red, cmap->red, size) || -- cgit v1.1