On 20 January 2014 13:29, Alon Levy <al...@redhat.com> wrote:
> Fix signed to unsigned comparison in qxl_create_guest_primary and add
> the size of the framebuffer to the error message used when setting the
> guest bug state (which causes a complete guess blackout until reset, so
> it helps if it is verbose).
>
> Signed-off-by: Alon Levy <al...@redhat.com>
> ---
> hw/display/qxl.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index e4f172e..b799b51 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -1360,14 +1360,15 @@ static void qxl_create_guest_primary(PCIQXLDevice
> *qxl, int loadvm,
> {
> QXLDevSurfaceCreate surface;
> QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
> - int size;
> + uint32_t size;
> int requested_height = le32_to_cpu(sc->height);
> int requested_stride = le32_to_cpu(sc->stride);
>
> size = abs(requested_stride) * requested_height;
This looks a bit dubious -- the multiply is still going
to be done with signed arithmetic, so if it's possible
we might overflow so as to require a uint32_t size
then we've already hit undefined behaviour. Also, if
the multiply overflows 32 bits the check will not
catch this. Probably better to do this as a 64 bit
multiply.
Is requested_height really supposed to be signed?
Why is requested_stride an int that needs to go
through abs()? What is the behaviour supposed to be
if it is the minimum integer (in which case abs(x)
is undefined)?
> if (size > qxl->vgamem_size) {
> - qxl_set_guest_bug(qxl, "%s: requested primary larger then
> framebuffer"
> - " size", __func__);
> + qxl_set_guest_bug(qxl, "%s: requested primary larger than
> framebuffer"
> + " size %u > %u", __func__, size,
> + qxl->vgamem_size);
> return;
> }
PRIu32 is more portable for printing uint32_t types.
thanks
-- PMM
