[edk2-devel] [PATCH v3 0/4] Support to unregister SMI handler inside SMI handler

[edk2-devel] [PATCH v3 0/4] Support to unregister SMI handler inside SMI handler

[Zhiguang Liu]

This patch set is to support to unregister SMI handler inside SMI handler,
also add check to not allow unregister SMI handler in other SMI handler.
This patch set also have the same logic in StandaloneMmPkg.
Because no change on the first patch, I kept the R-B for it.

V3:
Minor change on patch #2 and patch #4:
gCurrentSmiHandler -> mCurrentSmiHandler
change ASSERT to return EFI_INVALID_PARAMETER

Zhiguang Liu (4):
  MdeModulePkg/SMM: Support to unregister SMI handler inside SMI handler
  MdeModulePkg/SMM: Disallow unregister SMI handler in other SMI handler
  StandaloneMmPkg: Support to unregister MMI handler inside MMI handler
  StandaloneMmPkg: Disallow unregister MMI handler in other MMI handler

 MdeModulePkg/Core/PiSmmCore/Smi.c | 37 ++++++++++++++++++++++-------
 StandaloneMmPkg/Core/Mmi.c        | 39 +++++++++++++++++++++++--------
 2 files changed, 57 insertions(+), 19 deletions(-)

-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116202): https://edk2.groups.io/g/devel/message/116202
Mute This Topic: https://groups.io/mt/104657663/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 1/4] MdeModulePkg/SMM: Support to unregister SMI handler inside SMI handler

[Zhiguang Liu]

To support unregister SMI handler inside SMI handler itself,
get next node before SMI handler is executed, since LIST_ENTRY that
Link points to may be freed if unregister SMI handler in SMI handler
itself.

Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 MdeModulePkg/Core/PiSmmCore/Smi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c b/MdeModulePkg/Core/PiSmmCore/Smi.c
index 2985f989c3..3489c130fd 100644
--- a/MdeModulePkg/Core/PiSmmCore/Smi.c
+++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
@@ -134,8 +134,14 @@ SmiManage (
 
   Head = &SmiEntry->SmiHandlers;
 
-  for (Link = Head->ForwardLink; Link != Head; Link = Link->ForwardLink) {
+  for (Link = Head->ForwardLink; Link != Head;) {
     SmiHandler = CR (Link, SMI_HANDLER, Link, SMI_HANDLER_SIGNATURE);
+    //
+    // To support unregister SMI handler inside SMI handler itself,
+    // get next node before handler is executed, since LIST_ENTRY that
+    // Link points to may be freed if unregister SMI handler.
+    //
+    Link = Link->ForwardLink;
 
     Status = SmiHandler->Handler (
                            (EFI_HANDLE)SmiHandler,
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116203): https://edk2.groups.io/g/devel/message/116203
Mute This Topic: https://groups.io/mt/104657665/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 2/4] MdeModulePkg/SMM: Disallow unregister SMI handler in other SMI handler

[Zhiguang Liu]

In last patch, we add code support to unregister SMI handler inside
itself. However, the code doesn't support unregister SMI handler
insider other SMI handler. While this is not a must-have usage.
So add check to disallow unregister SMI handler in other SMI handler.

Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 MdeModulePkg/Core/PiSmmCore/Smi.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c b/MdeModulePkg/Core/PiSmmCore/Smi.c
index 3489c130fd..b3a81ac877 100644
--- a/MdeModulePkg/Core/PiSmmCore/Smi.c
+++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
@@ -8,7 +8,8 @@
 
 #include "PiSmmCore.h"
 
-LIST_ENTRY  mSmiEntryList = INITIALIZE_LIST_HEAD_VARIABLE (mSmiEntryList);
+SMI_HANDLER  *mCurrentSmiHandler = NULL;
+LIST_ENTRY   mSmiEntryList       = INITIALIZE_LIST_HEAD_VARIABLE (mSmiEntryList);
 
 SMI_ENTRY  mRootSmiEntry = {
   SMI_ENTRY_SIGNATURE,
@@ -142,13 +143,18 @@ SmiManage (
     // Link points to may be freed if unregister SMI handler.
     //
     Link = Link->ForwardLink;
-
-    Status = SmiHandler->Handler (
-                           (EFI_HANDLE)SmiHandler,
-                           Context,
-                           CommBuffer,
-                           CommBufferSize
-                           );
+    //
+    // Assign gCurrentSmiHandle before calling the SMI handler and
+    // set to NULL when it returns.
+    //
+    mCurrentSmiHandler = SmiHandler;
+    Status             = SmiHandler->Handler (
+                                       (EFI_HANDLE)SmiHandler,
+                                       Context,
+                                       CommBuffer,
+                                       CommBufferSize
+                                       );
+    mCurrentSmiHandler = NULL;
 
     switch (Status) {
       case EFI_INTERRUPT_PENDING:
@@ -328,6 +334,13 @@ SmiHandlerUnRegister (
     return EFI_INVALID_PARAMETER;
   }
 
+  //
+  // Do not allow to unregister SMI Handler inside other SMI Handler
+  //
+  if ((mCurrentSmiHandler != NULL) && (mCurrentSmiHandler != SmiHandler)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   SmiEntry = SmiHandler->SmiEntry;
 
   RemoveEntryList (&SmiHandler->Link);
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116204): https://edk2.groups.io/g/devel/message/116204
Mute This Topic: https://groups.io/mt/104657667/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 2/4] MdeModulePkg/SMM: Disallow unregister SMI handler in other SMI handler

[Ni, Ray]

Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Friday, March 1, 2024 11:02 AM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH v3 2/4] MdeModulePkg/SMM: Disallow unregister SMI
> handler in other SMI handler
> 
> In last patch, we add code support to unregister SMI handler inside
> itself. However, the code doesn't support unregister SMI handler
> insider other SMI handler. While this is not a must-have usage.
> So add check to disallow unregister SMI handler in other SMI handler.
> 
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  MdeModulePkg/Core/PiSmmCore/Smi.c | 29
> +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c
> b/MdeModulePkg/Core/PiSmmCore/Smi.c
> index 3489c130fd..b3a81ac877 100644
> --- a/MdeModulePkg/Core/PiSmmCore/Smi.c
> +++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
> @@ -8,7 +8,8 @@
> 
>  #include "PiSmmCore.h"
> 
> -LIST_ENTRY  mSmiEntryList = INITIALIZE_LIST_HEAD_VARIABLE
> (mSmiEntryList);
> +SMI_HANDLER  *mCurrentSmiHandler = NULL;
> +LIST_ENTRY   mSmiEntryList       = INITIALIZE_LIST_HEAD_VARIABLE
> (mSmiEntryList);
> 
>  SMI_ENTRY  mRootSmiEntry = {
>    SMI_ENTRY_SIGNATURE,
> @@ -142,13 +143,18 @@ SmiManage (
>      // Link points to may be freed if unregister SMI handler.
>      //
>      Link = Link->ForwardLink;
> -
> -    Status = SmiHandler->Handler (
> -                           (EFI_HANDLE)SmiHandler,
> -                           Context,
> -                           CommBuffer,
> -                           CommBufferSize
> -                           );
> +    //
> +    // Assign gCurrentSmiHandle before calling the SMI handler and
> +    // set to NULL when it returns.
> +    //
> +    mCurrentSmiHandler = SmiHandler;
> +    Status             = SmiHandler->Handler (
> +                                       (EFI_HANDLE)SmiHandler,
> +                                       Context,
> +                                       CommBuffer,
> +                                       CommBufferSize
> +                                       );
> +    mCurrentSmiHandler = NULL;
> 
>      switch (Status) {
>        case EFI_INTERRUPT_PENDING:
> @@ -328,6 +334,13 @@ SmiHandlerUnRegister (
>      return EFI_INVALID_PARAMETER;
>    }
> 
> +  //
> +  // Do not allow to unregister SMI Handler inside other SMI Handler
> +  //
> +  if ((mCurrentSmiHandler != NULL) && (mCurrentSmiHandler !=
> SmiHandler)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    SmiEntry = SmiHandler->SmiEntry;
> 
>    RemoveEntryList (&SmiHandler->Link);
> --
> 2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116208): https://edk2.groups.io/g/devel/message/116208
Mute This Topic: https://groups.io/mt/104657667/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 2/4] MdeModulePkg/SMM: Disallow unregister SMI handler in other SMI handler

[Laszlo Ersek]

On 3/1/24 04:01, Zhiguang Liu wrote:
> In last patch, we add code support to unregister SMI handler inside
> itself. However, the code doesn't support unregister SMI handler
> insider other SMI handler. While this is not a must-have usage.
> So add check to disallow unregister SMI handler in other SMI handler.
> 
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  MdeModulePkg/Core/PiSmmCore/Smi.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c b/MdeModulePkg/Core/PiSmmCore/Smi.c
> index 3489c130fd..b3a81ac877 100644
> --- a/MdeModulePkg/Core/PiSmmCore/Smi.c
> +++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
> @@ -8,7 +8,8 @@
>  
>  #include "PiSmmCore.h"
>  
> -LIST_ENTRY  mSmiEntryList = INITIALIZE_LIST_HEAD_VARIABLE (mSmiEntryList);
> +SMI_HANDLER  *mCurrentSmiHandler = NULL;
> +LIST_ENTRY   mSmiEntryList       = INITIALIZE_LIST_HEAD_VARIABLE (mSmiEntryList);
>  
>  SMI_ENTRY  mRootSmiEntry = {
>    SMI_ENTRY_SIGNATURE,
> @@ -142,13 +143,18 @@ SmiManage (
>      // Link points to may be freed if unregister SMI handler.
>      //
>      Link = Link->ForwardLink;
> -
> -    Status = SmiHandler->Handler (
> -                           (EFI_HANDLE)SmiHandler,
> -                           Context,
> -                           CommBuffer,
> -                           CommBufferSize
> -                           );
> +    //
> +    // Assign gCurrentSmiHandle before calling the SMI handler and
> +    // set to NULL when it returns.
> +    //
> +    mCurrentSmiHandler = SmiHandler;
> +    Status             = SmiHandler->Handler (
> +                                       (EFI_HANDLE)SmiHandler,
> +                                       Context,
> +                                       CommBuffer,
> +                                       CommBufferSize
> +                                       );
> +    mCurrentSmiHandler = NULL;
>  
>      switch (Status) {
>        case EFI_INTERRUPT_PENDING:
> @@ -328,6 +334,13 @@ SmiHandlerUnRegister (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> +  //
> +  // Do not allow to unregister SMI Handler inside other SMI Handler
> +  //
> +  if ((mCurrentSmiHandler != NULL) && (mCurrentSmiHandler != SmiHandler)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    SmiEntry = SmiHandler->SmiEntry;
>  
>    RemoveEntryList (&SmiHandler->Link);

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116240): https://edk2.groups.io/g/devel/message/116240
Mute This Topic: https://groups.io/mt/104657667/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 3/4] StandaloneMmPkg: Support to unregister MMI handler inside MMI handler

[Zhiguang Liu]

To support unregister MMI handler inside MMI handler itself,
get next node before MMI handler is executed, since LIST_ENTRY that
Link points to may be freed if unregister MMI handler in MMI handler
itself.

Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 StandaloneMmPkg/Core/Mmi.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/StandaloneMmPkg/Core/Mmi.c b/StandaloneMmPkg/Core/Mmi.c
index 0de6fd17fc..c1a1d76e85 100644
--- a/StandaloneMmPkg/Core/Mmi.c
+++ b/StandaloneMmPkg/Core/Mmi.c
@@ -154,9 +154,14 @@ MmiManage (
     Head = &MmiEntry->MmiHandlers;
   }
 
-  for (Link = Head->ForwardLink; Link != Head; Link = Link->ForwardLink) {
+  for (Link = Head->ForwardLink; Link != Head;) {
     MmiHandler = CR (Link, MMI_HANDLER, Link, MMI_HANDLER_SIGNATURE);
-
+    //
+    // To support unregister MMI handler inside MMI handler itself,
+    // get next node before handler is executed, since LIST_ENTRY that
+    // Link points to may be freed if unregister MMI handler.
+    //
+    Link   = Link->ForwardLink;
     Status = MmiHandler->Handler (
                            (EFI_HANDLE)MmiHandler,
                            Context,
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116205): https://edk2.groups.io/g/devel/message/116205
Mute This Topic: https://groups.io/mt/104657668/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v3 4/4] StandaloneMmPkg: Disallow unregister MMI handler in other MMI handler

[Zhiguang Liu]

In last patch, we add code support to unregister MMI handler inside
itself. However, the code doesn't support unregister MMI handler
insider other MMI handler. While this is not a must-have usage.
So add check to disallow unregister MMI handler in other MMI handler.

Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 StandaloneMmPkg/Core/Mmi.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/StandaloneMmPkg/Core/Mmi.c b/StandaloneMmPkg/Core/Mmi.c
index c1a1d76e85..9e52072bf7 100644
--- a/StandaloneMmPkg/Core/Mmi.c
+++ b/StandaloneMmPkg/Core/Mmi.c
@@ -36,8 +36,9 @@ typedef struct {
   MMI_ENTRY                     *MmiEntry;
 } MMI_HANDLER;
 
-LIST_ENTRY  mRootMmiHandlerList = INITIALIZE_LIST_HEAD_VARIABLE (mRootMmiHandlerList);
-LIST_ENTRY  mMmiEntryList       = INITIALIZE_LIST_HEAD_VARIABLE (mMmiEntryList);
+LIST_ENTRY   mRootMmiHandlerList = INITIALIZE_LIST_HEAD_VARIABLE (mRootMmiHandlerList);
+LIST_ENTRY   mMmiEntryList       = INITIALIZE_LIST_HEAD_VARIABLE (mMmiEntryList);
+MMI_HANDLER  *mCurrentMmiHandler = NULL;
 
 /**
   Finds the MMI entry for the requested handler type.
@@ -161,13 +162,19 @@ MmiManage (
     // get next node before handler is executed, since LIST_ENTRY that
     // Link points to may be freed if unregister MMI handler.
     //
-    Link   = Link->ForwardLink;
-    Status = MmiHandler->Handler (
-                           (EFI_HANDLE)MmiHandler,
-                           Context,
-                           CommBuffer,
-                           CommBufferSize
-                           );
+    Link = Link->ForwardLink;
+    //
+    // Assign gCurrentMmiHandle before calling the MMI handler and
+    // set to NULL when it returns.
+    //
+    mCurrentMmiHandler = MmiHandler;
+    Status             = MmiHandler->Handler (
+                                       (EFI_HANDLE)MmiHandler,
+                                       Context,
+                                       CommBuffer,
+                                       CommBufferSize
+                                       );
+    mCurrentMmiHandler = NULL;
 
     switch (Status) {
       case EFI_INTERRUPT_PENDING:
@@ -314,6 +321,13 @@ MmiHandlerUnRegister (
     return EFI_INVALID_PARAMETER;
   }
 
+  //
+  // Do not allow to unregister MMI Handler inside other MMI Handler
+  //
+  if ((mCurrentMmiHandler != NULL) && (mCurrentMmiHandler != MmiHandler)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   MmiEntry = MmiHandler->MmiEntry;
 
   RemoveEntryList (&MmiHandler->Link);
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116206): https://edk2.groups.io/g/devel/message/116206
Mute This Topic: https://groups.io/mt/104657669/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 4/4] StandaloneMmPkg: Disallow unregister MMI handler in other MMI handler

[Ni, Ray]

Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Friday, March 1, 2024 11:02 AM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Sami Mujawar <sami.mujawar@arm.com>
> Subject: [PATCH v3 4/4] StandaloneMmPkg: Disallow unregister MMI handler
> in other MMI handler
> 
> In last patch, we add code support to unregister MMI handler inside
> itself. However, the code doesn't support unregister MMI handler
> insider other MMI handler. While this is not a must-have usage.
> So add check to disallow unregister MMI handler in other MMI handler.
> 
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  StandaloneMmPkg/Core/Mmi.c | 32 +++++++++++++++++++++++---------
>  1 file changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/StandaloneMmPkg/Core/Mmi.c b/StandaloneMmPkg/Core/Mmi.c
> index c1a1d76e85..9e52072bf7 100644
> --- a/StandaloneMmPkg/Core/Mmi.c
> +++ b/StandaloneMmPkg/Core/Mmi.c
> @@ -36,8 +36,9 @@ typedef struct {
>    MMI_ENTRY                     *MmiEntry;
>  } MMI_HANDLER;
> 
> -LIST_ENTRY  mRootMmiHandlerList = INITIALIZE_LIST_HEAD_VARIABLE
> (mRootMmiHandlerList);
> -LIST_ENTRY  mMmiEntryList       = INITIALIZE_LIST_HEAD_VARIABLE
> (mMmiEntryList);
> +LIST_ENTRY   mRootMmiHandlerList = INITIALIZE_LIST_HEAD_VARIABLE
> (mRootMmiHandlerList);
> +LIST_ENTRY   mMmiEntryList       = INITIALIZE_LIST_HEAD_VARIABLE
> (mMmiEntryList);
> +MMI_HANDLER  *mCurrentMmiHandler = NULL;
> 
>  /**
>    Finds the MMI entry for the requested handler type.
> @@ -161,13 +162,19 @@ MmiManage (
>      // get next node before handler is executed, since LIST_ENTRY that
>      // Link points to may be freed if unregister MMI handler.
>      //
> -    Link   = Link->ForwardLink;
> -    Status = MmiHandler->Handler (
> -                           (EFI_HANDLE)MmiHandler,
> -                           Context,
> -                           CommBuffer,
> -                           CommBufferSize
> -                           );
> +    Link = Link->ForwardLink;
> +    //
> +    // Assign gCurrentMmiHandle before calling the MMI handler and
> +    // set to NULL when it returns.
> +    //
> +    mCurrentMmiHandler = MmiHandler;
> +    Status             = MmiHandler->Handler (
> +                                       (EFI_HANDLE)MmiHandler,
> +                                       Context,
> +                                       CommBuffer,
> +                                       CommBufferSize
> +                                       );
> +    mCurrentMmiHandler = NULL;
> 
>      switch (Status) {
>        case EFI_INTERRUPT_PENDING:
> @@ -314,6 +321,13 @@ MmiHandlerUnRegister (
>      return EFI_INVALID_PARAMETER;
>    }
> 
> +  //
> +  // Do not allow to unregister MMI Handler inside other MMI Handler
> +  //
> +  if ((mCurrentMmiHandler != NULL) && (mCurrentMmiHandler !=
> MmiHandler)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    MmiEntry = MmiHandler->MmiEntry;
> 
>    RemoveEntryList (&MmiHandler->Link);
> --
> 2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116209): https://edk2.groups.io/g/devel/message/116209
Mute This Topic: https://groups.io/mt/104657669/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 4/4] StandaloneMmPkg: Disallow unregister MMI handler in other MMI handler

[Laszlo Ersek]

On 3/1/24 04:01, Zhiguang Liu wrote:
> In last patch, we add code support to unregister MMI handler inside
> itself. However, the code doesn't support unregister MMI handler
> insider other MMI handler. While this is not a must-have usage.
> So add check to disallow unregister MMI handler in other MMI handler.
> 
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  StandaloneMmPkg/Core/Mmi.c | 32 +++++++++++++++++++++++---------
>  1 file changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/StandaloneMmPkg/Core/Mmi.c b/StandaloneMmPkg/Core/Mmi.c
> index c1a1d76e85..9e52072bf7 100644
> --- a/StandaloneMmPkg/Core/Mmi.c
> +++ b/StandaloneMmPkg/Core/Mmi.c
> @@ -36,8 +36,9 @@ typedef struct {
>    MMI_ENTRY                     *MmiEntry;
>  } MMI_HANDLER;
>  
> -LIST_ENTRY  mRootMmiHandlerList = INITIALIZE_LIST_HEAD_VARIABLE (mRootMmiHandlerList);
> -LIST_ENTRY  mMmiEntryList       = INITIALIZE_LIST_HEAD_VARIABLE (mMmiEntryList);
> +LIST_ENTRY   mRootMmiHandlerList = INITIALIZE_LIST_HEAD_VARIABLE (mRootMmiHandlerList);
> +LIST_ENTRY   mMmiEntryList       = INITIALIZE_LIST_HEAD_VARIABLE (mMmiEntryList);
> +MMI_HANDLER  *mCurrentMmiHandler = NULL;
>  
>  /**
>    Finds the MMI entry for the requested handler type.
> @@ -161,13 +162,19 @@ MmiManage (
>      // get next node before handler is executed, since LIST_ENTRY that
>      // Link points to may be freed if unregister MMI handler.
>      //
> -    Link   = Link->ForwardLink;
> -    Status = MmiHandler->Handler (
> -                           (EFI_HANDLE)MmiHandler,
> -                           Context,
> -                           CommBuffer,
> -                           CommBufferSize
> -                           );
> +    Link = Link->ForwardLink;
> +    //
> +    // Assign gCurrentMmiHandle before calling the MMI handler and
> +    // set to NULL when it returns.
> +    //
> +    mCurrentMmiHandler = MmiHandler;
> +    Status             = MmiHandler->Handler (
> +                                       (EFI_HANDLE)MmiHandler,
> +                                       Context,
> +                                       CommBuffer,
> +                                       CommBufferSize
> +                                       );
> +    mCurrentMmiHandler = NULL;
>  
>      switch (Status) {
>        case EFI_INTERRUPT_PENDING:
> @@ -314,6 +321,13 @@ MmiHandlerUnRegister (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> +  //
> +  // Do not allow to unregister MMI Handler inside other MMI Handler
> +  //
> +  if ((mCurrentMmiHandler != NULL) && (mCurrentMmiHandler != MmiHandler)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
>    MmiEntry = MmiHandler->MmiEntry;
>  
>    RemoveEntryList (&MmiHandler->Link);

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116241): https://edk2.groups.io/g/devel/message/116241
Mute This Topic: https://groups.io/mt/104657669/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 0/4] Support to unregister SMI handler inside SMI handler

[Laszlo Ersek]

On 3/1/24 04:01, Zhiguang Liu wrote:
> This patch set is to support to unregister SMI handler inside SMI handler,
> also add check to not allow unregister SMI handler in other SMI handler.
> This patch set also have the same logic in StandaloneMmPkg.
> Because no change on the first patch, I kept the R-B for it.
> 
> V3:
> Minor change on patch #2 and patch #4:
> gCurrentSmiHandler -> mCurrentSmiHandler
> change ASSERT to return EFI_INVALID_PARAMETER
> 
> Zhiguang Liu (4):
>   MdeModulePkg/SMM: Support to unregister SMI handler inside SMI handler
>   MdeModulePkg/SMM: Disallow unregister SMI handler in other SMI handler
>   StandaloneMmPkg: Support to unregister MMI handler inside MMI handler
>   StandaloneMmPkg: Disallow unregister MMI handler in other MMI handler
> 
>  MdeModulePkg/Core/PiSmmCore/Smi.c | 37 ++++++++++++++++++++++-------
>  StandaloneMmPkg/Core/Mmi.c        | 39 +++++++++++++++++++++++--------
>  2 files changed, 57 insertions(+), 19 deletions(-)
> 

Merged as

     3  ae1079b386a5 MdeModulePkg/SMM: Support to unregister SMI handler inside SMI handler
     4  17b28722008e MdeModulePkg/SMM: Disallow unregister SMI handler in other SMI handler
     5  049ff6c39c73 StandaloneMmPkg: Support to unregister MMI handler inside MMI handler
     6  2ec8f0c6407f StandaloneMmPkg: Disallow unregister MMI handler in other MMI handler

via <https://github.com/tianocore/edk2/pull/5432>.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116282): https://edk2.groups.io/g/devel/message/116282
Mute This Topic: https://groups.io/mt/104657663/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-