Messages in this thread Patch in this message | | | Date | Mon, 27 Apr 2009 13:58:17 +0200 | From | Roel Kluin <> | Subject | Re: [Linux-fbdev-devel] fbcmap: non working tests on unsigned cmap->start |
| |
Ville Syrjälä wrote: > On Sun, Apr 26, 2009 at 02:20:47PM +0200, Roel Kluin wrote: >> cmap->start is unsigned,
>>> On Thu, Apr 23, 2009 at 01:41:10PM -0700, Andrew Morton wrote: >>>> argh. >>>> >>>> - Perhaps userspace can kill the kernel by sending a "negative" >>>> `start'. Removing the test will make it even less likely that we'll >>>> fix this bug. >>> Shouldn't happen. 'start' is used as the starting index for the hardware >>> palette, 'start+len-1' is the last index. All drivers should already check >>> the passed values since the maximum index depends on the display mode. >>> And I suppose the worst thing that could happen if the driver fails to >>> check the values would be incorrect colors.
> You would rather need something like > 'if (start+len > 1 << max(red.len, green.len, blue.len, transp.len))'
what do you mean with `red.len'? is that `info->var.red.length'?
> and a check to make sure that start+len doesn't overflow. > > Oh and I guess it should also check that the visual is pseudocolor or > directcolor.
I am fairly new so please review carefully.
Not yet signed off-by: Roel Kluin <roel.kluin@gmail.com> --- diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c index f53b9f1..b34e74e 100644 --- a/drivers/video/fbcmap.c +++ b/drivers/video/fbcmap.c @@ -249,6 +249,7 @@ 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; + __u32 rgba_max = 0; memset(&umap, 0, sizeof(struct fb_cmap)); rc = fb_alloc_cmap(&umap, cmap->len, cmap->transp != NULL); @@ -261,13 +262,27 @@ int fb_set_user_cmap(struct fb_cmap_user *cmap, struct fb_info *info) rc = -EFAULT; goto out; } + + if (cmap->start + cmap->len < cmap->start) { + rc = -EINVAL; + goto out; + } + umap.start = cmap->start; if (!lock_fb_info(info)) { rc = -ENODEV; goto out; } - if (cmap->start < 0 || (!info->fbops->fb_setcolreg && - !info->fbops->fb_setcmap)) { + + rgba_max = max(info->var.red.length, info->var.green.length); + rgba_max = max(rgba_max, info->var.blue.length); + rgba_max = max(rgba_max, info->var.transp.length); + + if (cmap->start + cmap->len > 1 << rgba_max || + !(info->fbops->fb_setcolreg || + info->fbops->fb_setcmap) || + !(info->fix.visual == FB_VISUAL_PSEUDOCOLOR || + info->fix.visual == FB_VISUAL_TRUECOLOR)) { rc = -EINVAL; goto out1; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |