Re: [PATCH EDK2 v1 1/1] ArmPlatformPkg/Drivers/NorFlashDxe:avoid index out of bound

["Ard Biesheuvel" <ardb@kernel.org>]

On Sat, 29 Oct 2022 at 11:29, Wenyi Xie <xiewenyi2@huawei.com> wrote:
>
> The size of array BlockMap is 1 in struct FirmwareVolumeHeader, but in
> function InitializeFvAndVariableStoreHeaders, BlockMap[1] is been written.

The size of BlockMap[] is not 1. BlockMap is a flexible array declared
in an old fashioned way, and because the type definition is covered by
the spec, we can not change it. Given that this is established idiom,
compilers don't tend to warn about this.

> The memory of BlockMap[1] is already allocated, so the code is OK. But
> it is better to use a new point to assign this memory.
>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Wenyi Xie <xiewenyi2@huawei.com>

Thanks for the patch but this driver is going to be deleted soon so no point.

There is new version of this driver in OvmfPkg/ but given the above, I
don't think there is anything that needs fixing here.

> ---
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c
> index 0767581308d2..2130e2e76344 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c
> @@ -47,6 +47,7 @@ InitializeFvAndVariableStoreHeaders (
>    VOID                        *Headers;
>    UINTN                       HeadersLength;
>    EFI_FIRMWARE_VOLUME_HEADER  *FirmwareVolumeHeader;
> +  EFI_FV_BLOCK_MAP_ENTRY      *BlockMapEntry;
>    VARIABLE_STORE_HEADER       *VariableStoreHeader;
>    UINT32                      NvStorageFtwSpareSize;
>    UINT32                      NvStorageFtwWorkingSize;
> @@ -151,10 +152,15 @@ InitializeFvAndVariableStoreHeaders (
>    FirmwareVolumeHeader->Revision              = EFI_FVH_REVISION;
>    FirmwareVolumeHeader->BlockMap[0].NumBlocks = Instance->Media.LastBlock + 1;
>    FirmwareVolumeHeader->BlockMap[0].Length    = Instance->Media.BlockSize;
> -  FirmwareVolumeHeader->BlockMap[1].NumBlocks = 0;
> -  FirmwareVolumeHeader->BlockMap[1].Length    = 0;
>    FirmwareVolumeHeader->Checksum              = CalculateCheckSum16 ((UINT16 *)FirmwareVolumeHeader, FirmwareVolumeHeader->HeaderLength);
>
> +  //
> +  // EFI_FV_BLOCK_MAP_ENTRY
> +  //
> +  BlockMapEntry            = (EFI_FV_BLOCK_MAP_ENTRY *)((UINTN)Headers + sizeof (EFI_FIRMWARE_VOLUME_HEADER));
> +  BlockMapEntry->NumBlocks = 0;
> +  BlockMapEntry->Length    = 0;
> +
>    //
>    // VARIABLE_STORE_HEADER
>    //
> --
> 2.20.1.windows.1
>