Re: [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for authenticated variable

["Dandan Bi" <dandan.bi@intel.com>]

The change is ok to me. Reviewed-by: Dandan Bi <dandan.bi@intel.com>

Hi Liming,

Could you also help review it?


Thanks,
Dandan
> -----Original Message-----
> From: Huang, Long1 <long1.huang@intel.com>
> Sent: Thursday, January 13, 2022 1:06 AM
> To: devel@edk2.groups.io
> Cc: Huang, Long1 <long1.huang@intel.com>; Feng, Bob C
> <bob.c.feng@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Bi,
> Dandan <dandan.bi@intel.com>; Chen, Lin Z <lin.z.chen@intel.com>; Li,
> Zhuangzhi <zhuangzhi.li@intel.com>
> Subject: [PATCH] MdeModulePkg/HiiDatabaseDxe: Add Support for
> authenticated variable
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3796
> 
> Database.c:
> 	1. Replace PcdGetExPtr with PcdGetExPtr.
> 	2. Add FindAuthVariableData function to parse authenticated
> variable type for getting a correct default value in
> PcdNvStoreDefaultValueBuffer.
> 
> Signed-off-by: Huang Long <long1.huang@intel.com>
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Lin Z Chen <lin.z.chen@intel.com>
> Cc: Zhuangzhi Li <zhuangzhi.li@intel.com>
> ---
>  .../Universal/HiiDatabaseDxe/Database.c       | 130 ++++++++++++++----
>  .../HiiDatabaseDxe/HiiDatabaseDxe.inf         |   3 +
>  2 files changed, 105 insertions(+), 28 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> index 0b09c24d52..c7a92d6aed 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> @@ -603,6 +603,45 @@ FindVariableData (
>    return NULL;
> 
>  }
> 
> 
> 
> +/**
> 
> +  Find the matched authenticated variable from the input variable storage.
> 
> +
> 
> +  @param[in] VariableStorage Point to the variable storage header.
> 
> +  @param[in] VarGuid         A unique identifier for the variable.
> 
> +  @param[in] VarAttribute    The attributes bitmask for the variable.
> 
> +  @param[in] VarName         A Null-terminated ascii string that is the name of
> the variable.
> 
> +
> 
> +  @return Pointer to the matched variable header or NULL if not found.
> 
> +**/
> 
> +AUTHENTICATED_VARIABLE_HEADER *
> 
> +FindAuthVariableData (
> 
> +  IN  VARIABLE_STORE_HEADER  *VariableStorage,
> 
> +  IN  EFI_GUID               *VarGuid,
> 
> +  IN  UINT32                 VarAttribute,
> 
> +  IN  CHAR16                 *VarName
> 
> +  )
> 
> +{
> 
> +  AUTHENTICATED_VARIABLE_HEADER  *AuthVariableHeader;
> 
> +  AUTHENTICATED_VARIABLE_HEADER  *AuthVariableEnd;
> 
> +
> 
> +  AuthVariableEnd    = (AUTHENTICATED_VARIABLE_HEADER *)((UINT8
> *)VariableStorage + VariableStorage->Size);
> 
> +  AuthVariableHeader = (AUTHENTICATED_VARIABLE_HEADER
> *)(VariableStorage + 1);
> 
> +  AuthVariableHeader = (AUTHENTICATED_VARIABLE_HEADER
> *)HEADER_ALIGN (AuthVariableHeader);
> 
> +  while (AuthVariableHeader < AuthVariableEnd) {
> 
> +    if (CompareGuid (&AuthVariableHeader->VendorGuid, VarGuid) &&
> 
> +        (AuthVariableHeader->Attributes == VarAttribute) &&
> 
> +        (StrCmp (VarName, (CHAR16 *)(AuthVariableHeader + 1)) == 0))
> 
> +    {
> 
> +      return AuthVariableHeader;
> 
> +    }
> 
> +
> 
> +    AuthVariableHeader = (AUTHENTICATED_VARIABLE_HEADER *)((UINT8
> *)AuthVariableHeader + sizeof (AUTHENTICATED_VARIABLE_HEADER) +
> AuthVariableHeader->NameSize + AuthVariableHeader->DataSize);
> 
> +    AuthVariableHeader = (AUTHENTICATED_VARIABLE_HEADER
> *)HEADER_ALIGN (AuthVariableHeader);
> 
> +  }
> 
> +
> 
> +  return NULL;
> 
> +}
> 
> +
> 
>  /**
> 
>    Find question default value from PcdNvStoreDefaultValueBuffer
> 
> 
> 
> @@ -626,25 +665,29 @@ FindQuestionDefaultSetting (
>    IN  BOOLEAN                  BitFieldQuestion
> 
>    )
> 
>  {
> 
> -  VARIABLE_HEADER          *VariableHeader;
> 
> -  VARIABLE_STORE_HEADER    *VariableStorage;
> 
> -  LIST_ENTRY               *Link;
> 
> -  VARSTORAGE_DEFAULT_DATA  *Entry;
> 
> -  VARIABLE_STORE_HEADER    *NvStoreBuffer;
> 
> -  UINT8                    *DataBuffer;
> 
> -  UINT8                    *BufferEnd;
> 
> -  BOOLEAN                  IsFound;
> 
> -  UINTN                    Index;
> 
> -  UINT32                   BufferValue;
> 
> -  UINT32                   BitFieldVal;
> 
> -  UINTN                    BitOffset;
> 
> -  UINTN                    ByteOffset;
> 
> -  UINTN                    BitWidth;
> 
> -  UINTN                    StartBit;
> 
> -  UINTN                    EndBit;
> 
> -  PCD_DEFAULT_DATA         *DataHeader;
> 
> -  PCD_DEFAULT_INFO         *DefaultInfo;
> 
> -  PCD_DATA_DELTA           *DeltaData;
> 
> +  VARIABLE_HEADER                   *VariableHeader;
> 
> +  AUTHENTICATED_VARIABLE_HEADER     *AuthVariableHeader;
> 
> +  VARIABLE_STORE_HEADER             *VariableStorage;
> 
> +  LIST_ENTRY                        *Link;
> 
> +  VARSTORAGE_DEFAULT_DATA           *Entry;
> 
> +  VARIABLE_STORE_HEADER             *NvStoreBuffer;
> 
> +  VOID                              *ValueSource;
> 
> +  VOID                              *BitValueSource;
> 
> +  UINT8                             *DataBuffer;
> 
> +  UINT8                             *BufferEnd;
> 
> +  BOOLEAN                           AuthFormat;
> 
> +  BOOLEAN                           IsFound;
> 
> +  UINTN                             Index;
> 
> +  UINT32                            BufferValue;
> 
> +  UINT32                            BitFieldVal;
> 
> +  UINTN                             BitOffset;
> 
> +  UINTN                             ByteOffset;
> 
> +  UINTN                             BitWidth;
> 
> +  UINTN                             StartBit;
> 
> +  UINTN                             EndBit;
> 
> +  PCD_DEFAULT_DATA                  *DataHeader;
> 
> +  PCD_DEFAULT_INFO                  *DefaultInfo;
> 
> +  PCD_DATA_DELTA                    *DeltaData;
> 
> 
> 
>    if (gSkuId == 0xFFFFFFFFFFFFFFFF) {
> 
>      gSkuId = LibPcdGetSku ();
> 
> @@ -666,7 +709,7 @@ FindQuestionDefaultSetting (
>    }
> 
> 
> 
>    if (Link == &gVarStorageList) {
> 
> -    DataBuffer          = (UINT8 *)PcdGetPtr (PcdNvStoreDefaultValueBuffer);
> 
> +    DataBuffer          = (UINT8 *)PcdGetExPtr
> (&gEfiMdeModulePkgTokenSpaceGuid, PcdNvStoreDefaultValueBuffer);
> 
>      gNvDefaultStoreSize = ((PCD_NV_STORE_DEFAULT_BUFFER_HEADER
> *)DataBuffer)->Length;
> 
>      //
> 
>      // The first section data includes NV storage default setting.
> 
> @@ -750,12 +793,27 @@ FindQuestionDefaultSetting (
>      return EFI_NOT_FOUND;
> 
>    }
> 
> 
> 
> +  //
> 
> +  // Judge if the variable type is authenticated, default is false
> 
> +  //
> 
> +  AuthFormat = FALSE;
> 
> +  if (CompareGuid (&VariableStorage->Signature,
> &gEfiAuthenticatedVariableGuid)) {
> 
> +    AuthFormat = TRUE;
> 
> +  }
> 
> +
> 
>    //
> 
>    // Find the question default value from the variable storage
> 
>    //
> 
> -  VariableHeader = FindVariableData (VariableStorage, &EfiVarStore->Guid,
> EfiVarStore->Attributes, (CHAR16 *)EfiVarStore->Name);
> 
> -  if (VariableHeader == NULL) {
> 
> -    return EFI_NOT_FOUND;
> 
> +  if(AuthFormat) {
> 
> +    AuthVariableHeader = FindAuthVariableData (VariableStorage,
> &EfiVarStore->Guid, EfiVarStore->Attributes, (CHAR16 *)EfiVarStore-
> >Name);
> 
> +    if (AuthVariableHeader == NULL) {
> 
> +      return EFI_NOT_FOUND;
> 
> +    }
> 
> +  } else {
> 
> +    VariableHeader = FindVariableData (VariableStorage, &EfiVarStore->Guid,
> EfiVarStore->Attributes, (CHAR16 *)EfiVarStore->Name);
> 
> +    if (VariableHeader == NULL) {
> 
> +      return EFI_NOT_FOUND;
> 
> +    }
> 
>    }
> 
> 
> 
>    StartBit   = 0;
> 
> @@ -770,8 +828,24 @@ FindQuestionDefaultSetting (
>      Width      = EndBit / 8 + 1;
> 
>    }
> 
> 
> 
> -  if (VariableHeader->DataSize < ByteOffset + Width) {
> 
> -    return EFI_INVALID_PARAMETER;
> 
> +  if (AuthFormat) {
> 
> +    if (AuthVariableHeader->DataSize < ByteOffset + Width) {
> 
> +      return EFI_INVALID_PARAMETER;
> 
> +    }
> 
> +
> 
> +    ValueSource = (UINT8 *)AuthVariableHeader + sizeof
> (AUTHENTICATED_VARIABLE_HEADER) + AuthVariableHeader->NameSize +
> IfrQuestionHdr->VarStoreInfo.VarOffset;
> 
> +    if (BitFieldQuestion) {
> 
> +      BitValueSource = (UINT8 *)AuthVariableHeader + sizeof
> (AUTHENTICATED_VARIABLE_HEADER) + AuthVariableHeader->NameSize +
> ByteOffset;
> 
> +    }
> 
> +  } else {
> 
> +    if (VariableHeader->DataSize < ByteOffset + Width) {
> 
> +      return EFI_INVALID_PARAMETER;
> 
> +    }
> 
> +
> 
> +    ValueSource = (UINT8 *)VariableHeader + sizeof (VARIABLE_HEADER) +
> VariableHeader->NameSize + IfrQuestionHdr->VarStoreInfo.VarOffset;
> 
> +    if (BitFieldQuestion) {
> 
> +      BitValueSource = (UINT8 *)VariableHeader + sizeof (VARIABLE_HEADER)
> + VariableHeader->NameSize + ByteOffset;
> 
> +    }
> 
>    }
> 
> 
> 
>    //
> 
> @@ -779,11 +853,11 @@ FindQuestionDefaultSetting (
>    //
> 
>    if (ValueBuffer != NULL) {
> 
>      if (BitFieldQuestion) {
> 
> -      CopyMem (&BufferValue, (UINT8 *)VariableHeader + sizeof
> (VARIABLE_HEADER) + VariableHeader->NameSize + ByteOffset, Width);
> 
> +      CopyMem (&BufferValue, BitValueSource, Width);
> 
>        BitFieldVal = BitFieldRead32 (BufferValue, StartBit, EndBit);
> 
>        CopyMem (ValueBuffer, &BitFieldVal, Width);
> 
>      } else {
> 
> -      CopyMem (ValueBuffer, (UINT8 *)VariableHeader + sizeof
> (VARIABLE_HEADER) + VariableHeader->NameSize + IfrQuestionHdr-
> >VarStoreInfo.VarOffset, Width);
> 
> +      CopyMem (ValueBuffer, ValueSource, Width);
> 
>      }
> 
>    }
> 
> 
> 
> @@ -832,7 +906,7 @@ UpdateDefaultSettingInFormPackage (
>    // If no default setting, do nothing
> 
>    //
> 
>    if (gNvDefaultStoreSize == 0) {
> 
> -    gNvDefaultStoreSize = PcdGetSize (PcdNvStoreDefaultValueBuffer);
> 
> +    gNvDefaultStoreSize = PcdGetExSize
> (&gEfiMdeModulePkgTokenSpaceGuid, PcdNvStoreDefaultValueBuffer);
> 
>    }
> 
> 
> 
>    if (gNvDefaultStoreSize < sizeof
> (PCD_NV_STORE_DEFAULT_BUFFER_HEADER)) {
> 
> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> index 0116fb6ecb..dac4d614a8 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
> @@ -86,6 +86,9 @@
>    gEfiHiiImageDecoderNameJpegGuid
> |gEfiMdeModulePkgTokenSpaceGuid.PcdSupportHiiImageProtocol  ##
> SOMETIMES_CONSUMES ## GUID
> 
>    gEfiHiiImageDecoderNamePngGuid
> |gEfiMdeModulePkgTokenSpaceGuid.PcdSupportHiiImageProtocol  ##
> SOMETIMES_CONSUMES ## GUID
> 
>    gEdkiiIfrBitVarstoreGuid                                                                    ##
> SOMETIMES_CONSUMES ## GUID
> 
> +  gEfiAuthenticatedVariableGuid
> 
> +  gEfiVariableGuid
> 
> +  gEfiMdeModulePkgTokenSpaceGuid
> 
> 
> 
>  [Depex]
> 
>    TRUE
> 
> --
> 2.25.1