The variable driver doesn't distinguish "non-volatile non-authenticated"
variables from "volatile non-authenticated" variables, when checking
individual variable sizes against the permitted maximum.
PcdMaxVariableSize covers both kinds.
This prevents volatile non-authenticated variables from carrying large
data between UEFI drivers, despite having no flash impact. One example is
EFI_TLS_CA_CERTIFICATE_VARIABLE, which platforms might want to create as
volatile on every boot: the certificate list can be several hundred KB in
size.
Introduce PcdMaxVolatileVariableSize to represent the limit on individual
volatile non-authenticated variables. The default value is zero, which
makes Variable/RuntimeDxe fall back to PcdMaxVariableSize (i.e. the
current behavior). This is similar to the PcdMaxAuthVariableSize fallback.
Whenever the size limit is enforced, consult MaxVolatileVariableSize as
the last option, after checking
- MaxAuthVariableSize for VARIABLE_ATTRIBUTE_AT_AW,
- and MaxVariableSize for EFI_VARIABLE_NON_VOLATILE.
EFI_VARIABLE_HARDWARE_ERROR_RECORD is always handled separately; it always
takes priority over the three cases listed above.
Introduce the GetMaxVariableSize() helper to consider
PcdMaxVolatileVariableSize, in addition to
GetNonVolatileMaxVariableSize(). GetNonVolatileMaxVariableSize() is
currently called at three sites, and two of those need to start using
GetMaxVariableSize() instead:
- VariableServiceInitialize() [VariableSmm.c]: the SMM comms buffer must
accommodate all kinds of variables,
- VariableCommonInitialize() [Variable.c]: the preallocated scratch space
must also accommodate all kinds of variables,
- InitNonVolatileVariableStore() [Variable.c] can continue using
GetNonVolatileMaxVariableSize().
Don't modify the ReclaimForOS() function as it is specific to non-volatile
variables and should ignore PcdMaxVolatileVariableSize.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdeModulePkg/MdeModulePkg.dec | 8 ++++
MdeModulePkg/MdeModulePkg.uni | 8 ++++
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 1 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 1 +
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h | 12 +++++
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 50 +++++++++++++++++---
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 2 +-
7 files changed, 74 insertions(+), 8 deletions(-)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 5d561ff48495..cc397185f7b9 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1043,6 +1043,14 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
# @Prompt Maximum authenticated variable size.
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x00|UINT32|0x30000009
+ ## The maximum size of a single non-authenticated volatile variable.
+ # The default value is 0 for compatibility: in that case, the maximum
+ # non-authenticated volatile variable size remains specified by
+ # PcdMaxVariableSize. Only the MdeModulePkg/Universal/Variable/RuntimeDxe
+ # driver supports this PCD.
+ # @Prompt Maximum non-authenticated volatile variable size.
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x00|UINT32|0x3000000a
+
## The maximum size of single hardware error record variable.<BR><BR>
# In IA32/X64 platforms, this value should be larger than 1KB.<BR>
# In IA64 platforms, this value should be larger than 128KB.<BR>
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index f3fa616438b0..080b8a62c045 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -94,6 +94,14 @@
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxAuthVariableSize_HELP #language en-US "The maximum size of a single authenticated variable."
"The value is 0 as default for compatibility that maximum authenticated variable size is specified by PcdMaxVariableSize."
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxVolatileVariableSize_PROMPT #language en-US "The maximum size of a single non-authenticated volatile variable."
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxVolatileVariableSize_HELP #language en-US "The maximum size of a single non-authenticated volatile variable.<BR><BR>\n"
+ "The default value is 0 for compatibility: in that case, the maximum "
+ "non-authenticated volatile variable size remains specified by "
+ "PcdMaxVariableSize.<BR>\n"
+ "Only the MdeModulePkg/Universal/Variable/RuntimeDxe driver supports this PCD.<BR>"
+
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxHardwareErrorVariableSize_PROMPT #language en-US "Maximum HwErr variable size"
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxHardwareErrorVariableSize_HELP #language en-US "The maximum size of single hardware error record variable.<BR><BR>\n"
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
index e840fc9bff40..2d0a172ece35 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
@@ -123,6 +123,7 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize ## CONSUMES
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
index 69966f0d37ee..dbb0674a46ad 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
@@ -125,6 +125,7 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize ## CONSUMES
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
index b35e8ab91273..938eb5de61fa 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
@@ -101,6 +101,7 @@ typedef struct {
UINTN HwErrVariableTotalSize;
UINTN MaxVariableSize;
UINTN MaxAuthVariableSize;
+ UINTN MaxVolatileVariableSize;
UINTN ScratchBufferSize;
CHAR8 *PlatformLangCodes;
CHAR8 *LangCodes;
@@ -460,6 +461,17 @@ GetNonVolatileMaxVariableSize (
VOID
);
+/**
+ Get maximum variable size, covering both non-volatile and volatile variables.
+
+ @return Maximum variable size.
+
+**/
+UINTN
+GetMaxVariableSize (
+ VOID
+ );
+
/**
Initializes variable write service after FVB was ready.
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index c11842b5c3ba..5a9051648004 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -2345,12 +2345,14 @@ UpdateVariable (
CopyMem (BufferForMerge, (UINT8 *) ((UINTN) CacheVariable->CurrPtr + DataOffset), DataSizeOfVariable (CacheVariable->CurrPtr));
//
- // Set Max Common/Auth Variable Data Size as default MaxDataSize.
+ // Set Max Auth/Non-Volatile/Volatile Variable Data Size as default MaxDataSize.
//
if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
MaxDataSize = mVariableModuleGlobal->MaxAuthVariableSize - DataOffset;
- } else {
+ } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
MaxDataSize = mVariableModuleGlobal->MaxVariableSize - DataOffset;
+ } else {
+ MaxDataSize = mVariableModuleGlobal->MaxVolatileVariableSize - DataOffset;
}
//
@@ -3218,16 +3220,20 @@ VariableServiceSetVariable (
} else {
//
// The size of the VariableName, including the Unicode Null in bytes plus
- // the DataSize is limited to maximum size of Max(Auth)VariableSize bytes.
+ // the DataSize is limited to maximum size of Max(Auth|Volatile)VariableSize bytes.
//
if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ()) {
return EFI_INVALID_PARAMETER;
}
- } else {
+ } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ()) {
return EFI_INVALID_PARAMETER;
}
+ } else {
+ if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVolatileVariableSize - GetVariableHeaderSize ()) {
+ return EFI_INVALID_PARAMETER;
+ }
}
}
@@ -3399,12 +3405,14 @@ VariableServiceQueryVariableInfoInternal (
}
//
- // Let *MaximumVariableSize be Max(Auth)VariableSize with the exception of the variable header size.
+ // Let *MaximumVariableSize be Max(Auth|Volatile)VariableSize with the exception of the variable header size.
//
if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
*MaximumVariableSize = mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ();
- } else {
+ } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
*MaximumVariableSize = mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ();
+ } else {
+ *MaximumVariableSize = mVariableModuleGlobal->MaxVolatileVariableSize - GetVariableHeaderSize ();
}
}
@@ -3657,6 +3665,30 @@ GetNonVolatileMaxVariableSize (
}
}
+/**
+ Get maximum variable size, covering both non-volatile and volatile variables.
+
+ @return Maximum variable size.
+
+**/
+UINTN
+GetMaxVariableSize (
+ VOID
+ )
+{
+ UINTN MaxVariableSize;
+
+ MaxVariableSize = GetNonVolatileMaxVariableSize();
+ //
+ // The condition below fails implicitly if PcdMaxVolatileVariableSize equals
+ // the default zero value.
+ //
+ if (MaxVariableSize < PcdGet32 (PcdMaxVolatileVariableSize)) {
+ MaxVariableSize = PcdGet32 (PcdMaxVolatileVariableSize);
+ }
+ return MaxVariableSize;
+}
+
/**
Init non-volatile variable store.
@@ -3810,6 +3842,10 @@ InitNonVolatileVariableStore (
mVariableModuleGlobal->MaxVariableSize = PcdGet32 (PcdMaxVariableSize);
mVariableModuleGlobal->MaxAuthVariableSize = ((PcdGet32 (PcdMaxAuthVariableSize) != 0) ? PcdGet32 (PcdMaxAuthVariableSize) : mVariableModuleGlobal->MaxVariableSize);
+ mVariableModuleGlobal->MaxVolatileVariableSize = ((PcdGet32 (PcdMaxVolatileVariableSize) != 0) ?
+ PcdGet32 (PcdMaxVolatileVariableSize) :
+ mVariableModuleGlobal->MaxVariableSize
+ );
//
// Parse non-volatile variable data and get last variable offset.
@@ -4228,7 +4264,7 @@ VariableCommonInitialize (
//
// Allocate memory for volatile variable store, note that there is a scratch space to store scratch data.
//
- ScratchSize = GetNonVolatileMaxVariableSize ();
+ ScratchSize = GetMaxVariableSize ();
mVariableModuleGlobal->ScratchBufferSize = ScratchSize;
VolatileVariableStore = AllocateRuntimePool (PcdGet32 (PcdVariableStoreSize) + ScratchSize);
if (VolatileVariableStore == NULL) {
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index 8d73b6edee51..e495d971a08b 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -955,7 +955,7 @@ VariableServiceInitialize (
);
ASSERT_EFI_ERROR (Status);
- mVariableBufferPayloadSize = GetNonVolatileMaxVariableSize () +
+ mVariableBufferPayloadSize = GetMaxVariableSize () +
OFFSET_OF (SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY, Name) - GetVariableHeaderSize ();
Status = gSmst->SmmAllocatePool (
--
2.14.1.3.gb7cf6e02401b
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo,
Good patch and I assume you have done some tests with the patch. :)
One very minor comment.
Do you think the MaxVolatileVariableSize field assignment is better to be in VariableCommonInitialize(), but not InitNonVolatileVariableStore()?
mVariableModuleGlobal->MaxVolatileVariableSize = ((PcdGet32 (PcdMaxVolatileVariableSize) != 0) ?
PcdGet32 (PcdMaxVolatileVariableSize) :
mVariableModuleGlobal->MaxVariableSize
);
For example, have it right before the comments in VariableCommonInitialize().
//
// Allocate memory for volatile variable store, note that there is a scratch space to store scratch data.
//
Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Thursday, March 29, 2018 4:27 AM
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [PATCH 1/4] MdeModulePkg/Variable/RuntimeDxe: introduce PcdMaxVolatileVariableSize
The variable driver doesn't distinguish "non-volatile non-authenticated"
variables from "volatile non-authenticated" variables, when checking individual variable sizes against the permitted maximum.
PcdMaxVariableSize covers both kinds.
This prevents volatile non-authenticated variables from carrying large data between UEFI drivers, despite having no flash impact. One example is EFI_TLS_CA_CERTIFICATE_VARIABLE, which platforms might want to create as volatile on every boot: the certificate list can be several hundred KB in size.
Introduce PcdMaxVolatileVariableSize to represent the limit on individual volatile non-authenticated variables. The default value is zero, which makes Variable/RuntimeDxe fall back to PcdMaxVariableSize (i.e. the current behavior). This is similar to the PcdMaxAuthVariableSize fallback.
Whenever the size limit is enforced, consult MaxVolatileVariableSize as the last option, after checking
- MaxAuthVariableSize for VARIABLE_ATTRIBUTE_AT_AW,
- and MaxVariableSize for EFI_VARIABLE_NON_VOLATILE.
EFI_VARIABLE_HARDWARE_ERROR_RECORD is always handled separately; it always takes priority over the three cases listed above.
Introduce the GetMaxVariableSize() helper to consider PcdMaxVolatileVariableSize, in addition to GetNonVolatileMaxVariableSize(). GetNonVolatileMaxVariableSize() is currently called at three sites, and two of those need to start using
GetMaxVariableSize() instead:
- VariableServiceInitialize() [VariableSmm.c]: the SMM comms buffer must
accommodate all kinds of variables,
- VariableCommonInitialize() [Variable.c]: the preallocated scratch space
must also accommodate all kinds of variables,
- InitNonVolatileVariableStore() [Variable.c] can continue using
GetNonVolatileMaxVariableSize().
Don't modify the ReclaimForOS() function as it is specific to non-volatile variables and should ignore PcdMaxVolatileVariableSize.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdeModulePkg/MdeModulePkg.dec | 8 ++++
MdeModulePkg/MdeModulePkg.uni | 8 ++++
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 1 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 1 +
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h | 12 +++++
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 50 +++++++++++++++++---
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 2 +-
7 files changed, 74 insertions(+), 8 deletions(-)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 5d561ff48495..cc397185f7b9 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1043,6 +1043,14 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
# @Prompt Maximum authenticated variable size.
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x00|UINT32|0x30000009
+ ## The maximum size of a single non-authenticated volatile variable.
+ # The default value is 0 for compatibility: in that case, the maximum
+ # non-authenticated volatile variable size remains specified by #
+ PcdMaxVariableSize. Only the
+ MdeModulePkg/Universal/Variable/RuntimeDxe
+ # driver supports this PCD.
+ # @Prompt Maximum non-authenticated volatile variable size.
+
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x00|UINT32|
+ 0x3000000a
+
## The maximum size of single hardware error record variable.<BR><BR>
# In IA32/X64 platforms, this value should be larger than 1KB.<BR>
# In IA64 platforms, this value should be larger than 128KB.<BR> diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index f3fa616438b0..080b8a62c045 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -94,6 +94,14 @@
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxAuthVariableSize_HELP #language en-US "The maximum size of a single authenticated variable."
"The value is 0 as default for compatibility that maximum authenticated variable size is specified by PcdMaxVariableSize."
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxVolatileVariableSize_PROMPT #language en-US "The maximum size of a single non-authenticated volatile variable."
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxVolatileVariableSize_HELP #language en-US "The maximum size of a single non-authenticated volatile variable.<BR><BR>\n"
+ "The default value is 0 for compatibility: in that case, the maximum "
+ "non-authenticated volatile variable size remains specified by "
+ "PcdMaxVariableSize.<BR>\n"
+ "Only the MdeModulePkg/Universal/Variable/RuntimeDxe driver supports this PCD.<BR>"
+
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxHardwareErrorVariableSize_PROMPT #language en-US "Maximum HwErr variable size"
#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxHardwareErrorVariableSize_HELP #language en-US "The maximum size of single hardware error record variable.<BR><BR>\n"
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
index e840fc9bff40..2d0a172ece35 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
@@ -123,6 +123,7 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize ## CONSUMES
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
index 69966f0d37ee..dbb0674a46ad 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
@@ -125,6 +125,7 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize ## CONSUMES
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
index b35e8ab91273..938eb5de61fa 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
@@ -101,6 +101,7 @@ typedef struct {
UINTN HwErrVariableTotalSize;
UINTN MaxVariableSize;
UINTN MaxAuthVariableSize;
+ UINTN MaxVolatileVariableSize;
UINTN ScratchBufferSize;
CHAR8 *PlatformLangCodes;
CHAR8 *LangCodes;
@@ -460,6 +461,17 @@ GetNonVolatileMaxVariableSize (
VOID
);
+/**
+ Get maximum variable size, covering both non-volatile and volatile variables.
+
+ @return Maximum variable size.
+
+**/
+UINTN
+GetMaxVariableSize (
+ VOID
+ );
+
/**
Initializes variable write service after FVB was ready.
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index c11842b5c3ba..5a9051648004 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -2345,12 +2345,14 @@ UpdateVariable (
CopyMem (BufferForMerge, (UINT8 *) ((UINTN) CacheVariable->CurrPtr + DataOffset), DataSizeOfVariable (CacheVariable->CurrPtr));
//
- // Set Max Common/Auth Variable Data Size as default MaxDataSize.
+ // Set Max Auth/Non-Volatile/Volatile Variable Data Size as default MaxDataSize.
//
if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
MaxDataSize = mVariableModuleGlobal->MaxAuthVariableSize - DataOffset;
- } else {
+ } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
MaxDataSize = mVariableModuleGlobal->MaxVariableSize - DataOffset;
+ } else {
+ MaxDataSize = mVariableModuleGlobal->MaxVolatileVariableSize
+ - DataOffset;
}
//
@@ -3218,16 +3220,20 @@ VariableServiceSetVariable (
} else {
//
// The size of the VariableName, including the Unicode Null in bytes plus
- // the DataSize is limited to maximum size of Max(Auth)VariableSize bytes.
+ // the DataSize is limited to maximum size of Max(Auth|Volatile)VariableSize bytes.
//
if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ()) {
return EFI_INVALID_PARAMETER;
}
- } else {
+ } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ()) {
return EFI_INVALID_PARAMETER;
}
+ } else {
+ if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVolatileVariableSize - GetVariableHeaderSize ()) {
+ return EFI_INVALID_PARAMETER;
+ }
}
}
@@ -3399,12 +3405,14 @@ VariableServiceQueryVariableInfoInternal (
}
//
- // Let *MaximumVariableSize be Max(Auth)VariableSize with the exception of the variable header size.
+ // Let *MaximumVariableSize be Max(Auth|Volatile)VariableSize with the exception of the variable header size.
//
if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
*MaximumVariableSize = mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ();
- } else {
+ } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
*MaximumVariableSize = mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ();
+ } else {
+ *MaximumVariableSize =
+ mVariableModuleGlobal->MaxVolatileVariableSize - GetVariableHeaderSize
+ ();
}
}
@@ -3657,6 +3665,30 @@ GetNonVolatileMaxVariableSize (
}
}
+/**
+ Get maximum variable size, covering both non-volatile and volatile variables.
+
+ @return Maximum variable size.
+
+**/
+UINTN
+GetMaxVariableSize (
+ VOID
+ )
+{
+ UINTN MaxVariableSize;
+
+ MaxVariableSize = GetNonVolatileMaxVariableSize();
+ //
+ // The condition below fails implicitly if PcdMaxVolatileVariableSize
+equals
+ // the default zero value.
+ //
+ if (MaxVariableSize < PcdGet32 (PcdMaxVolatileVariableSize)) {
+ MaxVariableSize = PcdGet32 (PcdMaxVolatileVariableSize);
+ }
+ return MaxVariableSize;
+}
+
/**
Init non-volatile variable store.
@@ -3810,6 +3842,10 @@ InitNonVolatileVariableStore (
mVariableModuleGlobal->MaxVariableSize = PcdGet32 (PcdMaxVariableSize);
mVariableModuleGlobal->MaxAuthVariableSize = ((PcdGet32 (PcdMaxAuthVariableSize) != 0) ? PcdGet32 (PcdMaxAuthVariableSize) : mVariableModuleGlobal->MaxVariableSize);
+ mVariableModuleGlobal->MaxVolatileVariableSize = ((PcdGet32 (PcdMaxVolatileVariableSize) != 0) ?
+ PcdGet32 (PcdMaxVolatileVariableSize) :
+ mVariableModuleGlobal->MaxVariableSize
+ );
//
// Parse non-volatile variable data and get last variable offset.
@@ -4228,7 +4264,7 @@ VariableCommonInitialize (
//
// Allocate memory for volatile variable store, note that there is a scratch space to store scratch data.
//
- ScratchSize = GetNonVolatileMaxVariableSize ();
+ ScratchSize = GetMaxVariableSize ();
mVariableModuleGlobal->ScratchBufferSize = ScratchSize;
VolatileVariableStore = AllocateRuntimePool (PcdGet32 (PcdVariableStoreSize) + ScratchSize);
if (VolatileVariableStore == NULL) {
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index 8d73b6edee51..e495d971a08b 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -955,7 +955,7 @@ VariableServiceInitialize (
);
ASSERT_EFI_ERROR (Status);
- mVariableBufferPayloadSize = GetNonVolatileMaxVariableSize () +
+ mVariableBufferPayloadSize = GetMaxVariableSize () +
OFFSET_OF (SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY, Name) - GetVariableHeaderSize ();
Status = gSmst->SmmAllocatePool (
--
2.14.1.3.gb7cf6e02401b
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 03/29/18 03:34, Zeng, Star wrote:
> Laszlo,
>
> Good patch and I assume you have done some tests with the patch. :)
Yes, I have. For example I re-ran the Secure Boot Logo Test (the
automated one) from Microsoft HCK, using a "controller" VM (Windows
Server 2008R2) and a "client" VM (Windows Server 2016). This test
massages variables quite heavily.
I've never used the UEFI SCT; if someone could help with that, that
would be great.
> One very minor comment.
>
> Do you think the MaxVolatileVariableSize field assignment is better to be in VariableCommonInitialize(), but not InitNonVolatileVariableStore()?
>
> mVariableModuleGlobal->MaxVolatileVariableSize = ((PcdGet32 (PcdMaxVolatileVariableSize) != 0) ?
> PcdGet32 (PcdMaxVolatileVariableSize) :
> mVariableModuleGlobal->MaxVariableSize
> );
That's a good point! InitNonVolatileVariableStore() is not the right
place, judged simply by the function name. I originally put it there for
keeping the "size" PCDs close together, but there's likely a better spot.
> For example, have it right before the comments in VariableCommonInitialize().
>
> //
> // Allocate memory for volatile variable store, note that there is a scratch space to store scratch data.
> //
OK, I'll do that in v2.
Thanks!
Laszlo
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, March 29, 2018 4:27 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH 1/4] MdeModulePkg/Variable/RuntimeDxe: introduce PcdMaxVolatileVariableSize
>
> The variable driver doesn't distinguish "non-volatile non-authenticated"
> variables from "volatile non-authenticated" variables, when checking individual variable sizes against the permitted maximum.
> PcdMaxVariableSize covers both kinds.
>
> This prevents volatile non-authenticated variables from carrying large data between UEFI drivers, despite having no flash impact. One example is EFI_TLS_CA_CERTIFICATE_VARIABLE, which platforms might want to create as volatile on every boot: the certificate list can be several hundred KB in size.
>
> Introduce PcdMaxVolatileVariableSize to represent the limit on individual volatile non-authenticated variables. The default value is zero, which makes Variable/RuntimeDxe fall back to PcdMaxVariableSize (i.e. the current behavior). This is similar to the PcdMaxAuthVariableSize fallback.
>
> Whenever the size limit is enforced, consult MaxVolatileVariableSize as the last option, after checking
> - MaxAuthVariableSize for VARIABLE_ATTRIBUTE_AT_AW,
> - and MaxVariableSize for EFI_VARIABLE_NON_VOLATILE.
>
> EFI_VARIABLE_HARDWARE_ERROR_RECORD is always handled separately; it always takes priority over the three cases listed above.
>
> Introduce the GetMaxVariableSize() helper to consider PcdMaxVolatileVariableSize, in addition to GetNonVolatileMaxVariableSize(). GetNonVolatileMaxVariableSize() is currently called at three sites, and two of those need to start using
> GetMaxVariableSize() instead:
> - VariableServiceInitialize() [VariableSmm.c]: the SMM comms buffer must
> accommodate all kinds of variables,
> - VariableCommonInitialize() [Variable.c]: the preallocated scratch space
> must also accommodate all kinds of variables,
> - InitNonVolatileVariableStore() [Variable.c] can continue using
> GetNonVolatileMaxVariableSize().
>
> Don't modify the ReclaimForOS() function as it is specific to non-volatile variables and should ignore PcdMaxVolatileVariableSize.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> MdeModulePkg/MdeModulePkg.dec | 8 ++++
> MdeModulePkg/MdeModulePkg.uni | 8 ++++
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 1 +
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 1 +
> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h | 12 +++++
> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 50 +++++++++++++++++---
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 2 +-
> 7 files changed, 74 insertions(+), 8 deletions(-)
>
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 5d561ff48495..cc397185f7b9 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1043,6 +1043,14 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
> # @Prompt Maximum authenticated variable size.
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x00|UINT32|0x30000009
>
> + ## The maximum size of a single non-authenticated volatile variable.
> + # The default value is 0 for compatibility: in that case, the maximum
> + # non-authenticated volatile variable size remains specified by #
> + PcdMaxVariableSize. Only the
> + MdeModulePkg/Universal/Variable/RuntimeDxe
> + # driver supports this PCD.
> + # @Prompt Maximum non-authenticated volatile variable size.
> +
> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x00|UINT32|
> + 0x3000000a
> +
> ## The maximum size of single hardware error record variable.<BR><BR>
> # In IA32/X64 platforms, this value should be larger than 1KB.<BR>
> # In IA64 platforms, this value should be larger than 128KB.<BR> diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index f3fa616438b0..080b8a62c045 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -94,6 +94,14 @@
> #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxAuthVariableSize_HELP #language en-US "The maximum size of a single authenticated variable."
> "The value is 0 as default for compatibility that maximum authenticated variable size is specified by PcdMaxVariableSize."
>
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxVolatileVariableSize_PROMPT #language en-US "The maximum size of a single non-authenticated volatile variable."
> +
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxVolatileVariableSize_HELP #language en-US "The maximum size of a single non-authenticated volatile variable.<BR><BR>\n"
> + "The default value is 0 for compatibility: in that case, the maximum "
> + "non-authenticated volatile variable size remains specified by "
> + "PcdMaxVariableSize.<BR>\n"
> + "Only the MdeModulePkg/Universal/Variable/RuntimeDxe driver supports this PCD.<BR>"
> +
> #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxHardwareErrorVariableSize_PROMPT #language en-US "Maximum HwErr variable size"
>
> #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxHardwareErrorVariableSize_HELP #language en-US "The maximum size of single hardware error record variable.<BR><BR>\n"
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> index e840fc9bff40..2d0a172ece35 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> @@ -123,6 +123,7 @@ [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize ## CONSUMES
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> index 69966f0d37ee..dbb0674a46ad 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> @@ -125,6 +125,7 @@ [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize ## CONSUMES
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> index b35e8ab91273..938eb5de61fa 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> @@ -101,6 +101,7 @@ typedef struct {
> UINTN HwErrVariableTotalSize;
> UINTN MaxVariableSize;
> UINTN MaxAuthVariableSize;
> + UINTN MaxVolatileVariableSize;
> UINTN ScratchBufferSize;
> CHAR8 *PlatformLangCodes;
> CHAR8 *LangCodes;
> @@ -460,6 +461,17 @@ GetNonVolatileMaxVariableSize (
> VOID
> );
>
> +/**
> + Get maximum variable size, covering both non-volatile and volatile variables.
> +
> + @return Maximum variable size.
> +
> +**/
> +UINTN
> +GetMaxVariableSize (
> + VOID
> + );
> +
> /**
> Initializes variable write service after FVB was ready.
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index c11842b5c3ba..5a9051648004 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -2345,12 +2345,14 @@ UpdateVariable (
> CopyMem (BufferForMerge, (UINT8 *) ((UINTN) CacheVariable->CurrPtr + DataOffset), DataSizeOfVariable (CacheVariable->CurrPtr));
>
> //
> - // Set Max Common/Auth Variable Data Size as default MaxDataSize.
> + // Set Max Auth/Non-Volatile/Volatile Variable Data Size as default MaxDataSize.
> //
> if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
> MaxDataSize = mVariableModuleGlobal->MaxAuthVariableSize - DataOffset;
> - } else {
> + } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
> MaxDataSize = mVariableModuleGlobal->MaxVariableSize - DataOffset;
> + } else {
> + MaxDataSize = mVariableModuleGlobal->MaxVolatileVariableSize
> + - DataOffset;
> }
>
> //
> @@ -3218,16 +3220,20 @@ VariableServiceSetVariable (
> } else {
> //
> // The size of the VariableName, including the Unicode Null in bytes plus
> - // the DataSize is limited to maximum size of Max(Auth)VariableSize bytes.
> + // the DataSize is limited to maximum size of Max(Auth|Volatile)VariableSize bytes.
> //
> if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
> if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ()) {
> return EFI_INVALID_PARAMETER;
> }
> - } else {
> + } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
> if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ()) {
> return EFI_INVALID_PARAMETER;
> }
> + } else {
> + if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVolatileVariableSize - GetVariableHeaderSize ()) {
> + return EFI_INVALID_PARAMETER;
> + }
> }
> }
>
> @@ -3399,12 +3405,14 @@ VariableServiceQueryVariableInfoInternal (
> }
>
> //
> - // Let *MaximumVariableSize be Max(Auth)VariableSize with the exception of the variable header size.
> + // Let *MaximumVariableSize be Max(Auth|Volatile)VariableSize with the exception of the variable header size.
> //
> if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
> *MaximumVariableSize = mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ();
> - } else {
> + } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
> *MaximumVariableSize = mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ();
> + } else {
> + *MaximumVariableSize =
> + mVariableModuleGlobal->MaxVolatileVariableSize - GetVariableHeaderSize
> + ();
> }
> }
>
> @@ -3657,6 +3665,30 @@ GetNonVolatileMaxVariableSize (
> }
> }
>
> +/**
> + Get maximum variable size, covering both non-volatile and volatile variables.
> +
> + @return Maximum variable size.
> +
> +**/
> +UINTN
> +GetMaxVariableSize (
> + VOID
> + )
> +{
> + UINTN MaxVariableSize;
> +
> + MaxVariableSize = GetNonVolatileMaxVariableSize();
> + //
> + // The condition below fails implicitly if PcdMaxVolatileVariableSize
> +equals
> + // the default zero value.
> + //
> + if (MaxVariableSize < PcdGet32 (PcdMaxVolatileVariableSize)) {
> + MaxVariableSize = PcdGet32 (PcdMaxVolatileVariableSize);
> + }
> + return MaxVariableSize;
> +}
> +
> /**
> Init non-volatile variable store.
>
> @@ -3810,6 +3842,10 @@ InitNonVolatileVariableStore (
>
> mVariableModuleGlobal->MaxVariableSize = PcdGet32 (PcdMaxVariableSize);
> mVariableModuleGlobal->MaxAuthVariableSize = ((PcdGet32 (PcdMaxAuthVariableSize) != 0) ? PcdGet32 (PcdMaxAuthVariableSize) : mVariableModuleGlobal->MaxVariableSize);
> + mVariableModuleGlobal->MaxVolatileVariableSize = ((PcdGet32 (PcdMaxVolatileVariableSize) != 0) ?
> + PcdGet32 (PcdMaxVolatileVariableSize) :
> + mVariableModuleGlobal->MaxVariableSize
> + );
>
> //
> // Parse non-volatile variable data and get last variable offset.
> @@ -4228,7 +4264,7 @@ VariableCommonInitialize (
> //
> // Allocate memory for volatile variable store, note that there is a scratch space to store scratch data.
> //
> - ScratchSize = GetNonVolatileMaxVariableSize ();
> + ScratchSize = GetMaxVariableSize ();
> mVariableModuleGlobal->ScratchBufferSize = ScratchSize;
> VolatileVariableStore = AllocateRuntimePool (PcdGet32 (PcdVariableStoreSize) + ScratchSize);
> if (VolatileVariableStore == NULL) {
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> index 8d73b6edee51..e495d971a08b 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> @@ -955,7 +955,7 @@ VariableServiceInitialize (
> );
> ASSERT_EFI_ERROR (Status);
>
> - mVariableBufferPayloadSize = GetNonVolatileMaxVariableSize () +
> + mVariableBufferPayloadSize = GetMaxVariableSize () +
> OFFSET_OF (SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY, Name) - GetVariableHeaderSize ();
>
> Status = gSmst->SmmAllocatePool (
> --
> 2.14.1.3.gb7cf6e02401b
>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
No V2 needed for this case, Reviewed-by: Star Zeng <star.zeng@intel.com> with my comment handled.
Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Thursday, March 29, 2018 8:19 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
Subject: Re: [PATCH 1/4] MdeModulePkg/Variable/RuntimeDxe: introduce PcdMaxVolatileVariableSize
On 03/29/18 03:34, Zeng, Star wrote:
> Laszlo,
>
> Good patch and I assume you have done some tests with the patch. :)
Yes, I have. For example I re-ran the Secure Boot Logo Test (the automated one) from Microsoft HCK, using a "controller" VM (Windows Server 2008R2) and a "client" VM (Windows Server 2016). This test massages variables quite heavily.
I've never used the UEFI SCT; if someone could help with that, that would be great.
> One very minor comment.
>
> Do you think the MaxVolatileVariableSize field assignment is better to be in VariableCommonInitialize(), but not InitNonVolatileVariableStore()?
>
> mVariableModuleGlobal->MaxVolatileVariableSize = ((PcdGet32 (PcdMaxVolatileVariableSize) != 0) ?
> PcdGet32 (PcdMaxVolatileVariableSize) :
> mVariableModuleGlobal->MaxVariableSize
> );
That's a good point! InitNonVolatileVariableStore() is not the right place, judged simply by the function name. I originally put it there for keeping the "size" PCDs close together, but there's likely a better spot.
> For example, have it right before the comments in VariableCommonInitialize().
>
> //
> // Allocate memory for volatile variable store, note that there is a scratch space to store scratch data.
> //
OK, I'll do that in v2.
Thanks!
Laszlo
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, March 29, 2018 4:27 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> Zeng, Star <star.zeng@intel.com>
> Subject: [PATCH 1/4] MdeModulePkg/Variable/RuntimeDxe: introduce
> PcdMaxVolatileVariableSize
>
> The variable driver doesn't distinguish "non-volatile non-authenticated"
> variables from "volatile non-authenticated" variables, when checking individual variable sizes against the permitted maximum.
> PcdMaxVariableSize covers both kinds.
>
> This prevents volatile non-authenticated variables from carrying large data between UEFI drivers, despite having no flash impact. One example is EFI_TLS_CA_CERTIFICATE_VARIABLE, which platforms might want to create as volatile on every boot: the certificate list can be several hundred KB in size.
>
> Introduce PcdMaxVolatileVariableSize to represent the limit on individual volatile non-authenticated variables. The default value is zero, which makes Variable/RuntimeDxe fall back to PcdMaxVariableSize (i.e. the current behavior). This is similar to the PcdMaxAuthVariableSize fallback.
>
> Whenever the size limit is enforced, consult MaxVolatileVariableSize
> as the last option, after checking
> - MaxAuthVariableSize for VARIABLE_ATTRIBUTE_AT_AW,
> - and MaxVariableSize for EFI_VARIABLE_NON_VOLATILE.
>
> EFI_VARIABLE_HARDWARE_ERROR_RECORD is always handled separately; it always takes priority over the three cases listed above.
>
> Introduce the GetMaxVariableSize() helper to consider
> PcdMaxVolatileVariableSize, in addition to
> GetNonVolatileMaxVariableSize(). GetNonVolatileMaxVariableSize() is
> currently called at three sites, and two of those need to start using
> GetMaxVariableSize() instead:
> - VariableServiceInitialize() [VariableSmm.c]: the SMM comms buffer must
> accommodate all kinds of variables,
> - VariableCommonInitialize() [Variable.c]: the preallocated scratch space
> must also accommodate all kinds of variables,
> - InitNonVolatileVariableStore() [Variable.c] can continue using
> GetNonVolatileMaxVariableSize().
>
> Don't modify the ReclaimForOS() function as it is specific to non-volatile variables and should ignore PcdMaxVolatileVariableSize.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> MdeModulePkg/MdeModulePkg.dec | 8 ++++
> MdeModulePkg/MdeModulePkg.uni | 8 ++++
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 1 +
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 1 +
> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h | 12 +++++
> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 50 +++++++++++++++++---
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 2 +-
> 7 files changed, 74 insertions(+), 8 deletions(-)
>
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec index 5d561ff48495..cc397185f7b9
> 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1043,6 +1043,14 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
> # @Prompt Maximum authenticated variable size.
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x00|UINT32|0x30
> 000009
>
> + ## The maximum size of a single non-authenticated volatile variable.
> + # The default value is 0 for compatibility: in that case, the
> + maximum # non-authenticated volatile variable size remains specified
> + by # PcdMaxVariableSize. Only the
> + MdeModulePkg/Universal/Variable/RuntimeDxe
> + # driver supports this PCD.
> + # @Prompt Maximum non-authenticated volatile variable size.
> +
> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x00|UINT3
> + 2|
> + 0x3000000a
> +
> ## The maximum size of single hardware error record variable.<BR><BR>
> # In IA32/X64 platforms, this value should be larger than 1KB.<BR>
> # In IA64 platforms, this value should be larger than 128KB.<BR>
> diff --git a/MdeModulePkg/MdeModulePkg.uni
> b/MdeModulePkg/MdeModulePkg.uni index f3fa616438b0..080b8a62c045
> 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -94,6 +94,14 @@
> #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxAuthVariableSize_HELP #language en-US "The maximum size of a single authenticated variable."
> "The value is 0 as default for compatibility that maximum authenticated variable size is specified by PcdMaxVariableSize."
>
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxVolatileVariableSize_PROMPT #language en-US "The maximum size of a single non-authenticated volatile variable."
> +
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxVolatileVariableSize_HELP #language en-US "The maximum size of a single non-authenticated volatile variable.<BR><BR>\n"
> + "The default value is 0 for compatibility: in that case, the maximum "
> + "non-authenticated volatile variable size remains specified by "
> + "PcdMaxVariableSize.<BR>\n"
> + "Only the MdeModulePkg/Universal/Variable/RuntimeDxe driver supports this PCD.<BR>"
> +
> #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxHardwareErrorVariableSize_PROMPT #language en-US "Maximum HwErr variable size"
>
> #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxHardwareErrorVariableSize_HELP #language en-US "The maximum size of single hardware error record variable.<BR><BR>\n"
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> index e840fc9bff40..2d0a172ece35 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.in
> +++ f
> @@ -123,6 +123,7 @@ [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize ## CONSUMES
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> index 69966f0d37ee..dbb0674a46ad 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> @@ -125,6 +125,7 @@ [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize ## CONSUMES
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> index b35e8ab91273..938eb5de61fa 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> @@ -101,6 +101,7 @@ typedef struct {
> UINTN HwErrVariableTotalSize;
> UINTN MaxVariableSize;
> UINTN MaxAuthVariableSize;
> + UINTN MaxVolatileVariableSize;
> UINTN ScratchBufferSize;
> CHAR8 *PlatformLangCodes;
> CHAR8 *LangCodes;
> @@ -460,6 +461,17 @@ GetNonVolatileMaxVariableSize (
> VOID
> );
>
> +/**
> + Get maximum variable size, covering both non-volatile and volatile variables.
> +
> + @return Maximum variable size.
> +
> +**/
> +UINTN
> +GetMaxVariableSize (
> + VOID
> + );
> +
> /**
> Initializes variable write service after FVB was ready.
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index c11842b5c3ba..5a9051648004 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -2345,12 +2345,14 @@ UpdateVariable (
> CopyMem (BufferForMerge, (UINT8 *) ((UINTN)
> CacheVariable->CurrPtr + DataOffset), DataSizeOfVariable
> (CacheVariable->CurrPtr));
>
> //
> - // Set Max Common/Auth Variable Data Size as default MaxDataSize.
> + // Set Max Auth/Non-Volatile/Volatile Variable Data Size as default MaxDataSize.
> //
> if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
> MaxDataSize = mVariableModuleGlobal->MaxAuthVariableSize - DataOffset;
> - } else {
> + } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
> MaxDataSize = mVariableModuleGlobal->MaxVariableSize -
> DataOffset;
> + } else {
> + MaxDataSize =
> + mVariableModuleGlobal->MaxVolatileVariableSize
> + - DataOffset;
> }
>
> //
> @@ -3218,16 +3220,20 @@ VariableServiceSetVariable (
> } else {
> //
> // The size of the VariableName, including the Unicode Null in bytes plus
> - // the DataSize is limited to maximum size of Max(Auth)VariableSize bytes.
> + // the DataSize is limited to maximum size of Max(Auth|Volatile)VariableSize bytes.
> //
> if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
> if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ()) {
> return EFI_INVALID_PARAMETER;
> }
> - } else {
> + } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
> if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ()) {
> return EFI_INVALID_PARAMETER;
> }
> + } else {
> + if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->MaxVolatileVariableSize - GetVariableHeaderSize ()) {
> + return EFI_INVALID_PARAMETER;
> + }
> }
> }
>
> @@ -3399,12 +3405,14 @@ VariableServiceQueryVariableInfoInternal (
> }
>
> //
> - // Let *MaximumVariableSize be Max(Auth)VariableSize with the exception of the variable header size.
> + // Let *MaximumVariableSize be Max(Auth|Volatile)VariableSize with the exception of the variable header size.
> //
> if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
> *MaximumVariableSize = mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ();
> - } else {
> + } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) {
> *MaximumVariableSize = mVariableModuleGlobal->MaxVariableSize -
> GetVariableHeaderSize ();
> + } else {
> + *MaximumVariableSize =
> + mVariableModuleGlobal->MaxVolatileVariableSize -
> + mVariableModuleGlobal->GetVariableHeaderSize
> + ();
> }
> }
>
> @@ -3657,6 +3665,30 @@ GetNonVolatileMaxVariableSize (
> }
> }
>
> +/**
> + Get maximum variable size, covering both non-volatile and volatile variables.
> +
> + @return Maximum variable size.
> +
> +**/
> +UINTN
> +GetMaxVariableSize (
> + VOID
> + )
> +{
> + UINTN MaxVariableSize;
> +
> + MaxVariableSize = GetNonVolatileMaxVariableSize();
> + //
> + // The condition below fails implicitly if
> +PcdMaxVolatileVariableSize equals
> + // the default zero value.
> + //
> + if (MaxVariableSize < PcdGet32 (PcdMaxVolatileVariableSize)) {
> + MaxVariableSize = PcdGet32 (PcdMaxVolatileVariableSize);
> + }
> + return MaxVariableSize;
> +}
> +
> /**
> Init non-volatile variable store.
>
> @@ -3810,6 +3842,10 @@ InitNonVolatileVariableStore (
>
> mVariableModuleGlobal->MaxVariableSize = PcdGet32 (PcdMaxVariableSize);
> mVariableModuleGlobal->MaxAuthVariableSize = ((PcdGet32
> (PcdMaxAuthVariableSize) != 0) ? PcdGet32 (PcdMaxAuthVariableSize) :
> mVariableModuleGlobal->MaxVariableSize);
> + mVariableModuleGlobal->MaxVolatileVariableSize = ((PcdGet32 (PcdMaxVolatileVariableSize) != 0) ?
> + PcdGet32 (PcdMaxVolatileVariableSize) :
> + mVariableModuleGlobal->MaxVariableSize
> + );
>
> //
> // Parse non-volatile variable data and get last variable offset.
> @@ -4228,7 +4264,7 @@ VariableCommonInitialize (
> //
> // Allocate memory for volatile variable store, note that there is a scratch space to store scratch data.
> //
> - ScratchSize = GetNonVolatileMaxVariableSize ();
> + ScratchSize = GetMaxVariableSize ();
> mVariableModuleGlobal->ScratchBufferSize = ScratchSize;
> VolatileVariableStore = AllocateRuntimePool (PcdGet32 (PcdVariableStoreSize) + ScratchSize);
> if (VolatileVariableStore == NULL) { diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> index 8d73b6edee51..e495d971a08b 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> @@ -955,7 +955,7 @@ VariableServiceInitialize (
> );
> ASSERT_EFI_ERROR (Status);
>
> - mVariableBufferPayloadSize = GetNonVolatileMaxVariableSize () +
> + mVariableBufferPayloadSize = GetMaxVariableSize () +
> OFFSET_OF
> (SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY, Name) -
> GetVariableHeaderSize ();
>
> Status = gSmst->SmmAllocatePool (
> --
> 2.14.1.3.gb7cf6e02401b
>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2026 Red Hat, Inc.