diff options
author | czhang46 <czhang46@6f19259b-4bc3-4df7-8a09-765794883524> | 2013-04-15 01:56:31 +0000 |
---|---|---|
committer | czhang46 <czhang46@6f19259b-4bc3-4df7-8a09-765794883524> | 2013-04-15 01:56:31 +0000 |
commit | 3a146f2a7dca7cc279d752180d4cf728cd78a7b3 (patch) | |
tree | cecbf428047ee1cff48320fecd88b781478fe2f2 | |
parent | 387653a431766dbaaf76f7743cefc7aa5350b967 (diff) | |
download | edk2-3a146f2a7dca7cc279d752180d4cf728cd78a7b3.zip edk2-3a146f2a7dca7cc279d752180d4cf728cd78a7b3.tar.gz edk2-3a146f2a7dca7cc279d752180d4cf728cd78a7b3.tar.bz2 |
Fix SMM Variable driver stack GetVariable return INVALID_PARAMETER when DataSize is bigger than SMM communication buffer.
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
Reviewed-by : Dong Guo <guo.dong@intel.com>
Reviewed-by : Fu Siyuan <siyuan.fu@intel.com>
git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@14276 6f19259b-4bc3-4df7-8a09-765794883524
-rw-r--r-- | MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 38 | ||||
-rw-r--r-- | SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c | 38 |
2 files changed, 56 insertions, 20 deletions
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c index c85d12d..b83f8c9 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c @@ -189,6 +189,8 @@ RuntimeServiceGetVariable ( EFI_STATUS Status;
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *SmmVariableHeader;
+ UINTN SmmCommBufPayloadSize;
+ UINTN TempDataSize;
if (VariableName == NULL || VendorGuid == NULL || DataSize == NULL) {
return EFI_INVALID_PARAMETER;
@@ -198,13 +200,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.
- //
+ //
+ // SMM Communication Buffer max payload size
+ //
+ SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);
+ TempDataSize = *DataSize;
+
+ //
+ // If VariableName exceeds SMM payload limit. Return failure
+ //
+ if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {
return EFI_INVALID_PARAMETER;
}
@@ -214,7 +219,14 @@ RuntimeServiceGetVariable ( // 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_ACCESS_VARIABLE, Name) + StrSize (VariableName) + *DataSize;
+ if (TempDataSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - StrSize (VariableName)) {
+ //
+ // If output data buffer exceed SMM payload limit. Trim output buffer to SMM payload size
+ //
+ TempDataSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - StrSize (VariableName);
+ }
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + StrSize (VariableName) + TempDataSize;
+
Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize, SMM_VARIABLE_FUNCTION_GET_VARIABLE);
if (EFI_ERROR (Status)) {
goto Done;
@@ -222,7 +234,7 @@ RuntimeServiceGetVariable ( ASSERT (SmmVariableHeader != NULL);
CopyGuid (&SmmVariableHeader->Guid, VendorGuid);
- SmmVariableHeader->DataSize = *DataSize;
+ SmmVariableHeader->DataSize = TempDataSize;
SmmVariableHeader->NameSize = StrSize (VariableName);
if (Attributes == NULL) {
SmmVariableHeader->Attributes = 0;
@@ -239,7 +251,13 @@ RuntimeServiceGetVariable ( //
// Get data from SMM.
//
- *DataSize = SmmVariableHeader->DataSize;
+ if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
+ //
+ // SMM CommBuffer DataSize can be a trimed value
+ // Only update DataSize when needed
+ //
+ *DataSize = SmmVariableHeader->DataSize;
+ }
if (Attributes != NULL) {
*Attributes = SmmVariableHeader->Attributes;
}
diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c index f1111b3..9f750d6 100644 --- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c +++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c @@ -205,6 +205,8 @@ RuntimeServiceGetVariable ( EFI_STATUS Status;
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *SmmVariableHeader;
+ UINTN SmmCommBufPayloadSize;
+ UINTN TempDataSize;
if (VariableName == NULL || VendorGuid == NULL || DataSize == NULL) {
return EFI_INVALID_PARAMETER;
@@ -214,13 +216,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.
- //
+ //
+ // SMM Communication Buffer max payload size
+ //
+ SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);
+ TempDataSize = *DataSize;
+
+ //
+ // If VariableName exceeds SMM payload limit. Return failure
+ //
+ if (StrSize (VariableName) > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {
return EFI_INVALID_PARAMETER;
}
@@ -230,7 +235,14 @@ RuntimeServiceGetVariable ( // 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_ACCESS_VARIABLE, Name) + StrSize (VariableName) + *DataSize;
+ if (TempDataSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - StrSize (VariableName)) {
+ //
+ // If output data buffer exceed SMM payload limit. Trim output buffer to SMM payload size
+ //
+ TempDataSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - StrSize (VariableName);
+ }
+ PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + StrSize (VariableName) + TempDataSize;
+
Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader, PayloadSize, SMM_VARIABLE_FUNCTION_GET_VARIABLE);
if (EFI_ERROR (Status)) {
goto Done;
@@ -238,7 +250,7 @@ RuntimeServiceGetVariable ( ASSERT (SmmVariableHeader != NULL);
CopyGuid (&SmmVariableHeader->Guid, VendorGuid);
- SmmVariableHeader->DataSize = *DataSize;
+ SmmVariableHeader->DataSize = TempDataSize;
SmmVariableHeader->NameSize = StrSize (VariableName);
if (Attributes == NULL) {
SmmVariableHeader->Attributes = 0;
@@ -255,7 +267,13 @@ RuntimeServiceGetVariable ( //
// Get data from SMM.
//
- *DataSize = SmmVariableHeader->DataSize;
+ if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
+ //
+ // SMM CommBuffer DataSize can be a trimed value
+ // Only update DataSize when needed
+ //
+ *DataSize = SmmVariableHeader->DataSize;
+ }
if (Attributes != NULL) {
*Attributes = SmmVariableHeader->Attributes;
}
|