[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: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/1] cadence_uart: Check baud rate generator and divider values on migration |
Date: |
Mon, 28 Nov 2016 11:31:49 +0000 |
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.)
thanks
-- PMM