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 <ofour...@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.
So, you're right (of course), I looked further into this and an empty region *should* work, only problem is glamor_composite_clipped_region() will fail if the source is NULL (which is the case as COMPOSITE_REGION() will pass NULL if the regions is empty), so we either hit a null pointer dereference in glamor_composite_choose_shader() or an assert(0) in the COMPOSITE_REGION() macro. But the fix for this is trivial, as the code is supposed to be able to pass an empty region, glamor_composite_clipped_region() should be nice enough to not call glamor_composite_with_shader() if the source is NULL and return OK in this case (as this should be OK according to the rest of the caller stack). Will post a third patch that does this (tested with the image provided in bug 101894 with my overflow patch reverted to make sure we have an empty region). Cheers, Olivier _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
