Re: [edk2-devel] [PATCH v1 06/19] DynamicTablesPkg: Fix GT Block length assignment

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 8/23/19 12:55 PM, Sami Mujawar wrote:
> The VS2017 compiler reports 'warning C4267: '=': conversion from
> 'size_t' to 'UINT16', possible loss of data'.
> 
> The sizeof() operator is used to calculate the size of the
> GT Block structure. The length field in the GT Block structure
> is 16-bit wide. Since the return type of sizeof() operator
> is size_t the VS2017 compiler reports the above warning.
> 
> To fix the warning, an explicit type cast is added. An additional
> check is also performed to ensure that the calculated GT Block
> length does not exceed MAX_UINT16.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>   DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c | 25 ++++++++++++++++----
>   1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
> index 7e86c30649bd36ecebac75c7e3a86a0c25cd590b..0e996698887aefca8a7240fc27fe3cb9324fd3e2 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/GtdtGenerator.c
> @@ -350,6 +350,7 @@ AddGTBlockList (
>     EFI_ACPI_6_3_GTDT_GT_BLOCK_TIMER_STRUCTURE  * GtBlockFrame;
>     CM_ARM_GTBLOCK_TIMER_FRAME_INFO             * GTBlockTimerFrameList;
>     UINT32                                        GTBlockTimerFrameCount;
> +  UINTN                                         Length;
>   
>     ASSERT (Gtdt != NULL);
>     ASSERT (GTBlockInfo != NULL);
> @@ -376,11 +377,27 @@ AddGTBlockList (
>         return Status;
>       }
>   
> +    Length = sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_STRUCTURE) +
> +               (sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_TIMER_STRUCTURE) *
> +                GTBlockInfo->GTBlockTimerFrameCount);
> +
> +    // Check that the length of the GT block does not
> +    // exceed MAX_UINT16
> +    if (Length > MAX_UINT16) {
> +      Status = EFI_INVALID_PARAMETER;
> +      DEBUG ((
> +        DEBUG_ERROR,
> +        "ERROR: GTDT: Too many GT Frames. Count = %d. " \
> +        "Maximum supported GT Block size exceeded. " \
> +        "Status = %r\n",
> +        GTBlockInfo->GTBlockTimerFrameCount,
> +        Status
> +        ));
> +      return Status;
> +    }
> +
>       GTBlock->Type = EFI_ACPI_6_3_GTDT_GT_BLOCK;
> -    GTBlock->Length = sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_STRUCTURE) +
> -                        (sizeof (EFI_ACPI_6_3_GTDT_GT_BLOCK_TIMER_STRUCTURE) *
> -                          GTBlockInfo->GTBlockTimerFrameCount);
> -
> +    GTBlock->Length = (UINT16)Length;
>       GTBlock->Reserved = EFI_ACPI_RESERVED_BYTE;
>       GTBlock->CntCtlBase = GTBlockInfo->GTBlockPhysicalAddress;
>       GTBlock->GTBlockTimerCount = GTBlockInfo->GTBlockTimerFrameCount;
> 

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>