Do the following fix up in SetupBrowser driver:

  1)  Check whether ConfigAccess is NULL before use it.
  2)  Don't do call back for UI_ACTION_REFRESH_FORMSET action.
  3)  Release resource before leave SetupBrowser() function.
  4)  Use the unified check method (HiiHandle, FormsetGuid and FormId) to check FORM is open or close.


git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@10565 6f19259b-4bc3-4df7-8a09-765794883524

3 files changed, 51 insertions, 57 deletions

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c b/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c
index e9b4df4df9..a2704511c2 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c
@@ -16,7 +16,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 

 BOOLEAN            mHiiPackageListUpdated;

 UI_MENU_SELECTION  *gCurrentSelection;

-

+EFI_HII_HANDLE     mCurrentHiiHandle = NULL;

+EFI_GUID           mCurrentFormSetGuid = {0, 0, 0, {0, 0, 0, 0, 0, 0, 0, 0}};

+UINT16             mCurrentFormId = 0;

 

 /**

   Clear retangle with specified text attribute.

@@ -842,8 +844,6 @@ FormUpdateNotify (
   return EFI_SUCCESS;

 }

 

-BOOLEAN mFormCloseCallBack = FALSE;

-

 /**

   The worker function that send the displays to the screen. On output,

   the selection made by user is returned.

@@ -872,11 +872,7 @@ SetupBrowser (
   EFI_HII_CONFIG_ACCESS_PROTOCOL  *ConfigAccess;

   FORM_BROWSER_FORMSET            *FormSet;

   EFI_INPUT_KEY                   Key;

-  BOOLEAN                         FormOpenCallBack;

   BOOLEAN                         SubmitFormIsRequired;

-  EFI_GUID                        CurrentFormSetGuid;

-  EFI_HII_HANDLE                  CurrentHiiHandle;

-  UINT16                          CurrentFormId;

 

   gMenuRefreshHead = NULL;

   gResetRequired = FALSE;

@@ -903,7 +899,6 @@ SetupBrowser (
   //

   Status = InitializeCurrentSetting (Selection->FormSet);

   if (EFI_ERROR (Status)) {

-    Selection->Action = UI_ACTION_EXIT;

     goto Done;

   }

 

@@ -911,14 +906,8 @@ SetupBrowser (
     //

     // Initialize Selection->Form

     //

-    FormOpenCallBack = FALSE;

     if (Selection->FormId == 0) {

       //

-      // First Form will open.

-      //

-      FormOpenCallBack = TRUE;

-

-      //

       // Zero FormId indicates display the first Form in a FormSet

       //

       Link = GetFirstNode (&Selection->FormSet->FormListHead);

@@ -926,12 +915,6 @@ SetupBrowser (
       Selection->Form = FORM_BROWSER_FORM_FROM_LINK (Link);

       Selection->FormId = Selection->Form->FormId;

     } else {

-      if (Selection->Form == NULL) {

-        //

-        // First Form will open.

-        //

-        FormOpenCallBack = TRUE;

-      }

       Selection->Form = IdToForm (Selection->FormSet, Selection->FormId);

     }

 

@@ -939,7 +922,8 @@ SetupBrowser (
       //

       // No Form to display

       //

-      return EFI_NOT_FOUND;

+      Status = EFI_NOT_FOUND;

+      goto Done;

     }

 

     //

@@ -947,8 +931,9 @@ SetupBrowser (
     //

     if (Selection->Form->SuppressExpression != NULL) {

       Status = EvaluateExpression (Selection->FormSet, Selection->Form, Selection->Form->SuppressExpression);

-      if (EFI_ERROR (Status)) {

-        return Status;

+      if (EFI_ERROR (Status) || (Selection->Form->SuppressExpression->Result.Type != EFI_IFR_TYPE_BOOLEAN)) {

+        Status = EFI_INVALID_PARAMETER;

+        goto Done;

       }

 

       if (Selection->Form->SuppressExpression->Result.Value.b) {

@@ -959,18 +944,12 @@ SetupBrowser (
           CreateDialog (4, TRUE, 0, NULL, &Key, gEmptyString, gFormSuppress, gPressEnter, gEmptyString);

         } while (Key.UnicodeChar != CHAR_CARRIAGE_RETURN);

 

-        return EFI_NOT_FOUND;

+        Status = EFI_NOT_FOUND;

+        goto Done;

       }

     }

 

     //

-    // Keep current form information

-    //

-    CurrentHiiHandle   = Selection->Handle;

-    CopyGuid (&CurrentFormSetGuid, &Selection->FormSetGuid);

-    CurrentFormId      = Selection->FormId;

-

-    //

     // Reset FormPackage update flag

     //

     mHiiPackageListUpdated = FALSE;

@@ -980,8 +959,18 @@ SetupBrowser (
     // for each question with callback flag.

     // New form may be the first form, or the different form after another form close.

     //

-    if ((FormOpenCallBack || mFormCloseCallBack) && (ConfigAccess != NULL)) {

-      mFormCloseCallBack = FALSE;

+    if ((ConfigAccess != NULL) &&

+        ((Selection->Handle != mCurrentHiiHandle) ||

+        (!CompareGuid (&Selection->FormSetGuid, &mCurrentFormSetGuid)) ||

+        (Selection->FormId != mCurrentFormId))) {

+

+      //

+      // Keep current form information

+      //

+      mCurrentHiiHandle   = Selection->Handle;

+      CopyGuid (&mCurrentFormSetGuid, &Selection->FormSetGuid);

+      mCurrentFormId      = Selection->FormId;

+

       //

       // Go through each statement in this form

       //

@@ -1061,7 +1050,7 @@ SetupBrowser (
     //

     Status = LoadFormSetConfig (Selection, Selection->FormSet);

     if (EFI_ERROR (Status)) {

-      return Status;

+      goto Done;

     }

 

     //

@@ -1089,7 +1078,7 @@ SetupBrowser (
     //

     Status = DisplayForm (Selection);

     if (EFI_ERROR (Status)) {

-      return Status;

+      goto Done;

     }

 

     //

@@ -1106,12 +1095,11 @@ SetupBrowser (
       //

       mHiiPackageListUpdated = FALSE;

 

-      if (((Statement->QuestionFlags & EFI_IFR_FLAG_CALLBACK) == EFI_IFR_FLAG_CALLBACK) && (Statement->Operand != EFI_IFR_PASSWORD_OP)) {

-        ActionRequest = EFI_BROWSER_ACTION_REQUEST_NONE;

+      if ((ConfigAccess != NULL) && 

+          ((Statement->QuestionFlags & EFI_IFR_FLAG_CALLBACK) == EFI_IFR_FLAG_CALLBACK) && 

+          (Statement->Operand != EFI_IFR_PASSWORD_OP)) {

 

-        if (ConfigAccess == NULL) {

-          return EFI_UNSUPPORTED;

-        }

+        ActionRequest = EFI_BROWSER_ACTION_REQUEST_NONE;

 

         HiiValue = &Statement->HiiValue;

         TypeValue = &HiiValue->Value;

@@ -1191,16 +1179,14 @@ SetupBrowser (
     // Before exit the form, invoke ConfigAccess.Callback() with EFI_BROWSER_ACTION_FORM_CLOSE

     // for each question with callback flag.

     //

-    mFormCloseCallBack = FALSE;

     if ((ConfigAccess != NULL) && 

         ((Selection->Action == UI_ACTION_EXIT) || 

-         (Selection->Handle != CurrentHiiHandle) ||

-         (!CompareGuid (&CurrentFormSetGuid, &Selection->FormSetGuid)) ||

-         (Selection->FormId != CurrentFormId))) {

+         (Selection->Handle != mCurrentHiiHandle) ||

+         (!CompareGuid (&Selection->FormSetGuid, &mCurrentFormSetGuid)) ||

+         (Selection->FormId != mCurrentFormId))) {

       //

       // Go through each statement in this form

       //

-      mFormCloseCallBack   = TRUE;

       SubmitFormIsRequired = FALSE;

       Link = GetFirstNode (&Selection->Form->StatementListHead);

       while (!IsNull (&Selection->Form->StatementListHead, Link)) {

@@ -1257,11 +1243,20 @@ SetupBrowser (
 

 Done:

   //

+  // Reset current form information to the initial setting when error happens or form exit.

+  //

+  if (EFI_ERROR (Status) || Selection->Action == UI_ACTION_EXIT) {

+    mCurrentHiiHandle = NULL;

+    CopyGuid (&mCurrentFormSetGuid, &gZeroGuid);

+    mCurrentFormId = 0;

+  }

+

+  //

   // Unregister notify for Form package update

   //

-  Status = mHiiDatabase->UnregisterPackageNotify (

-                           mHiiDatabase,

-                           NotifyHandle

-                           );

+  mHiiDatabase->UnregisterPackageNotify (

+                   mHiiDatabase,

+                   NotifyHandle

+                   );

   return Status;

 }

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c b/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c
index 6bd2063caf..a4d19e0afe 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/ProcessOptions.c
@@ -778,8 +778,9 @@ ProcessOptions (
         //

         *StringPtr = 0;

         Status = PasswordCallback (Selection, MenuOption, StringPtr);

-        if (Status == EFI_NOT_AVAILABLE_YET) {

+        if (Status == EFI_NOT_AVAILABLE_YET || Status == EFI_UNSUPPORTED) {

           //

+          // Callback is not supported, or

           // Callback request to terminate password input

           //

           FreePool (StringPtr);

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
index e06228d4aa..42edc08970 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
@@ -2080,15 +2080,13 @@ LoadFormConfig (
     //

     // Check whether EfiVarstore with CallBack can be got.

     //

-    if ((Question->QuestionId != 0) && (Question->Storage != NULL) &&

+    if ((FormSet->ConfigAccess != NULL) &&

+        (Selection->Action != UI_ACTION_REFRESH_FORMSET) &&

+        (Question->QuestionId != 0) && 

+        (Question->Storage != NULL) &&

         (Question->Storage->Type == EFI_HII_VARSTORE_EFI_VARIABLE) && 

         ((Question->QuestionFlags & EFI_IFR_FLAG_CALLBACK) == EFI_IFR_FLAG_CALLBACK)) {

-      //

-      // ConfigAccess can't be NULL.

-      //

-      if (FormSet->ConfigAccess == NULL) {

-        return EFI_UNSUPPORTED;

-      }

+

       //

       // Check QuestionValue does exist.

       //