Revision: 14323
http://edk2.svn.sourceforge.net/edk2/?rev=14323&view=rev
Author: czhang46
Date: 2013-05-02 01:42:39 +0000 (Thu, 02 May 2013)
Log Message:
-----------
Fix memory overflow & VariableSize check issue for SetVariable append write.
Signed-off-by: Chao Zhang <[email protected]>
Reviewed-by : Fu Siyuan <[email protected]>
Reviewed-by : Dong Guo <[email protected]>
Modified Paths:
--------------
trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
Modified: trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
===================================================================
--- trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
2013-04-28 02:31:36 UTC (rev 14322)
+++ trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
2013-05-02 01:42:39 UTC (rev 14323)
@@ -192,7 +192,7 @@
//
// Reserved runtime buffer for "Append" operation in virtual mode.
//
- mStorageArea = AllocateRuntimePool (PcdGet32 (PcdMaxVariableSize));
+ mStorageArea = AllocateRuntimePool (MAX (PcdGet32 (PcdMaxVariableSize),
PcdGet32 (PcdMaxHardwareErrorVariableSize)));
if (mStorageArea == NULL) {
return EFI_OUT_OF_RESOURCES;
}
@@ -1316,20 +1316,24 @@
will be appended to the original EFI_SIGNATURE_LIST, duplicate
EFI_SIGNATURE_DATA
will be ignored.
- @param[in, out] Data Pointer to original EFI_SIGNATURE_LIST.
- @param[in] DataSize Size of Data buffer.
- @param[in] NewData Pointer to new EFI_SIGNATURE_LIST to be
appended.
- @param[in] NewDataSize Size of NewData buffer.
+ @param[in, out] Data Pointer to original EFI_SIGNATURE_LIST.
+ @param[in] DataSize Size of Data buffer.
+ @param[in] FreeBufSize Size of free data buffer
+ @param[in] NewData Pointer to new EFI_SIGNATURE_LIST to be
appended.
+ @param[in] NewDataSize Size of NewData buffer.
+ @param[out] MergedBufSize Size of the merged buffer
- @return Size of the merged buffer.
+ @return EFI_BUFFER_TOO_SMALL if input Data buffer overflowed
**/
-UINTN
+EFI_STATUS
AppendSignatureList (
IN OUT VOID *Data,
IN UINTN DataSize,
+ IN UINTN FreeBufSize,
IN VOID *NewData,
- IN UINTN NewDataSize
+ IN UINTN NewDataSize,
+ OUT UINTN *MergedBufSize
)
{
EFI_SIGNATURE_LIST *CertList;
@@ -1388,15 +1392,25 @@
// New EFI_SIGNATURE_DATA, append it.
//
if (CopiedCount == 0) {
+ if (FreeBufSize < sizeof (EFI_SIGNATURE_LIST) +
NewCertList->SignatureHeaderSize) {
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
//
// Copy EFI_SIGNATURE_LIST header for only once.
//
+
CopyMem (Tail, NewCertList, sizeof (EFI_SIGNATURE_LIST) +
NewCertList->SignatureHeaderSize);
Tail = Tail + sizeof (EFI_SIGNATURE_LIST) +
NewCertList->SignatureHeaderSize;
+ FreeBufSize -= sizeof (EFI_SIGNATURE_LIST) +
NewCertList->SignatureHeaderSize;
}
+ if (FreeBufSize < NewCertList->SignatureSize) {
+ return EFI_BUFFER_TOO_SMALL;
+ }
CopyMem (Tail, NewCert, NewCertList->SignatureSize);
Tail += NewCertList->SignatureSize;
+ FreeBufSize -= NewCertList->SignatureSize;
CopiedCount++;
}
@@ -1416,7 +1430,8 @@
NewCertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) NewCertList +
NewCertList->SignatureListSize);
}
- return (Tail - (UINT8 *) Data);
+ *MergedBufSize = (Tail - (UINT8 *) Data);
+ return EFI_SUCCESS;
}
/**
Modified: trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
===================================================================
--- trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
2013-04-28 02:31:36 UTC (rev 14322)
+++ trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.h
2013-05-02 01:42:39 UTC (rev 14323)
@@ -2,7 +2,7 @@
The internal header file includes the common header files, defines
internal structure and functions used by AuthService module.
-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
@@ -249,20 +249,24 @@
will be appended to the original EFI_SIGNATURE_LIST, duplicate
EFI_SIGNATURE_DATA
will be ignored.
- @param[in, out] Data Pointer to original EFI_SIGNATURE_LIST.
- @param[in] DataSize Size of Data buffer.
- @param[in] NewData Pointer to new EFI_SIGNATURE_LIST to be
appended.
- @param[in] NewDataSize Size of NewData buffer.
+ @param[in, out] Data Pointer to original EFI_SIGNATURE_LIST.
+ @param[in] DataSize Size of Data buffer.
+ @param[in] FreeBufSize Size of free data buffer
+ @param[in] NewData Pointer to new EFI_SIGNATURE_LIST to be
appended.
+ @param[in] NewDataSize Size of NewData buffer.
+ @param[out] MergedBufSize Size of the merged buffer
- @return Size of the merged buffer.
+ @return EFI_BUFFER_TOO_SMALL if input Data buffer overflowed
**/
-UINTN
+EFI_STATUS
AppendSignatureList (
IN OUT VOID *Data,
IN UINTN DataSize,
+ IN UINTN FreeBufSize,
IN VOID *NewData,
- IN UINTN NewDataSize
+ IN UINTN NewDataSize,
+ OUT UINTN *MergedBufSize
);
/**
Modified: trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
===================================================================
--- trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
2013-04-28 02:31:36 UTC (rev 14322)
+++ trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
2013-05-02 01:42:39 UTC (rev 14323)
@@ -1649,7 +1649,7 @@
EFI_STATUS Status;
VARIABLE_HEADER *NextVariable;
UINTN ScratchSize;
- UINTN ScratchDataSize;
+ UINTN MaxDataSize;
UINTN NonVolatileVarableStoreSize;
UINTN VarNameOffset;
UINTN VarDataOffset;
@@ -1664,7 +1664,6 @@
UINTN CacheOffset;
UINTN BufSize;
UINTN DataOffset;
- UINTN RevBufSize;
if (mVariableModuleGlobal->FvbInstance == NULL) {
//
@@ -1713,8 +1712,8 @@
//
NextVariable = GetEndPointer ((VARIABLE_STORE_HEADER *) ((UINTN)
mVariableModuleGlobal->VariableGlobal.VolatileVariableBase));
ScratchSize = MAX (PcdGet32 (PcdMaxVariableSize), PcdGet32
(PcdMaxHardwareErrorVariableSize));
- ScratchDataSize = ScratchSize - sizeof (VARIABLE_HEADER) - StrSize
(VariableName) - GET_PAD_SIZE (StrSize (VariableName));
+
if (Variable->CurrPtr != NULL) {
//
// Update/Delete existing variable.
@@ -1827,14 +1826,36 @@
DataOffset = sizeof (VARIABLE_HEADER) + Variable->CurrPtr->NameSize +
GET_PAD_SIZE (Variable->CurrPtr->NameSize);
CopyMem (mStorageArea, (UINT8*)((UINTN) Variable->CurrPtr +
DataOffset), Variable->CurrPtr->DataSize);
+ //
+ // Set Max Common Variable Data Size as default MaxDataSize
+ //
+ MaxDataSize = PcdGet32 (PcdMaxVariableSize) - sizeof (VARIABLE_HEADER)
- StrSize (VariableName) - GET_PAD_SIZE (StrSize (VariableName));
+
+
if ((CompareGuid (VendorGuid, &gEfiImageSecurityDatabaseGuid) &&
((StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) ||
(StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE1) == 0))) ||
(CompareGuid (VendorGuid, &gEfiGlobalVariableGuid) && (StrCmp
(VariableName, EFI_KEY_EXCHANGE_KEY_NAME) == 0))) {
+
//
// For variables with formatted as EFI_SIGNATURE_LIST, the driver
shall not perform an append of
// EFI_SIGNATURE_DATA values that are already part of the existing
variable value.
//
- BufSize = AppendSignatureList (mStorageArea,
Variable->CurrPtr->DataSize, Data, DataSize);
+ Status = AppendSignatureList (
+ mStorageArea,
+ Variable->CurrPtr->DataSize,
+ MaxDataSize - Variable->CurrPtr->DataSize,
+ Data,
+ DataSize,
+ &BufSize
+ );
+ if (Status == EFI_BUFFER_TOO_SMALL) {
+ //
+ // Signture List is too long, Failed to Append
+ //
+ Status = EFI_INVALID_PARAMETER;
+ goto Done;
+ }
+
if (BufSize == Variable->CurrPtr->DataSize) {
if ((TimeStamp == NULL) || CompareTimeStamp (TimeStamp,
&Variable->CurrPtr->TimeStamp)) {
//
@@ -1849,20 +1870,23 @@
} else {
//
// For other Variables, append the new data to the end of previous
data.
+ // Max Harware error record variable data size is different from
common variable
//
+ if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) ==
EFI_VARIABLE_HARDWARE_ERROR_RECORD) {
+ MaxDataSize = PcdGet32 (PcdMaxHardwareErrorVariableSize) - sizeof
(VARIABLE_HEADER) - StrSize (VariableName) - GET_PAD_SIZE (StrSize
(VariableName));
+ }
+
+ if (Variable->CurrPtr->DataSize + DataSize > MaxDataSize) {
+ //
+ // Exsiting data + Appended data exceed maximum variable size
limitation
+ //
+ Status = EFI_INVALID_PARAMETER;
+ goto Done;
+ }
CopyMem ((UINT8*)((UINTN) mStorageArea +
Variable->CurrPtr->DataSize), Data, DataSize);
BufSize = Variable->CurrPtr->DataSize + DataSize;
}
- RevBufSize = MIN (PcdGet32 (PcdMaxVariableSize), ScratchDataSize);
- if (BufSize > RevBufSize) {
- //
- // If variable size (previous + current) is bigger than reserved
buffer in runtime,
- // return EFI_OUT_OF_RESOURCES.
- //
- return EFI_OUT_OF_RESOURCES;
- }
-
//
// Override Data and DataSize which are used for combined data area
including previous and new data.
//
This was sent by the SourceForge.net collaborative development platform, the
world's largest Open Source development site.
------------------------------------------------------------------------------
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
Get 100% visibility into your production application - at no cost.
Code-level diagnostics for performance bottlenecks with <2% overhead
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
_______________________________________________
edk2-commits mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-commits
