Commit d3532a0d authored by Gerd Hoffmann's avatar Gerd Hoffmann
Browse files

cirrus: fix blit region check 



Issues:
 * Doesn't check pitches correctly in case it is negative.
 * Doesn't check width at all.

Turn macro into functions while being at it, also factor out the check
for one region which we then can simply call twice for src + dst.

This is CVE-2014-8106.

Reported-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
Signed-off-by: default avatarGerd Hoffmann <kraxel@redhat.com>
Reviewed-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent db12451d

hw/display/cirrus_vga.c

+44 −17
Original line number Diff line number Diff line
@@ -173,20 +173,6 @@ 


#define CIRRUS_PNPMMIO_SIZE         0x1000



#define BLTUNSAFE(s) \

    ( \

        ( /* check dst is within bounds */ \

            (s)->cirrus_blt_height * ABS((s)->cirrus_blt_dstpitch) \

                + ((s)->cirrus_blt_dstaddr & (s)->cirrus_addr_mask) > \

                    (s)->vga.vram_size \

        ) || \

        ( /* check src is within bounds */ \

            (s)->cirrus_blt_height * ABS((s)->cirrus_blt_srcpitch) \

                + ((s)->cirrus_blt_srcaddr & (s)->cirrus_addr_mask) > \

                    (s)->vga.vram_size \

        ) \

    )



struct CirrusVGAState;

typedef void (*cirrus_bitblt_rop_t) (struct CirrusVGAState *s,

                                     uint8_t * dst, const uint8_t * src,
@@ -279,6 +265,46 @@ static void cirrus_update_memory_access(CirrusVGAState *s); 
 *

 ***************************************/



static bool blit_region_is_unsafe(struct CirrusVGAState *s,

                                  int32_t pitch, int32_t addr)

{

    if (pitch < 0) {

        int64_t min = addr

            + ((int64_t)s->cirrus_blt_height-1) * pitch;

        int32_t max = addr

            + s->cirrus_blt_width;

        if (min < 0 || max >= s->vga.vram_size) {

            return true;

        }

    } else {

        int64_t max = addr

            + ((int64_t)s->cirrus_blt_height-1) * pitch

            + s->cirrus_blt_width;

        if (max >= s->vga.vram_size) {

            return true;

        }

    }

    return false;

}



static bool blit_is_unsafe(struct CirrusVGAState *s)

{

    /* should be the case, see cirrus_bitblt_start */

    assert(s->cirrus_blt_width > 0);

    assert(s->cirrus_blt_height > 0);



    if (blit_region_is_unsafe(s, s->cirrus_blt_dstpitch,

                              s->cirrus_blt_dstaddr & s->cirrus_addr_mask)) {

        return true;

    }

    if (blit_region_is_unsafe(s, s->cirrus_blt_srcpitch,

                              s->cirrus_blt_srcaddr & s->cirrus_addr_mask)) {

        return true;

    }



    return false;

}



static void cirrus_bitblt_rop_nop(CirrusVGAState *s,

                                  uint8_t *dst,const uint8_t *src,

                                  int dstpitch,int srcpitch,
@@ -636,7 +662,7 @@ static int cirrus_bitblt_common_patterncopy(CirrusVGAState * s, 


    dst = s->vga.vram_ptr + (s->cirrus_blt_dstaddr & s->cirrus_addr_mask);



    if (BLTUNSAFE(s))

    if (blit_is_unsafe(s))

        return 0;



    (*s->cirrus_rop) (s, dst, src,
@@ -654,8 +680,9 @@ static int cirrus_bitblt_solidfill(CirrusVGAState *s, int blt_rop) 
{

    cirrus_fill_t rop_func;



    if (BLTUNSAFE(s))

    if (blit_is_unsafe(s)) {

        return 0;

    }

    rop_func = cirrus_fill[rop_to_index[blt_rop]][s->cirrus_blt_pixelwidth - 1];

    rop_func(s, s->vga.vram_ptr + (s->cirrus_blt_dstaddr & s->cirrus_addr_mask),

             s->cirrus_blt_dstpitch,
@@ -752,7 +779,7 @@ static void cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h) 


static int cirrus_bitblt_videotovideo_copy(CirrusVGAState * s)

{

    if (BLTUNSAFE(s))

    if (blit_is_unsafe(s))

        return 0;



    cirrus_do_copy(s, s->cirrus_blt_dstaddr - s->vga.start_addr,