[AMD Official Use Only - General]
Reviewed-by: Abner Chang <abner.ch...@amd.com>
> -----Original Message-----
> From: Nickle Wang <nick...@nvidia.com>
> Sent: Monday, May 15, 2023 12:25 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <abner.ch...@amd.com>; Igor Kulchytskyy
> <ig...@ami.com>
> Subject: [PATCH] RedfishPkg/RedfishPlatformConfigDxe: Fix string assert
> issue
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> When calling SetValue() with string type input, there is
> assertion of providing zero string ID to HII string function.
> Fix this issue by creating string ID for input string buffer.
> Fix Unicode and Ascii code convert issue together.
> Add text op-code support
>
> Signed-off-by: Nickle Wang <nick...@nvidia.com>
> Cc: Abner Chang <abner.ch...@amd.com>
> Cc: Igor Kulchytskyy <ig...@ami.com>
> ---
> .../RedfishPlatformConfigDxe.h | 14 +++
> .../RedfishPlatformConfigImpl.h | 16 +++
> .../RedfishPlatformConfigDxe.c | 116 ++++++++++++++++--
> .../RedfishPlatformConfigImpl.c | 50 +++++---
> 4 files changed, 169 insertions(+), 27 deletions(-)
>
> diff --git
> a/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigDxe.h
> b/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigDxe.h
> index 67697ecda787..c86bc6e9ce2d 100644
> --- a/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigDxe.h
> +++ b/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigDxe.h
> @@ -78,4 +78,18 @@ typedef struct {
> #define REDFISH_PLATFORM_CONFIG_DEBUG DEBUG_VERBOSE
> #define REDFISH_MENU_PATH_SIZE 8
>
> +/**
> + Convert input unicode string to ascii string. It's caller's
> + responsibility to free returned buffer using FreePool().
> +
> + @param[in] UnicodeString Unicode string to be converted.
> +
> + @retval CHAR8 * Ascii string on return.
> +
> +**/
> +CHAR8 *
> +StrToAsciiStr (
> + IN EFI_STRING UnicodeString
> + );
> +
> #endif
> diff --git
> a/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigImpl.h
> b/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigImpl.h
> index 9ef032748663..9f4312decf50 100644
> --- a/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigImpl.h
> +++ b/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigImpl.h
> @@ -250,6 +250,22 @@ ProcessPendingList (
> IN LIST_ENTRY *PendingList
> );
>
> +/**
> + Delete a string from HII Package List by given HiiHandle.
> +
> + @param[in] StringId Id of the string in HII database.
> + @param[in] HiiHandle The HII package list handle.
> +
> + @retval EFI_SUCCESS The string was deleted successfully.
> + @retval EFI_INVALID_PARAMETER StringId is zero.
> +
> +**/
> +EFI_STATUS
> +HiiDeleteString (
> + IN EFI_STRING_ID StringId,
> + IN EFI_HII_HANDLE HiiHandle
> + );
> +
> /**
> Retrieves a unicode string from a string package in a given language. The
> returned string is allocated using AllocatePool(). The caller is
> responsible
> diff --git
> a/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigDxe.c
> b/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigDxe.c
> index d3902f4127c1..1172d1094b06 100644
> --- a/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigDxe.c
> +++ b/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigDxe.c
> @@ -1172,6 +1172,7 @@ HiiValueToRedfishValue (
> UINTN Index;
> UINTN Count;
> EFI_STRING_ID *StringIdArray;
> + CHAR8 NullChar;
>
> if ((HiiHandle == NULL) || (HiiStatement == NULL) || (Value == NULL) ||
> (RedfishValue == NULL) || IS_EMPTY_STRING (FullSchema)) {
> return EFI_INVALID_PARAMETER;
> @@ -1180,6 +1181,7 @@ HiiValueToRedfishValue (
> StringIdArray = NULL;
> Count = 0;
> Status = EFI_SUCCESS;
> + NullChar = '\0';
>
> switch (HiiStatement->Operand) {
> case EFI_IFR_ONE_OF_OP:
> @@ -1205,9 +1207,18 @@ HiiValueToRedfishValue (
> break;
> }
>
> - RedfishValue->Type = RedfishValueTypeString;
> - RedfishValue->Value.Buffer = AllocatePool (StrLen ((CHAR16 *)Value-
> >Buffer) + 1);
> - UnicodeStrToAsciiStrS ((CHAR16 *)Value->Buffer, RedfishValue-
> >Value.Buffer, StrLen ((CHAR16 *)Value->Buffer) + 1);
> + if (Value->Buffer == NULL) {
> + RedfishValue->Value.Buffer = AllocateCopyPool (sizeof (NullChar),
> &NullChar);
> + } else {
> + RedfishValue->Value.Buffer = StrToAsciiStr ((EFI_STRING)Value-
> >Buffer);
> + }
> +
> + if (RedfishValue->Value.Buffer == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + break;
> + }
> +
> + RedfishValue->Type = RedfishValueTypeString;
> break;
> case EFI_IFR_CHECKBOX_OP:
> case EFI_IFR_NUMERIC_OP:
> @@ -1256,6 +1267,30 @@ HiiValueToRedfishValue (
>
> FreePool (StringIdArray);
> break;
> + case EFI_IFR_TEXT_OP:
> + //
> + // Use text two as the value
> + //
> + if (HiiStatement->ExtraData.TextTwo == 0x00) {
> + Status = EFI_NOT_FOUND;
> + break;
> + }
> +
> + RedfishValue->Value.Buffer = HiiGetRedfishAsciiString (HiiHandle,
> FullSchema, HiiStatement->ExtraData.TextTwo);
> + if (RedfishValue->Value.Buffer == NULL) {
> + //
> + // No x-uefi-redfish string defined. Try to get string in English.
> + //
> + RedfishValue->Value.Buffer = HiiGetEnglishAsciiString (HiiHandle,
> HiiStatement->ExtraData.TextTwo);
> + }
> +
> + if (RedfishValue->Value.Buffer == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + break;
> + }
> +
> + RedfishValue->Type = RedfishValueTypeString;
> + break;
> default:
> DEBUG ((DEBUG_ERROR, "%a: catch unsupported type: 0x%x! Please
> contact with author if we need to support this type.\n", __func__,
> HiiStatement->Operand));
> ASSERT (FALSE);
> @@ -1284,7 +1319,7 @@ StrToUnicodeStr (
> EFI_STRING Buffer;
> EFI_STATUS Status;
>
> - if ((AsciiString == NULL) || (AsciiString[0] == '\0')) {
> + if (AsciiString == NULL) {
> return NULL;
> }
>
> @@ -1303,6 +1338,43 @@ StrToUnicodeStr (
> return Buffer;
> }
>
> +/**
> + Convert input unicode string to ascii string. It's caller's
> + responsibility to free returned buffer using FreePool().
> +
> + @param[in] UnicodeString Unicode string to be converted.
> +
> + @retval CHAR8 * Ascii string on return.
> +
> +**/
> +CHAR8 *
> +StrToAsciiStr (
> + IN EFI_STRING UnicodeString
> + )
> +{
> + UINTN StringLen;
> + CHAR8 *Buffer;
> + EFI_STATUS Status;
> +
> + if (UnicodeString == NULL) {
> + return NULL;
> + }
> +
> + StringLen = StrLen (UnicodeString) + 1;
> + Buffer = AllocatePool (StringLen * sizeof (CHAR8));
> + if (Buffer == NULL) {
> + return NULL;
> + }
> +
> + Status = UnicodeStrToAsciiStrS (UnicodeString, Buffer, StringLen);
> + if (EFI_ERROR (Status)) {
> + FreePool (Buffer);
> + return NULL;
> + }
> +
> + return Buffer;
> +}
> +
> /**
> Return the full Redfish schema string from the given Schema and Version.
>
> @@ -1641,6 +1713,17 @@ RedfishPlatformConfigSetStatementCommon (
> }
> }
>
> + if ((TargetStatement->HiiStatement->Operand == EFI_IFR_STRING_OP)
> && (StatementValue->Type == EFI_IFR_TYPE_STRING)) {
> + //
> + // Create string ID for new string.
> + //
> + StatementValue->Value.string = HiiSetString (TargetStatement-
> >ParentForm->ParentFormset->HiiHandle, 0x00,
> (EFI_STRING)StatementValue->Buffer, NULL);
> + if (StatementValue->Value.string == 0) {
> + DEBUG ((DEBUG_ERROR, "%a: can not create string id\n", __func__));
> + return EFI_OUT_OF_RESOURCES;
> + }
> + }
> +
> Status = RedfishPlatformConfigSaveQuestionValue (
> TargetStatement->ParentForm->ParentFormset->HiiFormSet,
> TargetStatement->ParentForm->HiiForm,
> @@ -1649,10 +1732,13 @@ RedfishPlatformConfigSetStatementCommon (
> );
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_ERROR, "%a: failed to save question value: %r\n",
> __func__, Status));
> - return Status;
> }
>
> - return EFI_SUCCESS;
> + if (StatementValue->Value.string != 0) {
> + HiiDeleteString (StatementValue->Value.string, TargetStatement-
> >ParentForm->ParentFormset->HiiHandle);
> + }
> +
> + return Status;
> }
>
> /**
> @@ -1712,9 +1798,14 @@ RedfishPlatformConfigProtocolSetValue (
>
> break;
> case RedfishValueTypeString:
> + if (Value.Value.Buffer == NULL) {
> + Status = EFI_INVALID_PARAMETER;
> + goto RELEASE_RESOURCE;
> + }
> +
> NewValue.Type = EFI_IFR_TYPE_STRING;
> - NewValue.BufferLen = (UINT16)AsciiStrSize (Value.Value.Buffer);
> - NewValue.Buffer = AllocateCopyPool (NewValue.BufferLen,
> Value.Value.Buffer);
> + NewValue.BufferLen = (UINT16)(AsciiStrSize (Value.Value.Buffer) *
> sizeof (CHAR16));
> + NewValue.Buffer = (UINT8 *)StrToUnicodeStr (Value.Value.Buffer);
> if (NewValue.Buffer == NULL) {
> Status = EFI_OUT_OF_RESOURCES;
> goto RELEASE_RESOURCE;
> @@ -1742,6 +1833,10 @@ RELEASE_RESOURCE:
> FreePool (FullSchema);
> }
>
> + if ((Value.Type == RedfishValueTypeString) && (NewValue.Buffer !=
> NULL)) {
> + FreePool (NewValue.Buffer);
> + }
> +
> return Status;
> }
>
> @@ -1784,6 +1879,7 @@ RedfishPlatformConfigProtocolGetConfigureLang (
> return EFI_INVALID_PARAMETER;
> }
>
> + ZeroMem (&StatementList, sizeof (StatementList));
> *Count = 0;
> *ConfigureLangList = NULL;
> FullSchema = NULL;
> @@ -1849,7 +1945,9 @@ RELEASE_RESOURCE:
> FreePool (FullSchema);
> }
>
> - ReleaseStatementList (&StatementList);
> + if (StatementList.Count > 0) {
> + ReleaseStatementList (&StatementList);
> + }
>
> return Status;
> }
> diff --git
> a/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigImpl.c
> b/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigImpl.c
> index 889448fe3870..e4a905aec44e 100644
> --- a/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigImpl.c
> +++ b/RedfishPkg/RedfishPlatformConfigDxe/RedfishPlatformConfigImpl.c
> @@ -143,6 +143,34 @@ DumpFormsetList (
> return EFI_SUCCESS;
> }
>
> +/**
> + Delete a string from HII Package List by given HiiHandle.
> +
> + @param[in] StringId Id of the string in HII database.
> + @param[in] HiiHandle The HII package list handle.
> +
> + @retval EFI_SUCCESS The string was deleted successfully.
> + @retval EFI_INVALID_PARAMETER StringId is zero.
> +
> +**/
> +EFI_STATUS
> +HiiDeleteString (
> + IN EFI_STRING_ID StringId,
> + IN EFI_HII_HANDLE HiiHandle
> + )
> +{
> + CHAR16 NullChar;
> +
> + if (StringId == 0x00) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + NullChar = CHAR_NULL;
> + HiiSetString (HiiHandle, StringId, &NullChar, NULL);
> +
> + return EFI_SUCCESS;
> +}
> +
> /**
> Retrieves a unicode string from a string package in a given language. The
> returned string is allocated using AllocatePool(). The caller is
> responsible
> @@ -259,7 +287,6 @@ HiiGetRedfishAsciiString (
> )
> {
> EFI_STRING HiiString;
> - UINTN StringSize;
> CHAR8 *AsciiString;
>
> HiiString = HiiGetRedfishString (HiiHandle, Language, StringId);
> @@ -268,15 +295,9 @@ HiiGetRedfishAsciiString (
> return NULL;
> }
>
> - StringSize = (StrLen (HiiString) + 1) * sizeof (CHAR8);
> - AsciiString = AllocatePool (StringSize);
> - if (AsciiString == NULL) {
> - return NULL;
> - }
> -
> - UnicodeStrToAsciiStrS (HiiString, AsciiString, StringSize);
> -
> + AsciiString = StrToAsciiStr (HiiString);
> FreePool (HiiString);
> +
> return AsciiString;
> }
>
> @@ -322,7 +343,6 @@ HiiGetEnglishAsciiString (
> )
> {
> EFI_STRING HiiString;
> - UINTN StringSize;
> CHAR8 *AsciiString;
>
> HiiString = HiiGetRedfishString (HiiHandle, ENGLISH_LANGUAGE_CODE,
> StringId);
> @@ -331,15 +351,9 @@ HiiGetEnglishAsciiString (
> return NULL;
> }
>
> - StringSize = (StrLen (HiiString) + 1) * sizeof (CHAR8);
> - AsciiString = AllocatePool (StringSize);
> - if (AsciiString == NULL) {
> - return NULL;
> - }
> -
> - UnicodeStrToAsciiStrS (HiiString, AsciiString, StringSize);
> -
> + AsciiString = StrToAsciiStr (HiiString);
> FreePool (HiiString);
> +
> return AsciiString;
> }
>
> --
> 2.17.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104849): https://edk2.groups.io/g/devel/message/104849
Mute This Topic: https://groups.io/mt/98897203/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
