Sync patches r14252, r14258, r14283 and r14284 from main trunk.

1. If DataSize or VariableNameSize is near MAX_ADDRESS, this can cause the computed PayLoadSize to overflow to a small value and pass the check in InitCommunicateBuffer(). To protect against this vulnerability, check DataSize and VariableNameSize to make sure PayloadSize doesn't overflow.
2. Update SMM variable DXE driver GetNextVariable interface to comply with UEFI spec VariableNameSize is the returned buffer size. GetNextVariable should behavior correct if it is bigger than SMM communication buffer or less than string size of VariableName. 
3. Update code not to block application/driver load when event log is full.
4. MdeModulePkg/AtaBus: AtaBusDxe module would ignore ATA Pass Thru Protocol instances that do not have the LOGICAL attribute set.

git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/branches/UDK2010.SR1@14287 6f19259b-4bc3-4df7-8a09-765794883524

7 files changed, 152 insertions, 8 deletions

diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.c b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.c
index 11e90f9..6732fd5 100644
--- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.c
+++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.c
@@ -4,7 +4,7 @@
   This file implements protocol interfaces: Driver Binding protocol,

   Block IO protocol and DiskInfo protocol.

 

-  Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.<BR>

+  Copyright (c) 2009 - 2013, 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

@@ -630,11 +630,36 @@ AtaBusDriverBindingSupported (
   }

 

   //

+  // Test to see if this ATA Pass Thru Protocol is for a LOGICAL channel

+  //

+  if ((AtaPassThru->Mode->Attributes & EFI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL) == 0) {

+    //

+    // Close the I/O Abstraction(s) used to perform the supported test

+    //

+    gBS->CloseProtocol (

+          Controller,

+          &gEfiAtaPassThruProtocolGuid,

+          This->DriverBindingHandle,

+          Controller

+          );

+    return EFI_UNSUPPORTED;

+  }

+

+  //

   // Test RemainingDevicePath is valid or not.

   //

   if ((RemainingDevicePath != NULL) && !IsDevicePathEnd (RemainingDevicePath)) {

     Status = AtaPassThru->GetDevice (AtaPassThru, RemainingDevicePath, &Port, &PortMultiplierPort);

     if (EFI_ERROR (Status)) {

+      //

+      // Close the I/O Abstraction(s) used to perform the supported test

+      //

+      gBS->CloseProtocol (

+            Controller,

+            &gEfiAtaPassThruProtocolGuid,

+            This->DriverBindingHandle,

+            Controller

+            );

       return Status;

     }

   }

diff --git a/MdeModulePkg/Include/Guid/SmmVariableCommon.h b/MdeModulePkg/Include/Guid/SmmVariableCommon.h
index c7fde00..478fd05 100644
--- a/MdeModulePkg/Include/Guid/SmmVariableCommon.h
+++ b/MdeModulePkg/Include/Guid/SmmVariableCommon.h
@@ -1,7 +1,7 @@
 /** @file

   The file defined some common structures used for communicating between SMM variable module and SMM variable wrapper module.

 

-Copyright (c) 2011, Intel Corporation. All rights reserved.<BR>

+Copyright (c) 2011 - 2013, 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 that accompanies this distribution.  

 The full text of the license may be found at

@@ -87,7 +87,7 @@ typedef struct {
 ///

 typedef struct {

   EFI_GUID    Guid;

-  UINTN       NameSize;

+  UINTN       NameSize;     // Return name buffer size

   CHAR16      Name[1];

 } SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME;

 

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 3aa45c2..f21ed58 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -2580,6 +2580,11 @@ VariableCommonInitialize (
   ASSERT(VariableStoreHeader->Size == VariableStoreLength);

     

   //

+  // The max variable or hardware error variable size should be < variable store size.

+  //

+  ASSERT(MAX (PcdGet32 (PcdMaxVariableSize), PcdGet32 (PcdMaxHardwareErrorVariableSize)) < VariableStoreLength);

+

+  //

   // Parse non-volatile variable data and get last variable offset.

   //

   NextVariable  = GetStartPointer ((VARIABLE_STORE_HEADER *)(UINTN)VariableStoreBase);

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 2a59ac1..c85d12d 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -198,6 +198,16 @@ RuntimeServiceGetVariable (
     return EFI_INVALID_PARAMETER;

   }

 

+  if (*DataSize >= mVariableBufferSize) {

+    //

+    // DataSize may be near MAX_ADDRESS incorrectly, this can cause the computed PayLoadSize to

+    // overflow to a small value and pass the check in InitCommunicateBuffer().

+    // To protect against this vulnerability, return EFI_INVALID_PARAMETER if DataSize is >= mVariableBufferSize.

+    // And there will be further check to ensure the total size is also not > mVariableBufferSize.

+    //

+    return EFI_INVALID_PARAMETER;

+  }

+

   AcquireLockOnlyAtBootTime(&mVariableServicesLock);

 

   //

@@ -270,27 +280,58 @@ RuntimeServiceGetNextVariableName (
   EFI_STATUS                                      Status;

   UINTN                                           PayloadSize;

   SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *SmmGetNextVariableName;

+  UINTN                                           SmmCommBufPayloadSize;

 

   if (VariableNameSize == NULL || VariableName == NULL || VendorGuid == NULL) {

     return EFI_INVALID_PARAMETER;

   }

 

+  //

+  // SMM Communication Buffer max payload size

+  //

+  SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);

+

+  //

+  // If input string exceeds SMM payload limit. Return failure

+  //

+  if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {

+    return EFI_INVALID_PARAMETER;

+  }

+

   AcquireLockOnlyAtBootTime(&mVariableServicesLock);

 

   //

   // Init the communicate buffer. The buffer data size is:

   // SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.

   //

-  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + *VariableNameSize; 

+  if (*VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {

+    //

+    // If output buffer exceed SMM payload limit. Trim output buffer to SMM payload size

+    //

+    *VariableNameSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);

+  }

+  //

+  // Payload should be Guid + NameSize + MAX of Input & Output buffer

+  //

+  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + MAX (*VariableNameSize, StrSize (VariableName));

+

+

   Status = InitCommunicateBuffer ((VOID **)&SmmGetNextVariableName, PayloadSize, SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);

   if (EFI_ERROR (Status)) {

     goto Done;

   }

   ASSERT (SmmGetNextVariableName != NULL);

 

+  //

+  // SMM comm buffer->NameSize is buffer size for return string

+  //

   SmmGetNextVariableName->NameSize = *VariableNameSize;

+

   CopyGuid (&SmmGetNextVariableName->Guid, VendorGuid);

-  CopyMem (SmmGetNextVariableName->Name, VariableName, *VariableNameSize);

+  //

+  // Copy whole string

+  //

+  CopyMem (SmmGetNextVariableName->Name, VariableName, StrSize (VariableName));

 

   //

   // Send data to SMM

@@ -355,6 +396,16 @@ RuntimeServiceSetVariable (
     return EFI_INVALID_PARAMETER;

   }

 

+  if (DataSize >= mVariableBufferSize) {

+    //

+    // DataSize may be near MAX_ADDRESS incorrectly, this can cause the computed PayLoadSize to

+    // overflow to a small value and pass the check in InitCommunicateBuffer().

+    // To protect against this vulnerability, return EFI_INVALID_PARAMETER if DataSize is >= mVariableBufferSize.

+    // And there will be further check to ensure the total size is also not > mVariableBufferSize.

+    //

+    return EFI_INVALID_PARAMETER;

+  }

+

   AcquireLockOnlyAtBootTime(&mVariableServicesLock);

  

   //

diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
index eae68ea..665e4b6 100644
--- a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
@@ -15,7 +15,7 @@
   TcgMeasureGptTable() function will receive untrusted GPT partition table, and parse

   partition data carefully.

 

-Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.<BR>

+Copyright (c) 2009 - 2013, 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 

@@ -664,6 +664,14 @@ TcgMeasurePeImage (
              &EventNumber,

              &EventLogLastEntry

              );

+  if (Status == EFI_OUT_OF_RESOURCES) {

+    //

+    // Out of resource here means the image is hashed and its result is extended to PCR.

+    // But the event log cann't be saved since log area is full.

+    // Just return EFI_SUCCESS in order not to block the image load.

+    //

+    Status = EFI_SUCCESS;

+  }

 

 Finish:

   FreePool (TcgEvent);

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
index 07fe99b..6d00513 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
@@ -2810,6 +2810,11 @@ VariableCommonInitialize (
   ASSERT(VariableStoreHeader->Size == VariableStoreLength);

 

   //

+  // The max variable or hardware error variable size should be < variable store size.

+  //

+  ASSERT(MAX (PcdGet32 (PcdMaxVariableSize), PcdGet32 (PcdMaxHardwareErrorVariableSize)) < VariableStoreLength);

+

+  //

   // Parse non-volatile variable data and get last variable offset.

   //

   NextVariable  = GetStartPointer ((VARIABLE_STORE_HEADER *)(UINTN)VariableStoreBase);

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
index a785476..f1111b3 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -214,6 +214,16 @@ RuntimeServiceGetVariable (
     return EFI_INVALID_PARAMETER;

   }

 

+  if (*DataSize >= mVariableBufferSize) {

+    //

+    // DataSize may be near MAX_ADDRESS incorrectly, this can cause the computed PayLoadSize to

+    // overflow to a small value and pass the check in InitCommunicateBuffer().

+    // To protect against this vulnerability, return EFI_INVALID_PARAMETER if DataSize is >= mVariableBufferSize.

+    // And there will be further check to ensure the total size is also not > mVariableBufferSize.

+    //

+    return EFI_INVALID_PARAMETER;

+  }

+

   AcquireLockOnlyAtBootTime(&mVariableServicesLock);

 

   //

@@ -286,27 +296,57 @@ RuntimeServiceGetNextVariableName (
   EFI_STATUS                                      Status;

   UINTN                                           PayloadSize;

   SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *SmmGetNextVariableName;

+  UINTN                                           SmmCommBufPayloadSize;

 

   if (VariableNameSize == NULL || VariableName == NULL || VendorGuid == NULL) {

     return EFI_INVALID_PARAMETER;

   }

 

+  //

+  // SMM Communication Buffer max payload size

+  //

+  SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);

+

+  //

+  // If input string exceeds SMM payload limit. Return failure

+  //

+  if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {

+    return EFI_INVALID_PARAMETER;

+  }

+

   AcquireLockOnlyAtBootTime(&mVariableServicesLock);

 

   //

   // Init the communicate buffer. The buffer data size is:

   // SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.

   //

-  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + *VariableNameSize; 

+  if (*VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {

+    //

+    // If output buffer exceed SMM payload limit. Trim output buffer to SMM payload size

+    //

+    *VariableNameSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);

+  }

+  //

+  // Payload should be Guid + NameSize + MAX of Input & Output buffer

+  //

+  PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name) + MAX (*VariableNameSize, StrSize (VariableName));

+

   Status = InitCommunicateBuffer ((VOID **)&SmmGetNextVariableName, PayloadSize, SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);

   if (EFI_ERROR (Status)) {

     goto Done;

   }

   ASSERT (SmmGetNextVariableName != NULL);

 

+  //

+  // SMM comm buffer->NameSize is buffer size for return string

+  //

   SmmGetNextVariableName->NameSize = *VariableNameSize;

+

   CopyGuid (&SmmGetNextVariableName->Guid, VendorGuid);

-  CopyMem (SmmGetNextVariableName->Name, VariableName, *VariableNameSize);

+  //

+  // Copy whole string

+  //

+  CopyMem (SmmGetNextVariableName->Name, VariableName, StrSize (VariableName));

 

   //

   // Send data to SMM

@@ -374,6 +414,16 @@ RuntimeServiceSetVariable (
     return EFI_INVALID_PARAMETER;

   }

 

+  if (DataSize >= mVariableBufferSize) {

+    //

+    // DataSize may be near MAX_ADDRESS incorrectly, this can cause the computed PayLoadSize to

+    // overflow to a small value and pass the check in InitCommunicateBuffer().

+    // To protect against this vulnerability, return EFI_INVALID_PARAMETER if DataSize is >= mVariableBufferSize.

+    // And there will be further check to ensure the total size is also not > mVariableBufferSize.

+    //

+    return EFI_INVALID_PARAMETER;

+  }

+

   AcquireLockOnlyAtBootTime(&mVariableServicesLock);

  

   //