[Pixman] [PATCH 5/7] armv7/mips/sse2: Fix bounds violations in bilinear cover scaled fast paths

[Siarhei Siamashka]

On Mon, 24 Aug 2015 21:42:04 +0100
Ben Avison <bavison at riscosopen.org> wrote:

> In functions named fast_composite_scaled_bilinear... as defined by
> FAST_BILINEAR_MAINLOOP_INT, there are multiple calls to scanline_func
> (a macro parameter) which is often implemented in assembly, and which
> takes as parameters two line buffer pointers (which are range checked)
> and fixed-point initial and incremental pixel offsets into those lines.
> 
> When handling edge cases, there are already implicit assumptions that
> scanline_func reads at most two input pixels horizontally for each
> output pixel. In practice, implementations typically read exactly two,
> even when the fractional part of the offset is zero; this leads to
> array bounds violations for the COVER case when the offset for the final
> pixel is integer (in other cases, such offsets move us into special
> handling for repeat zones and so are not affected).

The current COVER flag assumes that every pixel is fetched from the
source image as a 2x2 block (regardless of whether the fractional part
of the offset is zero or not) and this is explicitly expected to be
safe. Just compare the extents handling for nearest
    http://cgit.freedesktop.org/pixman/tree/pixman/pixman.c?id=pixman-0.33.2#n511
and bilinear cases:
    http://cgit.freedesktop.org/pixman/tree/pixman/pixman.c?id=pixman-0.33.2#n519

In the latter case, we get a reservation for one extra pixel to read
by doing "- pixman_fixed_1 / 2" and "+ pixman_fixed_1 / 2" adjustments.

However with your new proposed definition of the COVER_CLIP_BILINEAR
flag, the area is shrinked by "pixman_fixed_e" on the right side in
order to allow a special corner case to pass through. And this is where
the bounds violations are likely coming from. It is reserving not
exactly one full extra pixel, but only "pixman_fixed_1 - pixman_fixed_e"
space:
    http://lists.freedesktop.org/archives/pixman/2015-August/003874.html

Am I misunderstanding something? Is there actually a real bug in the
current pixman code? Because the commit summary looks kinda scary and
may be misinterpreted by the general public.

-- 
Best regards,
Siarhei Siamashka