From: Florian T. S. <Flo...@gm...> - 2009-07-06 14:17:49
|
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 |