qemu-devel
[Top][All Lists]
Advanced

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

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


From: Alistair Francis
Subject: Re: [Qemu-devel] [PATCH] cadence_uart: bounds check write offset
Date: Mon, 18 Apr 2016 13:50:26 -0700

On Mon, Apr 18, 2016 at 3:10 AM, Peter Maydell <address@hidden> wrote:
> CCing the maintainers for this device...
>
> On 18 April 2016 at 11:07, Michael S. Tsirkin <address@hidden> 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: 李强 <address@hidden>
>> Signed-off-by: Michael S. Tsirkin <address@hidden>

Good catch.

Reviewed-by: Alistair Francis <address@hidden>

Thanks,

Alistair

>>
>> ---
>>
>> 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
>



reply via email to

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