qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 22/30] tsc210x: fix buffer overrun on invalid


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH v4 22/30] tsc210x: fix buffer overrun on invalid state load
Date: Mon, 31 Mar 2014 16:39:11 +0100

On 31 March 2014 15:17, Michael S. Tsirkin <address@hidden> wrote:
> CVE-2013-4539
>
> s->precision, nextprecision, function and nextfunction
> come from wire and are used
> as idx into resolution[] in TSC_CUT_RESOLUTION.
>
> Validate after load to avoid buffer overrun.
>
> Cc: Andreas Färber <address@hidden>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
>  hw/input/tsc210x.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
> index 485c9e5..65a0d08 100644
> --- a/hw/input/tsc210x.c
> +++ b/hw/input/tsc210x.c
> @@ -1070,9 +1070,21 @@ static int tsc210x_load(QEMUFile *f, void *opaque, int 
> version_id)
>      s->enabled = qemu_get_byte(f);
>      s->host_mode = qemu_get_byte(f);
>      s->function = qemu_get_byte(f);
> +    if (s->function > ARRAY_SIZE(mode_regs)) {
> +        return -EINVAL;
> +    }

Why no check for negative values? Also, shouldn't
this be >=, like the checks below?

>      s->nextfunction = qemu_get_byte(f);
> +    if (s->nextfunction < 0 || s->nextfunction >= ARRAY_SIZE(mode_regs)) {
> +        return -EINVAL;
> +    }
>      s->precision = qemu_get_byte(f);
> +    if (s->precision < 0 || s->precision >= ARRAY_SIZE(resolution)) {
> +        return -EINVAL;
> +    }
>      s->nextprecision = qemu_get_byte(f);
> +    if (s->nextprecision < 0 || s->nextprecision >= ARRAY_SIZE(resolution)) {
> +        return -EINVAL;
> +    }
>      s->filter = qemu_get_byte(f);
>      s->pin_func = qemu_get_byte(f);
>      s->ref = qemu_get_byte(f);
> --
> MST
>


thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]