[PATCH] MdePkg/SmmPeriodicSmiLib: Get Periodic SMI Context More Robustly

* [PATCH] MdePkg/SmmPeriodicSmiLib: Get Periodic SMI Context More Robustly
@ 2018-05-09  9:36 Ruiyu Ni
  2018-05-17  8:08 ` Gao, Liming
  0 siblings, 1 reply; 2+ messages in thread
From: Ruiyu Ni @ 2018-05-09  9:36 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael D Kinney, Liming Gao

The PeriodicSmiDispatchFunction() in SmmPeriodicSmiLib may assert
with "Bad CR signature".

Currently, the SetActivePeriodicSmiLibraryHandler() function
(invoked at the beginning of the PeriodicSmiDispatchFunction()
function) attempts to locate the PERIODIC_SMI_LIBRARY_HANDLER_CONTEXT
structure pointer for the current periodic SMI from a given
EFI_SMM_PERIODIC_TIMER_REGISTER_CONTEXT (RegiserContext) structure
pointer (using the CR macro).

The RegisterContext structure pointer passed to the
PeriodicSmiDispatchFunction() is assumed to point to the same
RegisterContext structure address given to the
SmmPeriodicTimerDispatch2 protocol Register() API in
PeriodicSmiEnable().

However, certain SmmPeriodicTimerDispatch2 implementation may copy
the RegisterContext to a local buffer and pass that address as the
context to PeriodicSmiDispatchFunction() in which case usage of the
CR macro to find the parent structure base fails.

The patch uses the LookupPeriodicSmiLibraryHandler() function to
find the PERIODIC_SMI_LIBRARY_HANDLER_CONTEXT structure pointer.
This works even in this scenario since the DispatchHandle returned
from the SmmPeriodicTimerDispatch2 Register() function uniquely
identifies that registration.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
---
 .../Library/SmmPeriodicSmiLib/SmmPeriodicSmiLib.c  | 38 +++++++++-------------
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/MdePkg/Library/SmmPeriodicSmiLib/SmmPeriodicSmiLib.c b/MdePkg/Library/SmmPeriodicSmiLib/SmmPeriodicSmiLib.c
index 2016af60d8..ca6967c9be 100644
--- a/MdePkg/Library/SmmPeriodicSmiLib/SmmPeriodicSmiLib.c
+++ b/MdePkg/Library/SmmPeriodicSmiLib/SmmPeriodicSmiLib.c
@@ -1,7 +1,7 @@
 /** @file
   SMM Periodic SMI Library.
 
-  Copyright (c) 2011, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
   which accompanies this distribution.  The full text of the license may be found at
@@ -144,19 +144,6 @@ typedef struct {
   UINT64                                   ElapsedTime;
 } PERIODIC_SMI_LIBRARY_HANDLER_CONTEXT;
 
-/**
- Macro that returns a pointer to a PERIODIC_SMI_LIBRARY_HANDLER_CONTEXT 
- structure based on a pointer to a RegisterContext field.
-
-**/
-#define PERIODIC_SMI_LIBRARY_HANDLER_CONTEXT_FROM_REGISTER_CONTEXT(a) \
-  CR (                                                                \
-    a,                                                                \
-    PERIODIC_SMI_LIBRARY_HANDLER_CONTEXT,                             \
-    RegisterContext,                                                  \
-    PERIODIC_SMI_LIBRARY_HANDLER_CONTEXT_SIGNATURE                    \
-    )
-
 /**
  Macro that returns a pointer to a PERIODIC_SMI_LIBRARY_HANDLER_CONTEXT 
  structure based on a pointer to a Link field.
@@ -280,26 +267,31 @@ LookupPeriodicSmiLibraryHandler (
 
 /**
   Internal worker function that sets that active periodic SMI handler based on 
-  the Context used when the periodic SMI handler was registered with the 
-  SMM Periodic Timer Dispatch 2 Protocol.  If Context is NULL, then the 
+  the DispatchHandle that was returned when the periodic SMI handler was enabled
+  with PeriodicSmiEnable(). If DispatchHandle is NULL, then the 
   state is updated to show that there is not active periodic SMI handler.
   A pointer to the active PERIODIC_SMI_LIBRARY_HANDLER_CONTEXT structure 
   is returned.
-  
-  @retval  NULL   Context is NULL.
+
+  @param [in] DispatchHandle DispatchHandle that was returned when the periodic
+                             SMI handler was enabled with PeriodicSmiEnable().
+                             This is an optional parameter that may be NULL.
+                             If this parameter is NULL, then the state is updated
+                             to show that there is not active periodic SMI handler.
+  @retval  NULL   DispatchHandle is NULL.
   @retval  other  Pointer to the PERIODIC_SMI_LIBRARY_HANDLER_CONTEXT
-                  associated with Context.
+                  associated with DispatchHandle.
   
 **/
 PERIODIC_SMI_LIBRARY_HANDLER_CONTEXT *
 SetActivePeriodicSmiLibraryHandler (
-  IN CONST VOID  *Context  OPTIONAL
+  IN EFI_HANDLE                         DispatchHandle    OPTIONAL
   )
 {
-  if (Context == NULL) {
+  if (DispatchHandle == NULL) {
     gActivePeriodicSmiLibraryHandler = NULL;
   } else {
-    gActivePeriodicSmiLibraryHandler = PERIODIC_SMI_LIBRARY_HANDLER_CONTEXT_FROM_REGISTER_CONTEXT (Context);
+    gActivePeriodicSmiLibraryHandler = LookupPeriodicSmiLibraryHandler (DispatchHandle);
   }
   return gActivePeriodicSmiLibraryHandler;
 }
@@ -798,7 +790,7 @@ PeriodicSmiDispatchFunction (
   //
   // Set the active periodic SMI handler
   //  
-  PeriodicSmiLibraryHandler = SetActivePeriodicSmiLibraryHandler (Context);
+  PeriodicSmiLibraryHandler = SetActivePeriodicSmiLibraryHandler (DispatchHandle);
   if (PeriodicSmiLibraryHandler == NULL) {
     return EFI_NOT_FOUND;
   }
-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread