[PATCH xserver v2 2/2] glamor: Avoid overflow between box32 and box16 box

[Olivier Fourdan]

Hi Michel,

> > glamor_compute_transform_clipped_regions() uses a temporary box32
> > internally which is copied back to a box16 to init the regions16,
> > thus causing a potential overflow.
> > 
> > If an overflow occurs, the given region is invalid and the pixmap
> > init region will fail.
> > 
> > Simply check that the coordinates won't overflow when copying back to
> > the box16, avoiding a crash later down the line in glamor.
> > 
> > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=101894
> > Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> > ---
> >  v2: Make sure we have (x1,y1) < (x2,y2) in case of overflow to avoid an
> >      empty region.
> 
> An empty region actually seems more appropriate to me in that case.
> Maybe just don't call RegionInitBoxes if short_box is empty?

Thing is, looking at pixman's definition of GOOD_RECT() and BAD_RECT() [1], an empty region is neither, so not being a "GOOD_RECT()" we follow the same code path in RegionInitBoxes and therefore would most likely cause the same problems as with a BAD_RECT() as before.

glamor_compute_transform_clipped_regions() would return a NULL region [3] and we would be in the same case as with a null region that causes further problems down the line.

But I have no idea how likely this is to happen :)

Cheers,
Olivier

[1] https://cgit.freedesktop.org/pixman/tree/pixman/pixman-region.c#n84
[2] https://cgit.freedesktop.org/pixman/tree/pixman/pixman-region.c#n374
[3] https://cgit.freedesktop.org/xorg/xserver/tree/glamor/glamor_largepixmap.c#n65