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

[Florian Tobias Schandinat <FlorianSchandinat@gmx.de>]

Kai Jiang schrieb:
> Florian Tobias Schandinat wrote:
>> Ville Syrjälä schrieb:
>>>> So here we have to check the whether the x/yoffset is smaller than 
>>>> zero. If the offset is smaller than zero, in the driver, we should 
>>>> not move the virtual screen any more.
>>>
>>> Checking for overflow will catch you buggy application's negative
>>> values too.
>>
>> That's true, but the problem lies in the current implementation first 
>> adding the resolution, which results in small negative [0 to 
>> -resolution] values (=large positives) being accepted as they overflow 
>> during add and become small positive values.
>> I'd recommend changing
>>
>> var->yoffset + yres > info->var.yres_virtual ||
>> var->xoffset + info->var.xres > info->var.xres_virtual
>>
>> to
>>
>> var->yoffset > info->var.yres_virtual - yres ||
>> var->xoffset > info->var.xres_virtual - info->var.xres
>>
> I am not sure why do we have these change. Could you give a detail 
> description or an example?

A small program to illustrate it:

#include <stdio.h>
int main()
{
         unsigned int    a = -1;
         printf( "%X\n%X\n", a, a+1 );
         return 0;
}

It starts with "-1" in an u32 being represented as "0xFFFFFFFF", which 
would be caught by ">". The problem in the current code is it first adds 
the resolution before comparison and this causes an overflow.
Let's say the virtual resolution matches the real resolution:
yoffset + yres > yres
There the left side is evaluated at first:
(yoffset + yres)
You accept everything that is <=yres. In classical mathematics you would 
say yoffset has to be 0, but unfortunately this codes accept many more 
as it can overflow. You get
yoffset = -1:	(yres-1) > yres
offset = -yres:	0 > yres
So as you noticed, the current code will not just accept 0 as yoffset, 
but the whole range [-yres..0]. This can be fixed by moving the 
calculation to the right side, where we have trusted values, that do not 
cause an overflow.

Hope this helps.


Greetings,

Florian Tobias Schandinat

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