[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] acpi/erst: fix fallthrough code upon validation failure
From: |
Michael S. Tsirkin |
Subject: |
Re: [PATCH] acpi/erst: fix fallthrough code upon validation failure |
Date: |
Fri, 10 Jun 2022 07:27:40 -0400 |
On Fri, Jun 10, 2022 at 01:48:57PM +0530, Ani Sinha wrote:
> On Thu, May 19, 2022 at 6:35 PM Eric DeVolder <eric.devolder@oracle.com>
> wrote:
> >
> >
> >
> > On 5/13/22 09:10, Ani Sinha wrote:
> > > At any step when any validation fail in check_erst_backend_storage(),
> > > there is
> > > no need to continue further through other validation checks. Further, by
> > > continuing even when record_size is 0, we run the risk of triggering a
> > > divide
> > > by zero error if we continued with other validation checks. Hence, we
> > > should
> > > simply return from this function upon validation failure.
> > >
> > > CC: Peter Maydell <peter.maydell@linaro.org>
> > > CC: Eric DeVolder <eric.devolder@oracle.com>
> > > Signed-off-by: Ani Sinha <ani@anisinha.ca>
> >
> > Reviewed-by: Eric DeVolder <eric.devolder@oracle.com>
>
> Michael:
> Did you forget to queue this for your latest pull request?
>
I did, thanks for the reminder!
> >
> > My apologies, I've been away for the past week.
> > Thank you for taking the time to correct this issue!
> > eric
> >
> > > ---
> > > hw/acpi/erst.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/hw/acpi/erst.c b/hw/acpi/erst.c
> > > index de509c2b48..df856b2669 100644
> > > --- a/hw/acpi/erst.c
> > > +++ b/hw/acpi/erst.c
> > > @@ -440,6 +440,7 @@ static void
> > > check_erst_backend_storage(ERSTDeviceState *s, Error **errp)
> > > (record_size >= 4096) /* PAGE_SIZE */
> > > )) {
> > > error_setg(errp, "ERST record_size %u is invalid", record_size);
> > > + return;
> > > }
> > >
> > > /* Validity check header */
> > > @@ -450,6 +451,7 @@ static void
> > > check_erst_backend_storage(ERSTDeviceState *s, Error **errp)
> > > (le16_to_cpu(header->reserved) == 0)
> > > )) {
> > > error_setg(errp, "ERST backend storage header is invalid");
> > > + return;
> > > }
> > >
> > > /* Check storage_size against record_size */
> > > @@ -457,6 +459,7 @@ static void
> > > check_erst_backend_storage(ERSTDeviceState *s, Error **errp)
> > > (record_size > s->storage_size)) {
> > > error_setg(errp, "ACPI ERST requires storage size be multiple
> > > of "
> > > "record size (%uKiB)", record_size);
> > > + return;
> > > }
> > >
> > > /* Compute offset of first and last record storage slot */
> > >