public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH EDK2 v1 0/1] ArmPlatformPkg/Drivers/NorFlashDxe:avoid index out of bound
@ 2022-10-29  9:28 wenyi,xie
  2022-10-29  9:28 ` [PATCH EDK2 v1 1/1] " wenyi,xie
  0 siblings, 1 reply; 4+ messages in thread
From: wenyi,xie @ 2022-10-29  9:28 UTC (permalink / raw)
  To: devel, quic_llindhol, ardb+tianocore; +Cc: songdongkuang, xiewenyi2

Main Changes :
1.Adding new point to assign the value of struct EFI_FV_BLOCK_MAP_ENTRY.

Wenyi Xie (1):
  ArmPlatformPkg/Drivers/NorFlashDxe:avoid index out of bound

 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

-- 
2.20.1.windows.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH EDK2 v1 1/1] ArmPlatformPkg/Drivers/NorFlashDxe:avoid index out of bound
  2022-10-29  9:28 [PATCH EDK2 v1 0/1] ArmPlatformPkg/Drivers/NorFlashDxe:avoid index out of bound wenyi,xie
@ 2022-10-29  9:28 ` wenyi,xie
  2022-10-30 10:06   ` Ard Biesheuvel
  0 siblings, 1 reply; 4+ messages in thread
From: wenyi,xie @ 2022-10-29  9:28 UTC (permalink / raw)
  To: devel, quic_llindhol, ardb+tianocore; +Cc: songdongkuang, xiewenyi2

The size of array BlockMap is 1 in struct FirmwareVolumeHeader, but in
function InitializeFvAndVariableStoreHeaders, BlockMap[1] is been written.
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>
---
 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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH EDK2 v1 1/1] ArmPlatformPkg/Drivers/NorFlashDxe:avoid index out of bound
  2022-10-29  9:28 ` [PATCH EDK2 v1 1/1] " wenyi,xie
@ 2022-10-30 10:06   ` Ard Biesheuvel
  2022-10-31  1:24     ` wenyi,xie
  0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2022-10-30 10:06 UTC (permalink / raw)
  To: Wenyi Xie; +Cc: devel, quic_llindhol, ardb+tianocore, songdongkuang

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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH EDK2 v1 1/1] ArmPlatformPkg/Drivers/NorFlashDxe:avoid index out of bound
  2022-10-30 10:06   ` Ard Biesheuvel
@ 2022-10-31  1:24     ` wenyi,xie
  0 siblings, 0 replies; 4+ messages in thread
From: wenyi,xie @ 2022-10-31  1:24 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: devel, quic_llindhol, ardb+tianocore, songdongkuang



On 2022/10/30 18:06, Ard Biesheuvel wrote:
> 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.

Thank you for your answering, I got your point and so it seems no necessary to change.

Best Regards
Wenyi
> 
>> ---
>>  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
>>
> .
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-10-31  1:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-29  9:28 [PATCH EDK2 v1 0/1] ArmPlatformPkg/Drivers/NorFlashDxe:avoid index out of bound wenyi,xie
2022-10-29  9:28 ` [PATCH EDK2 v1 1/1] " wenyi,xie
2022-10-30 10:06   ` Ard Biesheuvel
2022-10-31  1:24     ` wenyi,xie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox