[edk2-devel] [PATCH V2 1/1] OvmfPkg/AcpiPlatformDxe: Fix Coverity report issues

[edk2-devel] [PATCH V2 1/1] OvmfPkg/AcpiPlatformDxe: Fix Coverity report issues

[sunceping]

From: Ceping Sun <cepingx.sun@intel.com>

v1 -> v2 Changed list:
 1:Since both commits are intended to fix coverity issues, they are merged into one
 2: Changed the debug info level to debug error when "DsdtTable == NULL"
 3:Add the Cc member as below
  Erdem Aktas erdemaktas@google.com
  James Bottomley jejb@linux.ibm.com
  Tom Lendacky thomas.lendacky@amd.com
  Michael Roth michael.roth@amd.com

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4568

The function InstallCloudHvTablesTdx had an Assert when "DsdtTable == NULL",
but this comes into play only in DEBUG mode. In Release mode , there is
no handling if the pointer is NULL. To avoid the possible null pointer
dereference, it is better to handle it when the pointer is null.

In addition, the status of "AcpiProtocol->InstallAcpiTable" is overwritten before
it can be used in the function, it is better to check it before overwriting.

code: https://github.com/sunceping/edk2/tree/fixcoverityerrors.v2

Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Michael Roth <michael.roth@amd.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Ceping Sun <cepingx.sun@intel.com>
---
 OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
index d3e73c155e8f..4629fa260366 100644
--- a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
@@ -53,6 +53,11 @@ InstallCloudHvTablesTdx (
                                CurrentTable->Length,
                                &TableHandle
                                );
+      if (EFI_ERROR (Status)) {
+        ASSERT_EFI_ERROR (Status);
+        return Status;
+      }
+
       for (UINTN i = 0; i < CurrentTable->Length; i++) {
         DEBUG ((DEBUG_INFO, " %x", *((UINT8 *)CurrentTable + i)));
       }
@@ -69,8 +74,9 @@ InstallCloudHvTablesTdx (
   // then we're out of sync with the hypervisor, and cannot continue.
   //
   if (DsdtTable == NULL) {
-    DEBUG ((DEBUG_INFO, "%a: no DSDT found\n", __func__));
+    DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __func__));
     ASSERT (FALSE);
+    CpuDeadLoop ();
   }
 
   Status = AcpiProtocol->InstallAcpiTable (
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109905): https://edk2.groups.io/g/devel/message/109905
Mute This Topic: https://groups.io/mt/102131520/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2 1/1] OvmfPkg/AcpiPlatformDxe: Fix Coverity report issues

[Min Xu]

Reviewed-by: Min Xu <min.m.xu@intel.com>

> -----Original Message-----
> From: Sun, CepingX <cepingx.sun@intel.com>
> Sent: Monday, October 23, 2023 5:06 PM
> To: devel@edk2.groups.io
> Cc: Sun, CepingX <cepingx.sun@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Xu, Min M <min.m.xu@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Michael Roth
> <michael.roth@amd.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH V2 1/1] OvmfPkg/AcpiPlatformDxe: Fix Coverity report
> issues
> 
> From: Ceping Sun <cepingx.sun@intel.com>
> 
> v1 -> v2 Changed list:
>  1:Since both commits are intended to fix coverity issues, they are merged
> into one
>  2: Changed the debug info level to debug error when "DsdtTable == NULL"
>  3:Add the Cc member as below
>   Erdem Aktas erdemaktas@google.com
>   James Bottomley jejb@linux.ibm.com
>   Tom Lendacky thomas.lendacky@amd.com
>   Michael Roth michael.roth@amd.com
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4568
> 
> The function InstallCloudHvTablesTdx had an Assert when "DsdtTable ==
> NULL", but this comes into play only in DEBUG mode. In Release mode ,
> there is no handling if the pointer is NULL. To avoid the possible null pointer
> dereference, it is better to handle it when the pointer is null.
> 
> In addition, the status of "AcpiProtocol->InstallAcpiTable" is overwritten
> before it can be used in the function, it is better to check it before
> overwriting.
> 
> code: https://github.com/sunceping/edk2/tree/fixcoverityerrors.v2
> 
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Michael Roth <michael.roth@amd.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Ceping Sun <cepingx.sun@intel.com>
> ---
>  OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> index d3e73c155e8f..4629fa260366 100644
> --- a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> +++ b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> @@ -53,6 +53,11 @@ InstallCloudHvTablesTdx (
>                                 CurrentTable->Length,
>                                 &TableHandle
>                                 );
> +      if (EFI_ERROR (Status)) {
> +        ASSERT_EFI_ERROR (Status);
> +        return Status;
> +      }
> +
>        for (UINTN i = 0; i < CurrentTable->Length; i++) {
>          DEBUG ((DEBUG_INFO, " %x", *((UINT8 *)CurrentTable + i)));
>        }
> @@ -69,8 +74,9 @@ InstallCloudHvTablesTdx (
>    // then we're out of sync with the hypervisor, and cannot continue.
>    //
>    if (DsdtTable == NULL) {
> -    DEBUG ((DEBUG_INFO, "%a: no DSDT found\n", __func__));
> +    DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __func__));
>      ASSERT (FALSE);
> +    CpuDeadLoop ();
>    }
> 
>    Status = AcpiProtocol->InstallAcpiTable (
> --
> 2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110013): https://edk2.groups.io/g/devel/message/110013
Mute This Topic: https://groups.io/mt/102131520/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2 1/1] OvmfPkg/AcpiPlatformDxe: Fix Coverity report issues

[Laszlo Ersek]

On 10/23/23 11:05, sunceping wrote:
> From: Ceping Sun <cepingx.sun@intel.com>
> 
> v1 -> v2 Changed list:
>  1:Since both commits are intended to fix coverity issues, they are merged into one
>  2: Changed the debug info level to debug error when "DsdtTable == NULL"
>  3:Add the Cc member as below
>   Erdem Aktas erdemaktas@google.com
>   James Bottomley jejb@linux.ibm.com
>   Tom Lendacky thomas.lendacky@amd.com
>   Michael Roth michael.roth@amd.com
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4568
> 
> The function InstallCloudHvTablesTdx had an Assert when "DsdtTable == NULL",
> but this comes into play only in DEBUG mode. In Release mode , there is
> no handling if the pointer is NULL. To avoid the possible null pointer
> dereference, it is better to handle it when the pointer is null.
> 
> In addition, the status of "AcpiProtocol->InstallAcpiTable" is overwritten before
> it can be used in the function, it is better to check it before overwriting.
> 
> code: https://github.com/sunceping/edk2/tree/fixcoverityerrors.v2
> 
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Michael Roth <michael.roth@amd.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Ceping Sun <cepingx.sun@intel.com>
> ---
>  OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> index d3e73c155e8f..4629fa260366 100644
> --- a/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> +++ b/OvmfPkg/AcpiPlatformDxe/CloudHvAcpi.c
> @@ -53,6 +53,11 @@ InstallCloudHvTablesTdx (
>                                 CurrentTable->Length,
>                                 &TableHandle
>                                 );
> +      if (EFI_ERROR (Status)) {
> +        ASSERT_EFI_ERROR (Status);
> +        return Status;
> +      }
> +
>        for (UINTN i = 0; i < CurrentTable->Length; i++) {
>          DEBUG ((DEBUG_INFO, " %x", *((UINT8 *)CurrentTable + i)));
>        }
> @@ -69,8 +74,9 @@ InstallCloudHvTablesTdx (
>    // then we're out of sync with the hypervisor, and cannot continue.
>    //
>    if (DsdtTable == NULL) {
> -    DEBUG ((DEBUG_INFO, "%a: no DSDT found\n", __func__));
> +    DEBUG ((DEBUG_ERROR, "%a: no DSDT found\n", __func__));
>      ASSERT (FALSE);
> +    CpuDeadLoop ();
>    }
>  
>    Status = AcpiProtocol->InstallAcpiTable (

merged via <https://github.com/tianocore/edk2/pull/4954> as commit
cf87fd95c1f5be4880a015c82a18e8ae12ff5e94



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110030): https://edk2.groups.io/g/devel/message/110030
Mute This Topic: https://groups.io/mt/102131520/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-