diff options
author | lzeng14 <lzeng14@6f19259b-4bc3-4df7-8a09-765794883524> | 2013-05-07 05:38:32 +0000 |
---|---|---|
committer | lzeng14 <lzeng14@6f19259b-4bc3-4df7-8a09-765794883524> | 2013-05-07 05:38:32 +0000 |
commit | 5e5bb2a9baefcd2f231696ea94576dab5565fbfb (patch) | |
tree | d4cfd6bc4bce3b30fcf84eb78d2c1fa55ff72328 /SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c | |
parent | 845a7fe028fd29ae0bde9d68cb2fd516f8f6608b (diff) | |
download | edk2-platforms-5e5bb2a9baefcd2f231696ea94576dab5565fbfb.tar.xz |
1. Fix TOCTOU issue in VariableSmm, FtwSmm, FpdtSmm, SmmCorePerformance SMM handler. For VariableSmm, pre-allocate a mVariableBufferPayload buffer with mVariableBufferPayloadSize(match with mVariableBufferPayloadSize in VariableSmmRuntimeDxe) to hold communicate buffer payload to avoid TOCTOU issue.
2. Add check to ensure CommBufferPayloadSize not exceed mVariableBufferPayloadSize or is enough to hold function structure in VariableSmm and FtwSmm.
3. Align FtwGetLastWrite() in FaultTolerantWriteSmmDxe.c to FtwGetLastWrite() in FaultTolerantWrite.c.
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@14325 6f19259b-4bc3-4df7-8a09-765794883524
Diffstat (limited to 'SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c')
-rw-r--r-- | SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c | 48 |
1 files changed, 16 insertions, 32 deletions
diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c index cdd407d66b..b7c7f4ffb7 100644 --- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c +++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableSmmRuntimeDxe.c @@ -52,6 +52,7 @@ EFI_SMM_COMMUNICATION_PROTOCOL *mSmmCommunication = NULL; UINT8 *mVariableBuffer = NULL;
UINT8 *mVariableBufferPhysical = NULL;
UINTN mVariableBufferSize;
+UINTN mVariableBufferPayloadSize;
EFI_LOCK mVariableServicesLock;
/**
@@ -205,7 +206,6 @@ RuntimeServiceGetVariable ( EFI_STATUS Status;
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *SmmVariableHeader;
- UINTN SmmCommBufPayloadSize;
UINTN TempDataSize;
UINTN VariableNameSize;
@@ -217,17 +217,13 @@ RuntimeServiceGetVariable ( return EFI_INVALID_PARAMETER;
}
- //
- // SMM Communication Buffer max payload size
- //
- SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);
TempDataSize = *DataSize;
VariableNameSize = StrSize (VariableName);
//
// If VariableName exceeds SMM payload limit. Return failure
//
- if (VariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {
+ if (VariableNameSize > mVariableBufferPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {
return EFI_INVALID_PARAMETER;
}
@@ -237,11 +233,11 @@ RuntimeServiceGetVariable ( // Init the communicate buffer. The buffer data size is:
// SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.
//
- if (TempDataSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize) {
+ if (TempDataSize > mVariableBufferPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize) {
//
// 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) - VariableNameSize;
+ TempDataSize = mVariableBufferPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize;
}
PayloadSize = OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + VariableNameSize + TempDataSize;
@@ -316,7 +312,6 @@ RuntimeServiceGetNextVariableName ( EFI_STATUS Status;
UINTN PayloadSize;
SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *SmmGetNextVariableName;
- UINTN SmmCommBufPayloadSize;
UINTN OutVariableNameSize;
UINTN InVariableNameSize;
@@ -324,17 +319,13 @@ RuntimeServiceGetNextVariableName ( return EFI_INVALID_PARAMETER;
}
- //
- // SMM Communication Buffer max payload size
- //
- SmmCommBufPayloadSize = mVariableBufferSize - (SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE);
OutVariableNameSize = *VariableNameSize;
InVariableNameSize = StrSize (VariableName);
//
// If input string exceeds SMM payload limit. Return failure
//
- if (InVariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
+ if (InVariableNameSize > mVariableBufferPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
return EFI_INVALID_PARAMETER;
}
@@ -344,11 +335,11 @@ RuntimeServiceGetNextVariableName ( // Init the communicate buffer. The buffer data size is:
// SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.
//
- if (OutVariableNameSize > SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
+ if (OutVariableNameSize > mVariableBufferPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name)) {
//
// If output buffer exceed SMM payload limit. Trim output buffer to SMM payload size
//
- OutVariableNameSize = SmmCommBufPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);
+ OutVariableNameSize = mVariableBufferPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME, Name);
}
//
// Payload should be Guid + NameSize + MAX of Input & Output buffer
@@ -448,21 +439,13 @@ 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;
- }
VariableNameSize = StrSize (VariableName);
- if ((UINTN)(~0) - VariableNameSize < OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + DataSize) {
- //
- // Prevent PayloadSize overflow
- //
+ //
+ // If VariableName or DataSize exceeds SMM payload limit. Return failure
+ //
+ if ((VariableNameSize > mVariableBufferPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) ||
+ (DataSize > mVariableBufferPayloadSize - OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - VariableNameSize)){
return EFI_INVALID_PARAMETER;
}
@@ -672,10 +655,11 @@ SmmVariableReady ( ASSERT_EFI_ERROR (Status);
//
- // Allocate memory for variable store.
+ // Allocate memory for variable communicate buffer.
//
- mVariableBufferSize = SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE;
- mVariableBufferSize += MAX (PcdGet32 (PcdMaxVariableSize), PcdGet32 (PcdMaxHardwareErrorVariableSize));
+ mVariableBufferPayloadSize = MAX (PcdGet32 (PcdMaxVariableSize), PcdGet32 (PcdMaxHardwareErrorVariableSize)) +
+ OFFSET_OF (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - sizeof (VARIABLE_HEADER);
+ mVariableBufferSize = SMM_COMMUNICATE_HEADER_SIZE + SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + mVariableBufferPayloadSize;
mVariableBuffer = AllocateRuntimePool (mVariableBufferSize);
ASSERT (mVariableBuffer != NULL);
|