public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: chenc2 <chen.a.chen@intel.com>, edk2-devel@lists.01.org
Cc: Wu Hao <hao.a.wu@intel.com>, Zhang Chao <chao.b.zhang@intel.com>
Subject: Re: [PATCH] SecurityPkg/SecureBootConfigDxe: Add check to avoid
Date: Wed, 18 Oct 2017 09:05:51 +0200	[thread overview]
Message-ID: <e9206f05-c85f-5325-5193-7e298a069f0d@redhat.com> (raw)
In-Reply-To: <20171018045001.16012-1-chen.a.chen@intel.com>

Hi,

On 10/18/17 06:50, chenc2 wrote:
> The function HiiGetString will return NULL pointer when
> the platform does not install the appropriate string or
> call HiiGetString fail.(For example, HII not support specified
> language.)
> 
> Cc: Zhang Chao <chao.b.zhang@intel.com>
> Cc: Wu Hao <hao.a.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: chenc2 <chen.a.chen@intel.com>
> ---
>  .../SecureBootConfigDxe/SecureBootConfigImpl.c     | 97 +++++++++++++++-------
>  1 file changed, 66 insertions(+), 31 deletions(-)

The subject line of this patch appears truncated.

Thanks
Laszlo

> diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
> index acb0dc0558..4ce5172701 100644
> --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
> +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
> @@ -3572,6 +3572,9 @@ LoadSignatureList (
>  {
>    EFI_STATUS            Status;
>    EFI_STRING_ID         ListType;
> +  EFI_STRING            FormatNameString;
> +  EFI_STRING            FormatHelpString;
> +  EFI_STRING            FormatTypeString;
>    EFI_SIGNATURE_LIST    *ListWalker;
>    EFI_IFR_GUID_LABEL    *StartLabel;
>    EFI_IFR_GUID_LABEL    *EndLabel;
> @@ -3705,6 +3708,12 @@ LoadSignatureList (
>      goto ON_EXIT;
>    }
>  
> +  FormatNameString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_NAME_FORMAT), NULL);
> +  FormatHelpString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_HELP_FORMAT), NULL);
> +  if (FormatNameString == NULL || FormatHelpString == NULL) {
> +    goto ON_EXIT;
> +  }
> +
>    RemainingSize = DataSize;
>    ListWalker    = (EFI_SIGNATURE_LIST *)VariableData;
>    while ((RemainingSize > 0) && (RemainingSize >= ListWalker->SignatureListSize)) {
> @@ -3725,21 +3734,23 @@ LoadSignatureList (
>      } else {
>        ListType = STRING_TOKEN (STR_LIST_TYPE_UNKNOWN);
>      }
> +    FormatTypeString = HiiGetString (PrivateData->HiiHandle, ListType, NULL);
> +    if (FormatTypeString == NULL) {
> +      goto ON_EXIT;
> +    }
>  
>      ZeroMem (NameBuffer, sizeof (NameBuffer));
> -    UnicodeSPrint (NameBuffer,
> -      sizeof (NameBuffer),
> -      HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_NAME_FORMAT), NULL),
> -      Index + 1
> -    );
> +    UnicodeSPrint (NameBuffer, sizeof (NameBuffer), FormatNameString, Index + 1);
>  
>      ZeroMem (HelpBuffer, sizeof (HelpBuffer));
>      UnicodeSPrint (HelpBuffer,
>        sizeof (HelpBuffer),
> -      HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_LIST_HELP_FORMAT), NULL),
> -      HiiGetString (PrivateData->HiiHandle, ListType, NULL),
> +      FormatHelpString,
> +      FormatTypeString,
>        SIGNATURE_DATA_COUNTS (ListWalker)
>      );
> +    SECUREBOOT_FREE_NON_NULL (FormatTypeString);
> +    FormatTypeString = NULL;
>  
>      HiiCreateGotoOpCode (
>        StartOpCodeHandle,
> @@ -3777,6 +3788,8 @@ ON_EXIT:
>    SECUREBOOT_FREE_NON_OPCODE (EndGotoHandle);
>  
>    SECUREBOOT_FREE_NON_NULL (VariableData);
> +  SECUREBOOT_FREE_NON_NULL (FormatNameString);
> +  SECUREBOOT_FREE_NON_NULL (FormatHelpString);
>  
>    PrivateData->ListCount = Index;
>  
> @@ -3922,6 +3935,8 @@ FormatHelpInfo (
>    EFI_STATUS      Status;
>    EFI_TIME        *Time;
>    EFI_STRING_ID   ListTypeId;
> +  EFI_STRING      FormatHelpString;
> +  EFI_STRING      FormatTypeString;
>    UINTN           DataSize;
>    UINTN           HelpInfoIndex;
>    UINTN           TotalSize;
> @@ -3969,6 +3984,11 @@ FormatHelpInfo (
>      goto ON_EXIT;
>    }
>  
> +  FormatTypeString = HiiGetString (PrivateData->HiiHandle, ListTypeId, NULL);
> +  if (FormatTypeString == NULL) {
> +    goto ON_EXIT;
> +  }
> +
>    TotalSize = 1024;
>    HelpInfoString = AllocateZeroPool (TotalSize);
>    if (HelpInfoString == NULL) {
> @@ -3981,40 +4001,45 @@ FormatHelpInfo (
>    //
>    ZeroMem (GuidString, sizeof (GuidString));
>    GuidToString(&DataEntry->SignatureOwner, GuidString, BUFFER_MAX_SIZE);
> +  FormatHelpString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_GUID), NULL);
> +  if (FormatHelpString == NULL) {
> +    goto ON_EXIT;
> +  }
>    HelpInfoIndex += UnicodeSPrint (
>                       &HelpInfoString[HelpInfoIndex],
>                       TotalSize - sizeof(CHAR16) * HelpInfoIndex,
> -                     HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_GUID), NULL),
> +                     FormatHelpString,
>                       GuidString
>                     );
> +  SECUREBOOT_FREE_NON_NULL (FormatHelpString);
> +  FormatHelpString = NULL;
>  
>    //
>    // Format content part, it depends on the type of signature list, hash value or CN.
>    //
>    if (IsCert) {
>      GetCommonNameFromX509 (ListEntry, DataEntry, &DataString);
> -    HelpInfoIndex += UnicodeSPrint(
> -                       &HelpInfoString[HelpInfoIndex],
> -                       TotalSize - sizeof(CHAR16) * HelpInfoIndex,
> -                       HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_CN), NULL),
> -                       HiiGetString (PrivateData->HiiHandle, ListTypeId, NULL),
> -                       DataSize,
> -                       DataString
> -                     );
> +    FormatHelpString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_CN), NULL);
>    } else {
>      //
>      //  Format hash value for each signature data entry.
>      //
>      ParseHashValue (ListEntry, DataEntry, &DataString);
> -    HelpInfoIndex += UnicodeSPrint (
> -                       &HelpInfoString[HelpInfoIndex],
> -                       TotalSize - sizeof(CHAR16) * HelpInfoIndex,
> -                       HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_HASH), NULL),
> -                       HiiGetString (PrivateData->HiiHandle, ListTypeId, NULL),
> -                       DataSize,
> -                       DataString
> -                     );
> +    FormatHelpString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_HASH), NULL);
> +  }
> +  if (FormatHelpString == NULL) {
> +    goto ON_EXIT;
>    }
> +  HelpInfoIndex += UnicodeSPrint (
> +                     &HelpInfoString[HelpInfoIndex],
> +                     TotalSize - sizeof (CHAR16) * HelpInfoIndex,
> +                     FormatHelpString,
> +                     FormatTypeString,
> +                     DataSize,
> +                     DataString
> +                   );
> +  SECUREBOOT_FREE_NON_NULL (FormatHelpString);
> +  FormatHelpString = NULL;
>  
>    //
>    // Format revocation time part.
> @@ -4032,13 +4057,18 @@ FormatHelpInfo (
>        Time->Minute,
>        Time->Second
>      );
> -
> +    FormatHelpString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_TIME), NULL);
> +    if (FormatHelpString == NULL) {
> +      goto ON_EXIT;
> +    }
>      UnicodeSPrint (
>        &HelpInfoString[HelpInfoIndex],
>        TotalSize - sizeof (CHAR16) * HelpInfoIndex,
> -      HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_HELP_FORMAT_TIME), NULL),
> +      FormatHelpString,
>        TimeString
>      );
> +    SECUREBOOT_FREE_NON_NULL (FormatHelpString);
> +    FormatHelpString = NULL;
>    }
>  
>    *StringId = HiiSetString (PrivateData->HiiHandle, 0, HelpInfoString, NULL);
> @@ -4046,6 +4076,8 @@ ON_EXIT:
>    SECUREBOOT_FREE_NON_NULL (DataString);
>    SECUREBOOT_FREE_NON_NULL (HelpInfoString);
>  
> +  SECUREBOOT_FREE_NON_NULL (FormatTypeString);
> +
>    return Status;
>  }
>  
> @@ -4076,6 +4108,7 @@ LoadSignatureData (
>    EFI_IFR_GUID_LABEL    *StartLabel;
>    EFI_IFR_GUID_LABEL    *EndLabel;
>    EFI_STRING_ID         HelpStringId;
> +  EFI_STRING            FormatNameString;
>    VOID                  *StartOpCodeHandle;
>    VOID                  *EndOpCodeHandle;
>    UINTN                 DataSize;
> @@ -4167,17 +4200,18 @@ LoadSignatureData (
>      ListWalker = (EFI_SIGNATURE_LIST *)((UINT8 *)ListWalker + ListWalker->SignatureListSize);
>    }
>  
> +  FormatNameString = HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_NAME_FORMAT), NULL);
> +  if (FormatNameString == NULL) {
> +    goto ON_EXIT;
> +  }
> +
>    DataWalker = (EFI_SIGNATURE_DATA *)((UINT8 *)ListWalker + sizeof(EFI_SIGNATURE_LIST) + ListWalker->SignatureHeaderSize);
>    for (Index = 0; Index < SIGNATURE_DATA_COUNTS(ListWalker); Index = Index + 1) {
>      //
>      // Format name buffer.
>      //
>      ZeroMem (NameBuffer, sizeof (NameBuffer));
> -    UnicodeSPrint (NameBuffer,
> -      sizeof (NameBuffer),
> -      HiiGetString (PrivateData->HiiHandle, STRING_TOKEN (STR_SIGNATURE_DATA_NAME_FORMAT), NULL),
> -      Index + 1
> -    );
> +    UnicodeSPrint (NameBuffer, sizeof (NameBuffer), FormatNameString, Index + 1);
>  
>      //
>      // Format help info buffer.
> @@ -4221,6 +4255,7 @@ ON_EXIT:
>    SECUREBOOT_FREE_NON_OPCODE (EndOpCodeHandle);
>  
>    SECUREBOOT_FREE_NON_NULL (VariableData);
> +  SECUREBOOT_FREE_NON_NULL (FormatNameString);
>  
>    return Status;
>  }
> 



      reply	other threads:[~2017-10-18  7:02 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-18  4:50 [PATCH] SecurityPkg/SecureBootConfigDxe: Add check to avoid chenc2
2017-10-18  7:05 ` Laszlo Ersek [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e9206f05-c85f-5325-5193-7e298a069f0d@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox