* [RFC] [patch] fbcmap: integer overflow bug @ 2010-10-27 9:37 Dan Carpenter 2010-11-05 20:40 ` Andrew Morton 0 siblings, 1 reply; 11+ messages in thread From: Dan Carpenter @ 2010-10-27 9:37 UTC (permalink / raw) To: linux-fbdev; +Cc: linux-kernel, kernel-janitors There is an integer overflow bug in the FBIOPUTCMAP ioctl if cmap->len * 2 overflows. It's harmless, except that it messes up the cmap until someone types `reset`. It could have been caught by checking the return from fb_copy_cmap(). Or it could have been caught by limiting the size of the cmaps to one page. The cmaps are allocated with GFP_ATOMIC and it makes sense to limit them. Different drivers use different sizes of cmaps. There are about 150 drivers. I've checked a bunch (50) of them and the larges cmap.len I've found is gxt4500 which maxes out at 1024 so PAGE_SIZE is about twice that length. For some of the 50 I wasn't sure on the limit. Is PAGE_SIZE a reasonable limit? Does anyone know or do I have to audit all 150 drivers? diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c index f53b9f1..6dc5817 100644 --- a/drivers/video/fbcmap.c +++ b/drivers/video/fbcmap.c @@ -110,7 +110,9 @@ 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); + if (fb_copy_cmap(fb_default_cmap(len), cmap)) + goto fail; + return 0; fail: @@ -250,6 +252,9 @@ 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 (cmap->len > PAGE_SIZE || cmap->len * sizeof(u16) > PAGE_SIZE) + return -EINVAL; + memset(&umap, 0, sizeof(struct fb_cmap)); rc = fb_alloc_cmap(&umap, cmap->len, cmap->transp != NULL); if (rc) ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC] [patch] fbcmap: integer overflow bug 2010-10-27 9:37 [RFC] [patch] fbcmap: integer overflow bug Dan Carpenter @ 2010-11-05 20:40 ` Andrew Morton 2010-11-13 10:06 ` [patch 1/2] fbcmap: cleanup white space in fb_alloc_cmap() Dan Carpenter 2010-11-13 10:07 ` [patch 2/2] fbcmap: integer overflow bug Dan Carpenter 0 siblings, 2 replies; 11+ messages in thread From: Andrew Morton @ 2010-11-05 20:40 UTC (permalink / raw) To: Dan Carpenter; +Cc: linux-fbdev, linux-kernel, kernel-janitors On Wed, 27 Oct 2010 11:37:16 +0200 Dan Carpenter <error27@gmail.com> wrote: > There is an integer overflow bug in the FBIOPUTCMAP ioctl if > cmap->len * 2 overflows. > > It's harmless, except that it messes up the cmap until someone types > `reset`. It could have been caught by checking the return from > fb_copy_cmap(). > > Or it could have been caught by limiting the size of the cmaps to one > page. The cmaps are allocated with GFP_ATOMIC and it makes sense to > limit them. > > Different drivers use different sizes of cmaps. There are about 150 > drivers. I've checked a bunch (50) of them and the larges cmap.len I've > found is gxt4500 which maxes out at 1024 so PAGE_SIZE is about twice that > length. For some of the 50 I wasn't sure on the limit. > > Is PAGE_SIZE a reasonable limit? Does anyone know or do I have to audit > all 150 drivers? No signed-off-by:. > diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c > index f53b9f1..6dc5817 100644 > --- a/drivers/video/fbcmap.c > +++ b/drivers/video/fbcmap.c > @@ -110,7 +110,9 @@ 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); > + if (fb_copy_cmap(fb_default_cmap(len), cmap)) > + goto fail; > + > return 0; > > fail: > @@ -250,6 +252,9 @@ 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 (cmap->len > PAGE_SIZE || cmap->len * sizeof(u16) > PAGE_SIZE) > + return -EINVAL; > + > memset(&umap, 0, sizeof(struct fb_cmap)); > rc = fb_alloc_cmap(&umap, cmap->len, cmap->transp != NULL); > if (rc) A suitable way to fix this would be to detect the overflow only, and then just throw the passed-in length at kmalloc(), and let kmalloc() decide whether it is sane or not. To do that, one would need to implement a new fb_alloc_cmap_not_stupid() which takes a gfp_t, and pass in GFP_KERNEL. Feel free to sanitise the fb_alloc_cmap() indenting in [patch 1/2] as well ;) ^ permalink raw reply [flat|nested] 11+ messages in thread
* [patch 1/2] fbcmap: cleanup white space in fb_alloc_cmap() 2010-11-05 20:40 ` Andrew Morton @ 2010-11-13 10:06 ` Dan Carpenter 2010-11-18 6:00 ` Paul Mundt 2010-11-13 10:07 ` [patch 2/2] fbcmap: integer overflow bug Dan Carpenter 1 sibling, 1 reply; 11+ messages in thread From: Dan Carpenter @ 2010-11-13 10:06 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-fbdev, linux-kernel, kernel-janitors checkpatch.pl and Andrew Morton both complained about the indenting in fb_alloc_cmap() Signed-off-by: Dan Carpenter <error27@gmail.com> 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; } /** ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [patch 1/2] fbcmap: cleanup white space in fb_alloc_cmap() 2010-11-13 10:06 ` [patch 1/2] fbcmap: cleanup white space in fb_alloc_cmap() Dan Carpenter @ 2010-11-18 6:00 ` Paul Mundt 0 siblings, 0 replies; 11+ messages in thread From: Paul Mundt @ 2010-11-18 6:00 UTC (permalink / raw) To: Dan Carpenter, Andrew Morton, linux-fbdev, linux-kernel, kernel-janitors On Sat, Nov 13, 2010 at 01:06:38PM +0300, Dan Carpenter wrote: > checkpatch.pl and Andrew Morton both complained about the indenting in > fb_alloc_cmap() On Tue, Nov 16, 2010 at 12:11:02PM +0300, Dan Carpenter wrote: > 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. Both applied, thanks. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [patch 2/2] fbcmap: integer overflow bug 2010-11-05 20:40 ` Andrew Morton 2010-11-13 10:06 ` [patch 1/2] fbcmap: cleanup white space in fb_alloc_cmap() Dan Carpenter @ 2010-11-13 10:07 ` Dan Carpenter 2010-11-15 4:48 ` Paul Mundt 1 sibling, 1 reply; 11+ messages in thread From: Dan Carpenter @ 2010-11-13 10:07 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-fbdev, linux-kernel, kernel-janitors 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 <error27@gmail.com> diff --git a/include/linux/fb.h b/include/linux/fb.h index 7fca3dc..d1631d3 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -1122,6 +1122,7 @@ extern const struct fb_videomode *fb_find_best_display(const struct fb_monspecs /* drivers/video/fbcmap.c */ extern int fb_alloc_cmap(struct fb_cmap *cmap, int len, int transp); +extern int fb_alloc_cmap_gfp(struct fb_cmap *cmap, int len, int transp, gfp_t flags); extern void fb_dealloc_cmap(struct fb_cmap *cmap); extern int fb_copy_cmap(const struct fb_cmap *from, struct fb_cmap *to); extern int fb_cmap_to_user(const struct fb_cmap *from, struct fb_cmap_user *to); diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c index a79b976..e6a01bf 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 (cmap->len * 2 > INT_MAX) + return -EINVAL; + 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) || ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [patch 2/2] fbcmap: integer overflow bug 2010-11-13 10:07 ` [patch 2/2] fbcmap: integer overflow bug Dan Carpenter @ 2010-11-15 4:48 ` Paul Mundt 2010-11-15 6:56 ` Geert Uytterhoeven 2010-11-16 9:11 ` [patch 2/2 v2] " Dan Carpenter 0 siblings, 2 replies; 11+ messages in thread From: Paul Mundt @ 2010-11-15 4:48 UTC (permalink / raw) To: Dan Carpenter, Andrew Morton, linux-fbdev, linux-kernel, kernel-janitors On Sat, Nov 13, 2010 at 01:07:18PM +0300, Dan Carpenter wrote: > @@ -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 (cmap->len * 2 > INT_MAX) > + return -EINVAL; > + > 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) || This looks reasonable, but it probably makes more sense to use -E2BIG for the overflow case (as other cases are doing already), and also just to check size directly rather than open-coding the * 2. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch 2/2] fbcmap: integer overflow bug 2010-11-15 4:48 ` Paul Mundt @ 2010-11-15 6:56 ` Geert Uytterhoeven 2010-11-15 7:20 ` Dan Carpenter 2010-11-16 9:11 ` [patch 2/2 v2] " Dan Carpenter 1 sibling, 1 reply; 11+ messages in thread From: Geert Uytterhoeven @ 2010-11-15 6:56 UTC (permalink / raw) To: Paul Mundt Cc: Dan Carpenter, Andrew Morton, linux-fbdev, linux-kernel, kernel-janitors On Mon, Nov 15, 2010 at 05:48, Paul Mundt <lethal@linux-sh.org> wrote: > On Sat, Nov 13, 2010 at 01:07:18PM +0300, Dan Carpenter wrote: >> @@ -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 (cmap->len * 2 > INT_MAX) Isn't that another integer overflow? I.e. should be "if (cmap->len > INT_MAX / sizeof(u16))" instead? >> + return -EINVAL; >> + >> 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) || > > This looks reasonable, but it probably makes more sense to use -E2BIG > for the overflow case (as other cases are doing already), and also just > to check size directly rather than open-coding the * 2. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch 2/2] fbcmap: integer overflow bug 2010-11-15 6:56 ` Geert Uytterhoeven @ 2010-11-15 7:20 ` Dan Carpenter 2010-11-15 8:01 ` walter harms 0 siblings, 1 reply; 11+ messages in thread From: Dan Carpenter @ 2010-11-15 7:20 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Paul Mundt, Andrew Morton, linux-fbdev, linux-kernel, kernel-janitors On Mon, Nov 15, 2010 at 07:56:05AM +0100, Geert Uytterhoeven wrote: > On Mon, Nov 15, 2010 at 05:48, Paul Mundt <lethal@linux-sh.org> wrote: > > On Sat, Nov 13, 2010 at 01:07:18PM +0300, Dan Carpenter wrote: > >> @@ -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 (cmap->len * 2 > INT_MAX) > > Isn't that another integer overflow? I.e. should be "if (cmap->len > > INT_MAX / sizeof(u16))" instead? > Yeah it is. :/ I'll change it to: if (size < 0 || size < cmap->len) like Paul asked. regards, dan carpenter ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch 2/2] fbcmap: integer overflow bug 2010-11-15 7:20 ` Dan Carpenter @ 2010-11-15 8:01 ` walter harms 2010-11-15 8:14 ` Dan Carpenter 0 siblings, 1 reply; 11+ messages in thread From: walter harms @ 2010-11-15 8:01 UTC (permalink / raw) To: Dan Carpenter, Geert Uytterhoeven, Paul Mundt, Andrew Morton, linux-fbdev, linux-kernel, kernel-janitors Am 15.11.2010 08:20, schrieb Dan Carpenter: > On Mon, Nov 15, 2010 at 07:56:05AM +0100, Geert Uytterhoeven wrote: >> On Mon, Nov 15, 2010 at 05:48, Paul Mundt <lethal@linux-sh.org> wrote: >>> On Sat, Nov 13, 2010 at 01:07:18PM +0300, Dan Carpenter wrote: >>>> @@ -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 (cmap->len * 2 > INT_MAX) >> >> Isn't that another integer overflow? I.e. should be "if (cmap->len > >> INT_MAX / sizeof(u16))" instead? >> > > Yeah it is. :/ > > I'll change it to: > if (size < 0 || size < cmap->len) > like Paul asked. > I do not see the rest of the code but it looks like cmap->len is size in int8_t. So the upper limit is something like INT_MAX/(sizeof(u16)*2). Perhaps we can call it a char ? is there ANY system that has a more than 256 colors in R|G|B ? <spying for struct fb_cmap_user > __u32 len; __u16 __user *red; So no need to check for <0. hope that helps, re, wh ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch 2/2] fbcmap: integer overflow bug 2010-11-15 8:01 ` walter harms @ 2010-11-15 8:14 ` Dan Carpenter 0 siblings, 0 replies; 11+ messages in thread From: Dan Carpenter @ 2010-11-15 8:14 UTC (permalink / raw) To: walter harms Cc: Geert Uytterhoeven, Paul Mundt, Andrew Morton, linux-fbdev, linux-kernel, kernel-janitors On Mon, Nov 15, 2010 at 09:01:14AM +0100, walter harms wrote: > I do not see the rest of the code but it looks like > cmap->len is size in int8_t. So the upper limit is something like > INT_MAX/(sizeof(u16)*2). Perhaps we can call it a char ? > is there ANY system that has a more than 256 colors in R|G|B ? > Yeah. There are. I had a list of some but I've deleted it. > <spying for struct fb_cmap_user > > __u32 len; > __u16 __user *red; > > So no need to check for <0. I test size which is an int so that's why it's needed. regards, dan carpenter ^ permalink raw reply [flat|nested] 11+ messages in thread
* [patch 2/2 v2] fbcmap: integer overflow bug 2010-11-15 4:48 ` Paul Mundt 2010-11-15 6:56 ` Geert Uytterhoeven @ 2010-11-16 9:11 ` Dan Carpenter 1 sibling, 0 replies; 11+ messages in thread From: Dan Carpenter @ 2010-11-16 9:11 UTC (permalink / raw) To: Paul Mundt; +Cc: Andrew Morton, linux-fbdev, linux-kernel, kernel-janitors 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 <error27@gmail.com> --- v2: Fix overflow check in fb_set_user_cmap(). Return -E2BIG on error. diff --git a/include/linux/fb.h b/include/linux/fb.h index 7fca3dc..d1631d3 100644 --- a/include/linux/fb.h +++ b/include/linux/fb.h @@ -1122,6 +1122,7 @@ extern const struct fb_videomode *fb_find_best_display(const struct fb_monspecs /* drivers/video/fbcmap.c */ extern int fb_alloc_cmap(struct fb_cmap *cmap, int len, int transp); +extern int fb_alloc_cmap_gfp(struct fb_cmap *cmap, int len, int transp, gfp_t flags); extern void fb_dealloc_cmap(struct fb_cmap *cmap); extern int fb_copy_cmap(const struct fb_cmap *from, struct fb_cmap *to); extern int fb_cmap_to_user(const struct fb_cmap *from, struct fb_cmap_user *to); 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) || ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-11-18 6:00 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-10-27 9:37 [RFC] [patch] fbcmap: integer overflow bug Dan Carpenter 2010-11-05 20:40 ` Andrew Morton 2010-11-13 10:06 ` [patch 1/2] fbcmap: cleanup white space in fb_alloc_cmap() Dan Carpenter 2010-11-18 6:00 ` Paul Mundt 2010-11-13 10:07 ` [patch 2/2] fbcmap: integer overflow bug Dan Carpenter 2010-11-15 4:48 ` Paul Mundt 2010-11-15 6:56 ` Geert Uytterhoeven 2010-11-15 7:20 ` Dan Carpenter 2010-11-15 8:01 ` walter harms 2010-11-15 8:14 ` Dan Carpenter 2010-11-16 9:11 ` [patch 2/2 v2] " Dan Carpenter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).