Re: [Qemu-devel] [PATCH for 2.8 v3 1/1] cadence_uart: Check baud rate generator and divider values on migration

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

On 5 December 2016 at 18:35, Alistair Francis
<alistair.francis@xilinx.com> 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 <psirt@huawei.com>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
> It would be nice to squeeze this into 2.8 if possible.
>
> V3:
>  - Fix broken migration logic
>  - Manually double checked and it passes migration.
> 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 0215d65..ce9063b 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -502,6 +502,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) {

The uart_write() code says BRGR == 1 is valid, but
this code says it isn't. Which is correct?

thanks
-- PMM