[patch] MdeModulePkg/Setup: Fix incorrect size used in AllocateCopyPool

[patch] MdeModulePkg/Setup: Fix incorrect size used in AllocateCopyPool

[Dandan Bi]

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1115

When the type of HiiValue is EFI_IFR_TYPE_BUFFER,
its question type is EFI_IFR_ORDERED_LIST_OP.
And the buffer size allocated for Statement->BufferValue
of orderedList is "Statement->StorageWidth"
in IfrParse.c.

So here when backup the buffer value and copy the size of
"Statement->StorageWidth + sizeof(CHAR16)" is incorrect.

This patch is to fix this issue.

Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c b/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c
index ded1c7ad11..58daaab404 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c
@@ -2002,11 +2002,11 @@ ProcessCallBackFunction (
     //
     // If EFI_BROWSER_ACTION_CHANGING type, back up the new question value.
     //
     if (Action == EFI_BROWSER_ACTION_CHANGING) {
       if (HiiValue->Type == EFI_IFR_TYPE_BUFFER) {
-        BackUpBuffer = AllocateCopyPool(Statement->StorageWidth + sizeof(CHAR16), Statement->BufferValue);
+        BackUpBuffer = AllocateCopyPool(Statement->StorageWidth, Statement->BufferValue);
         ASSERT (BackUpBuffer != NULL);
       } else {
         CopyMem (&BackUpValue, &HiiValue->Value, sizeof (EFI_IFR_TYPE_VALUE));
       }
     }
@@ -2128,11 +2128,11 @@ ProcessCallBackFunction (
       // then the browser will use the value passed to Callback() and ignore the
       // value returned by Callback().
       //
       if (Action  == EFI_BROWSER_ACTION_CHANGING && Status == EFI_UNSUPPORTED) {
         if (HiiValue->Type == EFI_IFR_TYPE_BUFFER) {
-          CopyMem (Statement->BufferValue, BackUpBuffer, Statement->StorageWidth + sizeof(CHAR16));
+          CopyMem (Statement->BufferValue, BackUpBuffer, Statement->StorageWidth);
         } else {
           CopyMem (&HiiValue->Value, &BackUpValue, sizeof (EFI_IFR_TYPE_VALUE));
         }
 
         //
-- 
2.14.3.windows.1

Re: [patch] MdeModulePkg/Setup: Fix incorrect size used in AllocateCopyPool

[Dong, Eric]

Reviewed-by: Eric Dong <eric.dong@intel.com>

-----Original Message-----
From: Bi, Dandan 
Sent: Tuesday, August 28, 2018 10:06 AM
To: edk2-devel@lists.01.org
Cc: Dong, Eric <eric.dong@intel.com>
Subject: [patch] MdeModulePkg/Setup: Fix incorrect size used in AllocateCopyPool

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1115

When the type of HiiValue is EFI_IFR_TYPE_BUFFER, its question type is EFI_IFR_ORDERED_LIST_OP.
And the buffer size allocated for Statement->BufferValue of orderedList is "Statement->StorageWidth"
in IfrParse.c.

So here when backup the buffer value and copy the size of "Statement->StorageWidth + sizeof(CHAR16)" is incorrect.

This patch is to fix this issue.

Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c b/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c
index ded1c7ad11..58daaab404 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c
@@ -2002,11 +2002,11 @@ ProcessCallBackFunction (
     //
     // If EFI_BROWSER_ACTION_CHANGING type, back up the new question value.
     //
     if (Action == EFI_BROWSER_ACTION_CHANGING) {
       if (HiiValue->Type == EFI_IFR_TYPE_BUFFER) {
-        BackUpBuffer = AllocateCopyPool(Statement->StorageWidth + sizeof(CHAR16), Statement->BufferValue);
+        BackUpBuffer = AllocateCopyPool(Statement->StorageWidth, 
+ Statement->BufferValue);
         ASSERT (BackUpBuffer != NULL);
       } else {
         CopyMem (&BackUpValue, &HiiValue->Value, sizeof (EFI_IFR_TYPE_VALUE));
       }
     }
@@ -2128,11 +2128,11 @@ ProcessCallBackFunction (
       // then the browser will use the value passed to Callback() and ignore the
       // value returned by Callback().
       //
       if (Action  == EFI_BROWSER_ACTION_CHANGING && Status == EFI_UNSUPPORTED) {
         if (HiiValue->Type == EFI_IFR_TYPE_BUFFER) {
-          CopyMem (Statement->BufferValue, BackUpBuffer, Statement->StorageWidth + sizeof(CHAR16));
+          CopyMem (Statement->BufferValue, BackUpBuffer, 
+ Statement->StorageWidth);
         } else {
           CopyMem (&HiiValue->Value, &BackUpValue, sizeof (EFI_IFR_TYPE_VALUE));
         }
 
         //
--
2.14.3.windows.1