Re: [PATCH 2.6.28 ] i810: kernel crash fix when structfb_var_screeninfo is supplied

[Andrew Morton]

On Wed, 4 Mar 2009 23:54:49 +0100 (CET)
Jiri Kosina <jkosina@xxxxxxx> wrote:

> On Wed, 4 Mar 2009, Andrew Morton wrote:
> 
> > > This is not appropriate for trivial tree. CCing akpm and lkml.
> > I don't have a copy of the original patch.
> 
> This is what I got from Samuel (and rejected it for trivial tree, not 
> being trivial enough :) )
> 

ok, thanks.

> --- linux-2.6.28/drivers/video/i810/i810_main.c.orig    2009-02-26 15:23:03.000000000 +0100
> +++ linux-2.6.28/drivers/video/i810/i810_main.c 2009-02-26 14:50:06.000000000 +0100
> @@ -993,6 +993,8 @@ static int i810_check_params(struct fb_v
>         struct i810fb_par *par = info->par;
>         int line_length, vidmem, mode_valid = 0, retval = 0;
>         u32 vyres = var->yres_virtual, vxres = var->xres_virtual;
> +       u32 yres = info->var.yres;
> +
>         /*
>          *  Memory limit
>          */

So we have a file-wide static called `yres' - seems a bit dangerous.

Rather than defining a local which shadows the global, it would be
clearer to do it in this equivalent way, yes?

--- a/drivers/video/i810/i810_main.c~i810-fix-kernel-crash-fix-when-struct-fb_var_screeninfo-is-supplied
+++ a/drivers/video/i810/i810_main.c
@@ -993,6 +993,7 @@ static int i810_check_params(struct fb_v
 	struct i810fb_par *par = info->par;
 	int line_length, vidmem, mode_valid = 0, retval = 0;
 	u32 vyres = var->yres_virtual, vxres = var->xres_virtual;
+
 	/*
 	 *  Memory limit
 	 */
@@ -1002,12 +1003,12 @@ static int i810_check_params(struct fb_v
 	if (vidmem > par->fb.size) {
 		vyres = par->fb.size/line_length;
 		if (vyres < var->yres) {
-			vyres = yres;
+			vyres = info->var.yres;
 			vxres = par->fb.size/vyres;
 			vxres /= var->bits_per_pixel >> 3;
 			line_length = get_line_length(par, vxres, 
 						      var->bits_per_pixel);
-			vidmem = line_length * yres;
+			vidmem = line_length * info->var.yres;
 			if (vxres < var->xres) {
 				printk("i810fb: required video memory, "
 				       "%d bytes, for %dx%d-%d (virtual) "
_

This assumes that the original change was actually correct.  Should
both sites whcih use the global `yres' have been converted to use
info->var.yres, or just one of them?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/