Check the validation when return from callback function to avoid use the invalid form set.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>

git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@15654 6f19259b-4bc3-4df7-8a09-765794883524

MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c

@@ -817,6 +817,13 @@ UpdateStatementStatusForForm (
     Question = FORM_BROWSER_STATEMENT_FROM_LINK (Link);
     Link = GetNextNode (&Form->StatementListHead, Link);
 
+    //
+    // For password opcode, not set the the value changed flag.
+    //
+    if (Question->Operand == EFI_IFR_PASSWORD_OP) {
+      continue;
+    }
+
     IsQuestionValueChanged(FormSet, Form, Question, GetSetValueWithBuffer);
   }
 }
@@ -1228,15 +1235,7 @@ FindParentFormSet (
   FORM_ENTRY_INFO            *ParentMenu;
 
   CurrentMenu = Selection->CurrentMenu;
-  ParentMenu  = UiFindParentMenu(CurrentMenu);
+  ParentMenu  = UiFindParentMenu(CurrentMenu, FormSetLevel);
-
-  //
-  // Find a menu which has different formset guid with current.
-  //
-  while (ParentMenu != NULL && CompareGuid (&CurrentMenu->FormSetGuid, &ParentMenu->FormSetGuid)) {
-    CurrentMenu = ParentMenu;
-    ParentMenu  = UiFindParentMenu(CurrentMenu);
-  }
 
   if (ParentMenu != NULL) {
     CopyMem (&Selection->FormSetGuid, &ParentMenu->FormSetGuid, sizeof (EFI_GUID));
@@ -1820,31 +1819,16 @@ FindNextMenu (
   BROWSER_SETTING_SCOPE      Scope;
   
   CurrentMenu = Selection->CurrentMenu;
-  ParentMenu  = NULL;
   Scope       = FormSetLevel;
 
-  if (CurrentMenu != NULL && (ParentMenu = UiFindParentMenu(CurrentMenu)) != NULL) {
+  ParentMenu = UiFindParentMenu(CurrentMenu, SettingLevel);
-    //
+  while (ParentMenu != NULL && !ValidateHiiHandle(ParentMenu->HiiHandle)) {
-    // we have a parent, so go to the parent menu
+    ParentMenu = UiFindParentMenu(ParentMenu, SettingLevel);
-    //
-    if (CompareGuid (&CurrentMenu->FormSetGuid, &ParentMenu->FormSetGuid)) {
-      if (SettingLevel == FormSetLevel) {
-        //
-        // Find a menu which has different formset guid with current.
-        //
-        while (CompareGuid (&CurrentMenu->FormSetGuid, &ParentMenu->FormSetGuid)) {
-          CurrentMenu = ParentMenu;
-          if ((ParentMenu = UiFindParentMenu(CurrentMenu)) == NULL) {
-            break;
-          }
   }
 
   if (ParentMenu != NULL) {
-          Scope = FormSetLevel;
+    if (CompareGuid (&CurrentMenu->FormSetGuid, &ParentMenu->FormSetGuid)) {
-        }
-      } else {
       Scope = FormLevel;
-      }
     } else {
       Scope = FormSetLevel;
     }
@@ -2001,6 +1985,17 @@ ProcessCallBackFunction (
                              TypeValue,
                              &ActionRequest
                              );
+    //
+    // IFR is updated, force to reparse the IFR binary
+    //
+    if (mHiiPackageListUpdated) {
+      if (BackUpBuffer != NULL) {
+        FreePool (BackUpBuffer);
+      }
+
+      return EFI_SUCCESS;
+    }
+
     if (!EFI_ERROR (Status)) {
       //
       // Need to sync the value between Statement->HiiValue->Value and Statement->BufferValue
@@ -2134,6 +2129,15 @@ ProcessCallBackFunction (
     if (BackUpBuffer != NULL) {
       FreePool (BackUpBuffer);
     }
+
+    //
+    // If Question != NULL, means just process one question
+    // and if code reach here means this question has finished
+    // processing, so just break.
+    //
+    if (Question != NULL) {
+      break;
+    }
   }
 
   if (SubmitFormIsRequired && !SkipSaveOrDiscard) {
@@ -2277,6 +2281,8 @@ SetupBrowser (
   do {
     //
     // IFR is updated, force to reparse the IFR binary
+    // This check is shared by EFI_BROWSER_ACTION_FORM_CLOSE and 
+    // EFI_BROWSER_ACTION_RETRIEVE, so code place here.
     //
     if (mHiiPackageListUpdated) {
       Selection->Action = UI_ACTION_REFRESH_FORMSET;
@@ -2348,7 +2354,7 @@ SetupBrowser (
         }
 
         //
-        // IFR is updated during callback of open form, force to reparse the IFR binary
+        // IFR is updated during callback of EFI_BROWSER_ACTION_FORM_OPEN, force to reparse the IFR binary
         //
         if (mHiiPackageListUpdated) {
           Selection->Action = UI_ACTION_REFRESH_FORMSET;
@@ -2406,6 +2412,15 @@ SetupBrowser (
           ((Statement->QuestionFlags & EFI_IFR_FLAG_CALLBACK) == EFI_IFR_FLAG_CALLBACK) && 
           (Statement->Operand != EFI_IFR_PASSWORD_OP)) {
         Status = ProcessCallBackFunction(Selection, Selection->FormSet, Selection->Form, Statement, EFI_BROWSER_ACTION_CHANGING, FALSE);
+        //
+        // IFR is updated during callback of EFI_BROWSER_ACTION_CHANGING, force to reparse the IFR binary
+        //
+        if (mHiiPackageListUpdated) {
+          Selection->Action = UI_ACTION_REFRESH_FORMSET;
+          mHiiPackageListUpdated = FALSE;
+          break;
+        }
+
         if (Statement->Operand == EFI_IFR_REF_OP) {
           //
           // Process dynamic update ref opcode.
@@ -2433,6 +2448,14 @@ SetupBrowser (
 
         if (!EFI_ERROR (Status) && Statement->Operand != EFI_IFR_REF_OP) {
           ProcessCallBackFunction(Selection, Selection->FormSet, Selection->Form, Statement, EFI_BROWSER_ACTION_CHANGED, FALSE);
+          //
+          // IFR is updated during callback of EFI_BROWSER_ACTION_CHANGED, force to reparse the IFR binary
+          //
+          if (mHiiPackageListUpdated) {
+            Selection->Action = UI_ACTION_REFRESH_FORMSET;
+            mHiiPackageListUpdated = FALSE;
+            break;
+          }
         }
       } else {
         //

MdeModulePkg/Universal/SetupBrowserDxe/Setup.c

@@ -206,19 +206,56 @@ UiFindMenuList (
   Find parent menu for current menu.
 
   @param  CurrentMenu    Current Menu
+  @param  SettingLevel   Whether find parent menu in Form Level or Formset level.
+                         In form level, just find the parent menu; 
+                         In formset level, find the parent menu which has different
+                         formset guid value.
 
   @retval   The parent menu for current menu.
 **/
 FORM_ENTRY_INFO *
 UiFindParentMenu (
-  IN FORM_ENTRY_INFO  *CurrentMenu
+  IN FORM_ENTRY_INFO          *CurrentMenu,
+  IN BROWSER_SETTING_SCOPE    SettingLevel
   )
 {
   FORM_ENTRY_INFO    *ParentMenu;
+  LIST_ENTRY         *Link;
+
+  ASSERT (SettingLevel == FormLevel || SettingLevel == FormSetLevel);
+
+  if (CurrentMenu == NULL) {
+    return NULL;
+  }
   
   ParentMenu = NULL;
-  if (CurrentMenu->Link.BackLink != &mPrivateData.FormBrowserEx2.FormViewHistoryHead) {
+  Link       = &CurrentMenu->Link;
-    ParentMenu = FORM_ENTRY_INFO_FROM_LINK (CurrentMenu->Link.BackLink);
+
+  while (Link->BackLink != &mPrivateData.FormBrowserEx2.FormViewHistoryHead) {
+    ParentMenu = FORM_ENTRY_INFO_FROM_LINK (Link->BackLink);
+
+    if (SettingLevel == FormLevel) {
+      //
+      // For FormLevel, just find the parent menu, return.
+      //
+      break;
+    }
+
+    if (!CompareGuid (&CurrentMenu->FormSetGuid, &ParentMenu->FormSetGuid)) {
+      //
+      // For SystemLevel, must find the menu which has different formset.
+      //
+      break;
+    }
+
+    Link = Link->BackLink;
+  }
+
+  //
+  // Not find the parent menu, just return NULL.
+  //
+  if (Link->BackLink == &mPrivateData.FormBrowserEx2.FormViewHistoryHead) {
+    return NULL;
   }
 
   return ParentMenu;
@@ -481,6 +518,14 @@ SendForm (
       FormSet = AllocateZeroPool (sizeof (FORM_BROWSER_FORMSET));
       ASSERT (FormSet != NULL);
 
+      //
+      // Validate the HiiHandle
+      // if validate failed, find the first validate parent HiiHandle.
+      //
+      if (!ValidateHiiHandle(Selection->Handle)) {
+        FindNextMenu (Selection, FormSetLevel);
+      }
+
       //
       // Initialize internal data structures of FormSet
       //
@@ -2356,40 +2401,60 @@ SendDiscardInfoToDriver (
 
 **/
 BOOLEAN
-ValidateFormSet (
+ValidateHiiHandle (
-  FORM_BROWSER_FORMSET    *FormSet
+  EFI_HII_HANDLE          HiiHandle
   )
 {
   EFI_HII_HANDLE          *HiiHandles;
   UINTN                   Index;
   BOOLEAN                 Find;
 
-  ASSERT (FormSet != NULL);
+  if (HiiHandle == NULL) {
+    return FALSE;
+  }
+
   Find = FALSE;
-  //
+
-  // Get all the Hii handles
-  //
   HiiHandles = HiiGetHiiHandles (NULL);
   ASSERT (HiiHandles != NULL);
 
-  //
-  // Search for formset of each class type
-  //
   for (Index = 0; HiiHandles[Index] != NULL; Index++) {
-    if (HiiHandles[Index] == FormSet->HiiHandle) {
+    if (HiiHandles[Index] == HiiHandle) {
       Find = TRUE;
       break;
     }
   }
 
+  FreePool (HiiHandles);
+
+  return Find;
+}
+
+/**
+  Validate the FormSet. If the formset is not validate, remove it from the list.
+
+  @param  FormSet                The input FormSet which need to validate.
+
+  @retval TRUE                   The handle is validate.
+  @retval FALSE                  The handle is invalidate.
+
+**/
+BOOLEAN
+ValidateFormSet (
+  FORM_BROWSER_FORMSET    *FormSet
+  )
+{
+  BOOLEAN  Find;
+
+  ASSERT (FormSet != NULL);
+
+  Find = ValidateHiiHandle(FormSet->HiiHandle);
   if (!Find) {
     CleanBrowserStorage(FormSet);
     RemoveEntryList (&FormSet->Link);
     DestroyFormSet (FormSet);
   }
 
-  FreePool (HiiHandles);
-
   return Find;
 }
 /**

MdeModulePkg/Universal/SetupBrowserDxe/Setup.h

@@ -1530,12 +1530,31 @@ UiFreeMenuList (
   Find parent menu for current menu.
 
   @param  CurrentMenu    Current Menu
+  @param  SettingLevel   Whether find parent menu in Form Level or Formset level.
+                         In form level, just find the parent menu; 
+                         In formset level, find the parent menu which has different
+                         formset guid value.
 
   @retval   The parent menu for current menu.
 **/
 FORM_ENTRY_INFO *
 UiFindParentMenu (
-  IN FORM_ENTRY_INFO  *CurrentMenu
+  IN FORM_ENTRY_INFO          *CurrentMenu,
+  IN BROWSER_SETTING_SCOPE    SettingLevel
+  );
+  
+/**
+  Validate the FormSet. If the formset is not validate, remove it from the list.
+
+  @param  FormSet                The input FormSet which need to validate.
+
+  @retval TRUE                   The handle is validate.
+  @retval FALSE                  The handle is invalidate.
+
+**/
+BOOLEAN
+ValidateHiiHandle (
+  EFI_HII_HANDLE          HiiHandle
   );
 
 /**