On Sat, Nov 5, 2016 at 6:51 AM, Peter Maydell <peter.mayd...@linaro.org> wrote: > On 5 November 2016 at 00:00, Alistair Francis > <alistair.fran...@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 <ps...@huawei.com> >> Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com> >> --- >> >> hw/char/cadence_uart.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c >> index def34cd..e3a6248 100644 >> --- a/hw/char/cadence_uart.c >> +++ b/hw/char/cadence_uart.c >> @@ -487,6 +487,19 @@ 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) { >> + /* Value is invalid, reset it */ >> + s->r[R_BRGR] = 0x0000028B; >> + } >> + if (s->r[R_BDIV] <= 3) { >> + /* Value is invalid, reset it */ >> + s->r[R_BDIV] = 0x0000000F; >> + } >> + >> + s->r[R_BRGR] = s->r[R_BRGR] & 0xFFFF; >> + s->r[R_BDIV] = s->r[R_BDIV] & 0xFF; >> + >> uart_parameters_setup(s); >> uart_update_status(s); >> return 0; > > > Usually we just fail the migration if the incoming > data is bogus -- any particular reason not to take that > approach here?
There is no reason, it just seemed a bit much to abort just for this. Should I change it to abort? Thanks, Alistair > > thanks > -- PMM