* [PATCH] acpi/erst: fix fallthrough code upon validation failure
@ 2022-05-13 14:10 Ani Sinha
2022-05-19 12:40 ` Igor Mammedov
2022-05-19 13:05 ` Eric DeVolder
0 siblings, 2 replies; 5+ messages in thread
From: Ani Sinha @ 2022-05-13 14:10 UTC (permalink / raw)
To: Michael S. Tsirkin, Igor Mammedov, Ani Sinha
Cc: Peter Maydell, Eric DeVolder, qemu-devel
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] acpi/erst: fix fallthrough code upon validation failure
2022-05-13 14:10 [PATCH] acpi/erst: fix fallthrough code upon validation failure Ani Sinha
@ 2022-05-19 12:40 ` Igor Mammedov
2022-05-19 13:05 ` Eric DeVolder
1 sibling, 0 replies; 5+ messages in thread
From: Igor Mammedov @ 2022-05-19 12:40 UTC (permalink / raw)
To: Ani Sinha; +Cc: Michael S. Tsirkin, Peter Maydell, Eric DeVolder, qemu-devel
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 */
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] acpi/erst: fix fallthrough code upon validation failure
2022-05-13 14:10 [PATCH] acpi/erst: fix fallthrough code upon validation failure Ani Sinha
2022-05-19 12:40 ` Igor Mammedov
@ 2022-05-19 13:05 ` Eric DeVolder
2022-06-10 8:18 ` Ani Sinha
1 sibling, 1 reply; 5+ messages in thread
From: Eric DeVolder @ 2022-05-19 13:05 UTC (permalink / raw)
To: Ani Sinha, Michael S. Tsirkin, Igor Mammedov; +Cc: Peter Maydell, qemu-devel
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 */
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] acpi/erst: fix fallthrough code upon validation failure
2022-05-19 13:05 ` Eric DeVolder
@ 2022-06-10 8:18 ` Ani Sinha
2022-06-10 11:27 ` Michael S. Tsirkin
0 siblings, 1 reply; 5+ messages in thread
From: Ani Sinha @ 2022-06-10 8:18 UTC (permalink / raw)
To: Eric DeVolder
Cc: Michael S. Tsirkin, Igor Mammedov, Peter Maydell, qemu-devel
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 */
> >
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] acpi/erst: fix fallthrough code upon validation failure
2022-06-10 8:18 ` Ani Sinha
@ 2022-06-10 11:27 ` Michael S. Tsirkin
0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2022-06-10 11:27 UTC (permalink / raw)
To: Ani Sinha; +Cc: Eric DeVolder, Igor Mammedov, Peter Maydell, qemu-devel
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 */
> > >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-06-10 11:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-13 14:10 [PATCH] acpi/erst: fix fallthrough code upon validation failure Ani Sinha
2022-05-19 12:40 ` Igor Mammedov
2022-05-19 13:05 ` Eric DeVolder
2022-06-10 8:18 ` Ani Sinha
2022-06-10 11:27 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).