hw/acpi/erst.c | 3 +++ 1 file changed, 3 insertions(+)
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>
---
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 */
--
2.25.1
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> 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 */ >
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? > > 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 */ > >
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 */ > > >
On Fri, 13 May 2022 19:40:05 +0530 Ani Sinha <ani@anisinha.ca> 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: Igor Mammedov <imammedo@redhat.com> > --- > 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 */
© 2016 - 2024 Red Hat, Inc.