| Message ID | cb06a932a7c6b29bc6eb0d35f31766bfb0bba47f.1480962799.git.alistair.francis@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
On Tue, Dec 6, 2016 at 1:31 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > 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? 1 is valid, it is bypass mode, fixing. Thanks, Alistair > > thanks > -- PMM
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) { + /* Value is invalid, abort */ + return 1; + } + uart_parameters_setup(s); uart_update_status(s); return 0;
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(+)