Reviewed-by: Liming Gao <liming....@intel.com>

>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Ruiyu Ni
>Sent: Wednesday, May 09, 2018 5:37 PM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Gao, Liming
><liming....@intel.com>
>Subject: [edk2] [PATCH] MdePkg/SmmPeriodicSmiLib: Get Periodic SMI
>Context More Robustly
>
>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...@intel.com>
>Cc: Michael D Kinney <michael.d.kin...@intel.com>
>Cc: Liming Gao <liming....@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
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to