qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v2 1/1] cadence_uart: Check baud rate generator


From: Alistair Francis
Subject: Re: [Qemu-devel] [PATCH v2 1/1] cadence_uart: Check baud rate generator and divider values on migration
Date: Mon, 28 Nov 2016 15:15:51 -0800

On Mon, Nov 28, 2016 at 3:31 AM, Peter Maydell <address@hidden> wrote:
> On 8 November 2016 at 00:34, Alistair Francis
> <address@hidden> wrote:
>> The Cadence UART device emulator calculates speed by dividing the
>> baud rate by a 'baud rate generator' & 'baud rate divider' value.
>> The device specification defines these register values to be
>> non-zero and within certain limits. Checks were recently added when
>> writing to these registers but not when restoring from migration.
>>
>> This patch adds checks when restoring from migration to avoid divide by
>> zero errors.
>>
>> Reported-by: Huawei PSIRT <address@hidden>
>> Signed-off-by: Alistair Francis <address@hidden>
>> ---
>> V2:
>>  - Abort the migration if the data is invalid
>>
>>  hw/char/cadence_uart.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
>> index def34cd..9568ac6 100644
>> --- a/hw/char/cadence_uart.c
>> +++ b/hw/char/cadence_uart.c
>> @@ -487,6 +487,13 @@ static int cadence_uart_post_load(void *opaque, int 
>> version_id)
>>  {
>>      CadenceUARTState *s = opaque;
>>
>> +    /* Ensure these two aren't invalid numbers */
>> +    if (s->r[R_BRGR] <= 1 || s->r[R_BRGR] & 0xFFFF ||
>> +        s->r[R_BDIV] <= 3 || s->r[R_BDIV] & 0xFF) {
>> +        /* Value is invalid, abort */
>> +        return 1;
>> +    }
>
> The "s->r[R_BRGR] & 0xFFFF" and "s->r[R_BDIV] & 0xFF" checks
> look wrong -- it's ok for the low bits to be set, it's only
> if high bits are set that we want to abort the migration.
> Missing '~'s ? (I'm surprised this bug doesn't cause migration
> to fail every time.)

Yep, you are right. Those are wrong. I'll fix them up.

I didn't have a migration test case to work with. I will set something
up this time to make sure something like that doesn't slip though.

Thanks,

Alistair

>
> thanks
> -- PMM



reply via email to

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