MdeModulePkg/UefiBootManagerLib: limit recursive call depth

Function BmRepairAllControllers may recursively call itself if some
driver health protocol returns EfiDriverHealthStatusReconnectRequired.
However, driver health protocol of some buggy third party driver may
always return such status even after one and another reconnect. The
endless iteration will cause stack overflow and then system exception,
and it may be not easy to find that the exception is actually caused
by stack overflow.

So we limit the number of reconnect retry to 10 to improve code
robustness, and DEBUG_CODE is moved ahead before recursive repair to
track the repair result.

We also remove a duplicated declaration of BmRepairAllControllers() in
InternalBm.h in this patch, for it is only a trivial change.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index ce19ae4006604a9cc80261bda04e0254b949fd12..b842d5824aed669551154fa34c20146064b84629 100644 (file)
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1767,7 +1767,7 @@ EfiBootManagerBoot (
     //\r
     // 4. Repair system through DriverHealth protocol\r
     //\r
-    BmRepairAllControllers ();\r
+    BmRepairAllControllers (0);\r
   }\r
 \r
   PERF_START_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32) OptionNumber);\r

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
index ddcee8b0676f9f387b897dfe29bb9a74e9e0230d..db2f859ae73da750dafd2bd1a749c1e675e996a1 100644 (file)
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
@@ -423,10 +423,13 @@ EfiBootManagerFreeDriverHealthInfo (
 \r
 /**\r
   Repair all the controllers according to the Driver Health status queried.\r
+\r
+  @param ReconnectRepairCount     To record the number of recursive call of\r
+                                  this function itself.\r
 **/\r
 VOID\r
 BmRepairAllControllers (\r
-  VOID\r
+  UINTN       ReconnectRepairCount\r
   )\r
 {\r
   EFI_STATUS                          Status;\r
@@ -548,10 +551,6 @@ BmRepairAllControllers (
   EfiBootManagerFreeDriverHealthInfo (DriverHealthInfo, Count);\r
 \r
 \r
-  if (ReconnectRequired) {\r
-    BmRepairAllControllers ();\r
-  }\r
-\r
   DEBUG_CODE (\r
     CHAR16 *ControllerName;\r
 \r
@@ -576,6 +575,15 @@ BmRepairAllControllers (
     EfiBootManagerFreeDriverHealthInfo (DriverHealthInfo, Count);\r
     );\r
 \r
+  if (ReconnectRequired) {\r
+    if (ReconnectRepairCount < MAX_RECONNECT_REPAIR) {\r
+      BmRepairAllControllers (ReconnectRepairCount + 1);\r
+    } else {\r
+      DEBUG ((DEBUG_ERROR, "[%a:%d] Repair failed after %d retries.\n",\r
+        __FUNCTION__, __LINE__, ReconnectRepairCount));\r
+    }\r
+  }\r
+\r
   if (RebootRequired) {\r
     DEBUG ((EFI_D_INFO, "[BDS] One of the Driver Health instances requires rebooting.\n"));\r
     gRT->ResetSystem (EfiResetWarm, EFI_SUCCESS, 0, NULL);\r

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 25a1d522fe843d7750b6c2301ee60a775c1f6fb5..21ecd8584d24afd7b6ab20b1da367bda0b888d75 100644 (file)
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -108,6 +108,12 @@ CHAR16 *
 #define BM_OPTION_NAME_LEN                          sizeof ("PlatformRecovery####")\r
 extern CHAR16  *mBmLoadOptionName[];\r
 \r
+//\r
+// Maximum number of reconnect retry to repair controller; it is to limit the\r
+// number of recursive call of BmRepairAllControllers.\r
+//\r
+#define MAX_RECONNECT_REPAIR                        10\r
+\r
 /**\r
   Visitor function to be called by BmForEachVariable for each variable\r
   in variable storage.\r
@@ -145,10 +151,13 @@ typedef struct {
 \r
 /**\r
   Repair all the controllers according to the Driver Health status queried.\r
+\r
+  @param ReconnectRepairCount     To record the number of recursive call of\r
+                                  this function itself.\r
 **/\r
 VOID\r
 BmRepairAllControllers (\r
-  VOID\r
+  UINTN       ReconnectRepairCount\r
   );\r
 \r
 #define BM_HOTKEY_SIGNATURE SIGNATURE_32 ('b', 'm', 'h', 'k')\r
@@ -327,14 +336,6 @@ BmDelPartMatchInstance (
   IN     EFI_DEVICE_PATH_PROTOCOL  *Single\r
   );\r
 \r
-/**\r
-  Repair all the controllers according to the Driver Health status queried.\r
-**/\r
-VOID\r
-BmRepairAllControllers (\r
-  VOID\r
-  );\r
-\r
 /**\r
   Print the device path info.\r
 \r