Re: [linux-fbdev-devel][PATCH]fb_pan_display:add x/yoffset check

["Ville Syrjälä" <syrjala@sci.fi>]

On Mon, Jun 29, 2009 at 11:49:31AM +0800, Kai Jiang wrote:
>

> >From a01ede69772634b30a83b44eada5a8db66f8463a Mon Sep 17 00:00:00 2001
> From: Kai Jiang <Kai.Jiang@freescale.com>
> Date: Mon, 29 Jun 2009 11:25:58 +0800
> Subject: [PATCH] When moving virtual space straight to one side in the screen(ex.
>  straight to the left),finally the virtual space will move outside
>  of the real screen. Then the xoffset or yoffset will be nagative
>  value(transfered from user application) to indicate that the virtual
>  space is beyond the screen boundary. In the function fb_pan_disaplay,
>  xoffset and yoffset should be checked to ensure that, when they are
>  negative, the virtual space will not move any more,and the function
>  will return an error. However, xoffset and yoffset in the structure
>  fb_var_screeninfo are "__u32" type, here need to transfer them to
>  "int" type for comparing.
> 
> Signed-off-by: Kai Jiang <Kai.Jiang@freescale.com>
> ---
>  drivers/video/fbmem.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index d412a1d..27628de 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -855,6 +855,8 @@ fb_pan_display(struct fb_info *info, struct fb_var_screeninfo *var)
>  {
>  	struct fb_fix_screeninfo *fix = &info->fix;
>  	unsigned int yres = info->var.yres;
> +	int xoffset = var->xoffset;
> +	int yoffset = var->yoffset;
>  	int err = 0;
>  
>  	if (var->yoffset > 0) {
> @@ -873,7 +875,8 @@ fb_pan_display(struct fb_info *info, struct fb_var_screeninfo *var)
>  
>  	if (err || !info->fbops->fb_pan_display ||
>  	    var->yoffset + yres > info->var.yres_virtual ||
> -	    var->xoffset + info->var.xres > info->var.xres_virtual)
> +	    var->xoffset + info->var.xres > info->var.xres_virtual ||
> +	    xoffset < 0 || yoffset < 0)

Well negative xoffset/yoffset don't really exist so what you're
essentially checking is whether offset+res overflows. Your check will
not catch all overflows though. xres/yres would have to be huge
(> 2^31) to cause such overflows though so your check should catch all
cases that can happen in practice. However I think it would be better
to make the overflow check clearer (eg. 'offset + res < res').

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/

------------------------------------------------------------------------------