On 10/25/17 03:33, Zeng, Star wrote:
> Reviewed-by: Star Zeng <[email protected]>

Commit 704b71d7e11f.

Thank you, Star!
Laszlo

> -----Original Message-----
> From: edk2-devel [mailto:[email protected]] On Behalf Of Laszlo 
> Ersek
> Sent: Tuesday, October 24, 2017 11:38 PM
> To: edk2-devel-01 <[email protected]>
> Cc: Yao, Jiewen <[email protected]>; Dong, Eric <[email protected]>; 
> Zeng, Star <[email protected]>
> Subject: [edk2] [PATCH v2] MdeModulePkg/Variable/RuntimeDxe: delete & lock 
> MOR in the absence of SMM
> 
> VariableRuntimeDxe deletes and locks the MorLock variable in
> MorLockInit(), with the argument that any protection provided by MorLock
> can be circumvented if MorLock can be overwritten by unprivileged code
> (i.e., outside of SMM).
> 
> Extend the argument and the logic to the MOR variable, which is supposed
> to be protected by MorLock. Pass Attributes=0 when deleting MorLock and
> MOR both.
> 
> This change was suggested by Star; it is inspired by earlier VariableSmm
> commit fda8f631edbb ("MdeModulePkg/Variable/RuntimeDxe: delete and lock
> OS-created MOR variable", 2017-10-03).
> 
> Cc: Eric Dong <[email protected]>
> Cc: Jiewen Yao <[email protected]>
> Cc: Star Zeng <[email protected]>
> Suggested-by: Star Zeng <[email protected]>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <[email protected]>
> ---
> 
> Notes:
>     v2:
>     - Use Attributes=0 for deleting MorLock too [Star]
>     - Branch: del_and_lock_mor_without_smm_v2
>     
>     v1:
>     - Branch: del_and_lock_mor_without_smm
>     
>     Repo: https://github.com/lersek/edk2.git
> 
>  MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c | 30 
> ++++++++++++++++++--
>  1 file changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c 
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
> index 7142e2da2073..fb4e13ab25a7 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
> @@ -78,15 +78,39 @@ MorLockInit (
>    VariableServiceSetVariable (
>      MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME,
>      &gEfiMemoryOverwriteRequestControlLockGuid,
> -    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | 
> EFI_VARIABLE_RUNTIME_ACCESS,
> -    0,
> -    NULL
> +    0,                                          // Attributes
> +    0,                                          // DataSize
> +    NULL                                        // Data
>      );
>  
>    //
>    // Need set this variable to be read-only to prevent other module set it.
>    //
>    VariableLockRequestToLock (&mVariableLock, 
> MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, 
> &gEfiMemoryOverwriteRequestControlLockGuid);
> +
> +  //
> +  // The MOR variable can effectively improve platform security only when the
> +  // MorLock variable protects the MOR variable. In turn MorLock cannot be 
> made
> +  // secure without SMM support in the platform firmware (see above).
> +  //
> +  // Thus, delete the MOR variable, should it exist for any reason (some OSes
> +  // are known to create MOR unintentionally, in an attempt to set it), then
> +  // also lock the MOR variable, in order to prevent other modules from
> +  // creating it.
> +  //
> +  VariableServiceSetVariable (
> +    MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME,
> +    &gEfiMemoryOverwriteControlDataGuid,
> +    0,                                      // Attributes
> +    0,                                      // DataSize
> +    NULL                                    // Data
> +    );
> +  VariableLockRequestToLock (
> +    &mVariableLock,
> +    MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME,
> +    &gEfiMemoryOverwriteControlDataGuid
> +    );
> +
>    return EFI_SUCCESS;
>  }
>  
> 

_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to