public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] RedfishPkg/RedfishPlatformConfigDxe: Fix string assert issue
@ 2023-05-15  4:24 Nickle Wang
  2023-05-15  4:48 ` Chang, Abner
  2023-05-15 14:01 ` Igor Kulchytskyy
  0 siblings, 2 replies; 3+ messages in thread
From: Nickle Wang @ 2023-05-15  4:24 UTC (permalink / raw)
  To: devel; +Cc: Abner Chang, Igor Kulchytskyy

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 <nicklew@nvidia.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@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


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

* Re: [PATCH] RedfishPkg/RedfishPlatformConfigDxe: Fix string assert issue
  2023-05-15  4:24 [PATCH] RedfishPkg/RedfishPlatformConfigDxe: Fix string assert issue Nickle Wang
@ 2023-05-15  4:48 ` Chang, Abner
  2023-05-15 14:01 ` Igor Kulchytskyy
  1 sibling, 0 replies; 3+ messages in thread
From: Chang, Abner @ 2023-05-15  4:48 UTC (permalink / raw)
  To: Nickle Wang, devel@edk2.groups.io; +Cc: Igor Kulchytskyy

[AMD Official Use Only - General]

Reviewed-by: Abner Chang <abner.chang@amd.com>

> -----Original Message-----
> From: Nickle Wang <nicklew@nvidia.com>
> Sent: Monday, May 15, 2023 12:25 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; Igor Kulchytskyy
> <igork@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 <nicklew@nvidia.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Igor Kulchytskyy <igork@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

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

* Re: [PATCH] RedfishPkg/RedfishPlatformConfigDxe: Fix string assert issue
  2023-05-15  4:24 [PATCH] RedfishPkg/RedfishPlatformConfigDxe: Fix string assert issue Nickle Wang
  2023-05-15  4:48 ` Chang, Abner
@ 2023-05-15 14:01 ` Igor Kulchytskyy
  1 sibling, 0 replies; 3+ messages in thread
From: Igor Kulchytskyy @ 2023-05-15 14:01 UTC (permalink / raw)
  To: Nickle Wang, devel@edk2.groups.io; +Cc: Abner Chang

Reviewed-by: Igor Kulchytskyy <igork@ami.com>

-----Original Message-----
From: Nickle Wang <nicklew@nvidia.com>
Sent: Monday, May 15, 2023 12:25 AM
To: devel@edk2.groups.io
Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy <igork@ami.com>
Subject: [EXTERNAL] [PATCH] RedfishPkg/RedfishPlatformConfigDxe: Fix string assert issue


**CAUTION: The e-mail below is from an external source. Please exercise caution before opening attachments, clicking links, or following guidance.**

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 <nicklew@nvidia.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@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

-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.

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

end of thread, other threads:[~2023-05-15 14:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-15  4:24 [PATCH] RedfishPkg/RedfishPlatformConfigDxe: Fix string assert issue Nickle Wang
2023-05-15  4:48 ` Chang, Abner
2023-05-15 14:01 ` Igor Kulchytskyy

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