Re: [Qemu-devel] [PATCH] cadence_uart: bounds check write offset

[Peter Maydell <peter.maydell@linaro.org>]

CCing the maintainers for this device...

On 18 April 2016 at 11:07, Michael S. Tsirkin <mst@redhat.com> wrote:
> cadence_uart_init() initializes an I/O memory region of size 0x1000
> bytes.  However in uart_write(), the 'offset' parameter (offset within
> region) is divided by 4 and then used to index the array 'r' of size
> CADENCE_UART_R_MAX which is much smaller: (0x48/4).  If 'offset>>=2'
> exceeds CADENCE_UART_R_MAX, this will cause an out-of-bounds memory
> write where the offset and the value are controlled by guest.
>
> This will corrupt QEMU memory, in most situations this causes the vm to
> crash.
>
> Fix by checking the offset against the array size.
>
> Reported-by: 李强 <liqiang6-s@360.cn>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ---
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index 486591b..7977878 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -375,6 +375,9 @@ static void uart_write(void *opaque, hwaddr offset,
>
>      DB_PRINT(" offset:%x data:%08x\n", (unsigned)offset, (unsigned)value);
>      offset >>= 2;
> +    if (offset >= CADENCE_UART_R_MAX) {
> +        return;
> +    }
>      switch (offset) {
>      case R_IER: /* ier (wts imr) */
>          s->r[R_IMR] |= value;

thanks
-- PMM