diff options
-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 c85d12d951..b83f8c9f4b 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 f1111b3b6b..9f750d6780 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;
}
|