Hi Heyi, Just a suggestion. Is it better to use a PCD instead of a define for MAX_RECONNECT_REPAIR? So that we can easily override it in our platform dsc file.
Regards, Sunny Wang -----Original Message----- From: edk2-devel [mailto:[email protected]] On Behalf Of Heyi Guo Sent: Monday, February 26, 2018 4:30 PM To: [email protected] Cc: Ruiyu Ni <[email protected]>; Heyi Guo <[email protected]>; Eric Dong <[email protected]>; Star Zeng <[email protected]> Subject: [edk2] [PATCH 1/1] 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. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Heyi Guo <[email protected]> Cc: Star Zeng <[email protected]> Cc: Eric Dong <[email protected]> Cc: Ruiyu Ni <[email protected]> --- MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h | 6 ++++++ MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h index 25a1d522fe84..9aa86b096525 100644 --- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h +++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h @@ -108,6 +108,12 @@ CHAR16 * #define BM_OPTION_NAME_LEN sizeof ("PlatformRecovery####") extern CHAR16 *mBmLoadOptionName[]; +// +// Maximum number of reconnect retry to repair controller; it is to +limit the // number of recursive call of BmRepairAllControllers. +// +#define MAX_RECONNECT_REPAIR 10 + /** Visitor function to be called by BmForEachVariable for each variable in variable storage. diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c index ddcee8b0676f..30d70f32af84 100644 --- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c @@ -26,6 +26,12 @@ GLOBAL_REMOVE_IF_UNREFERENCED L"Reboot Required" }; +// +// Counter of reconnect retry to repair controller; it is to limit the +// number of recursive call of BmRepairAllControllers. +// +STATIC UINTN mReconnectRepairCount; + /** Return the controller name. @@ -549,7 +555,16 @@ BmRepairAllControllers ( if (ReconnectRequired) { - BmRepairAllControllers (); + if (mReconnectRepairCount < MAX_RECONNECT_REPAIR) { + mReconnectRepairCount++; + BmRepairAllControllers (); + } else { + DEBUG ((DEBUG_ERROR, "[%a:%d] Repair failed after %d retries.\n", + __FUNCTION__, __LINE__, mReconnectRepairCount)); + // Reset counter so that it will not affect calling + // BmRepairAllControllers() somewhere else + mReconnectRepairCount = 0; + } } DEBUG_CODE ( -- 2.7.4 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
