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

["Min Xu" <min.m.xu@intel.com>]

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]
-=-=-=-=-=-=-=-=-=-=-=-