On 31 March 2014 15:16, Michael S. Tsirkin <m...@redhat.com> wrote:
> CVE-2013-4530
>
> pl022.c did not bounds check tx_fifo_head and
> rx_fifo_head after loading them from file and
> before they are used to dereference array.
>
> Reported-by: Michael S. Tsirkin <m...@redhat.com
> Reported-by: Anthony Liguori <anth...@codemonkey.ws>
> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> ---
> hw/ssi/pl022.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/hw/ssi/pl022.c b/hw/ssi/pl022.c
> index fd479ef..49b3f61 100644
> --- a/hw/ssi/pl022.c
> +++ b/hw/ssi/pl022.c
> @@ -240,11 +240,23 @@ static const MemoryRegionOps pl022_ops = {
> .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> +static int pl022_post_load(void *opaque, int version_id)
> +{
> + PL022State *s = opaque;
> +
> + if (s->tx_fifo_head > ARRAY_SIZE(s->tx_fifo) ||
> + s->rx_fifo_head > ARRAY_SIZE(s->rx_fifo)) {
> + return -1;
Shouldn't these be '>=' checks? Also if the
incoming values are negative then we'll go
wrong in the other direction.
Given the way we do calculations involving *_fifo_len
as well, it might be best to also sanitize those,
though I think it's not possible currently to provoke
a buffer overrun with them. (NB that the _fifo_len
fields can validly be equal to the ARRAY_SIZE, unlike
the _fifo_head fields.)
thanks
-- PMM
