On Thu, May 19, 2022 at 6:35 PM Eric DeVolder <[email protected]> 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 <[email protected]>
> > CC: Eric DeVolder <[email protected]>
> > Signed-off-by: Ani Sinha <[email protected]>
>
> Reviewed-by: Eric DeVolder <[email protected]>
Michael:
Did you forget to queue this for your latest pull request?
>
> 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 */
> >
