From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C554B22106DCE for ; Wed, 28 Mar 2018 18:27:55 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Mar 2018 18:34:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,374,1517904000"; d="scan'208";a="32299410" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga002.fm.intel.com with ESMTP; 28 Mar 2018 18:34:34 -0700 Received: from FMSMSX109.amr.corp.intel.com (10.18.116.9) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 28 Mar 2018 18:34:34 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx109.amr.corp.intel.com (10.18.116.9) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 28 Mar 2018 18:34:33 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.80]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.235]) with mapi id 14.03.0319.002; Thu, 29 Mar 2018 09:34:32 +0800 From: "Zeng, Star" To: Laszlo Ersek , edk2-devel-01 CC: "Dong, Eric" , "Ni, Ruiyu" , "Zeng, Star" Thread-Topic: [PATCH 1/4] MdeModulePkg/Variable/RuntimeDxe: introduce PcdMaxVolatileVariableSize Thread-Index: AQHTxtMhsMuOHAKSPEuRjBP8hx/e2qPmbY/Q Date: Thu, 29 Mar 2018 01:34:31 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103BA7437D@shsmsx102.ccr.corp.intel.com> References: <20180328202651.1478-1-lersek@redhat.com> <20180328202651.1478-2-lersek@redhat.com> In-Reply-To: <20180328202651.1478-2-lersek@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH 1/4] MdeModulePkg/Variable/RuntimeDxe: introduce PcdMaxVolatileVariableSize X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Mar 2018 01:27:56 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 i= n VariableCommonInitialize(), but not InitNonVolatileVariableStore()?=20 mVariableModuleGlobal->MaxVolatileVariableSize =3D ((PcdGet32 (PcdMaxVola= tileVariableSize) !=3D 0) ? PcdGet32 (PcdMaxVolatil= eVariableSize) : mVariableModuleGlobal->= MaxVariableSize ); For example, have it right before the comments in VariableCommonInitialize(= ). // // Allocate memory for volatile variable store, note that there is a scra= tch space to store scratch data. // Thanks, Star -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com]=20 Sent: Thursday, March 29, 2018 4:27 AM To: edk2-devel-01 Cc: Dong, Eric ; Ni, Ruiyu ; Zeng,= Star Subject: [PATCH 1/4] MdeModulePkg/Variable/RuntimeDxe: introduce PcdMaxVola= tileVariableSize The variable driver doesn't distinguish "non-volatile non-authenticated" variables from "volatile non-authenticated" variables, when checking indivi= dual 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_T= LS_CA_CERTIFICATE_VARIABLE, which platforms might want to create as volatil= e on every boot: the certificate list can be several hundred KB in size. Introduce PcdMaxVolatileVariableSize to represent the limit on individual v= olatile non-authenticated variables. The default value is zero, which makes= Variable/RuntimeDxe fall back to PcdMaxVariableSize (i.e. the current beha= vior). 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 PcdMaxVolatileVariabl= eSize, in addition to GetNonVolatileMaxVariableSize(). GetNonVolatileMaxVar= iableSize() is currently called at three sites, and two of those need to st= art 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 Cc: Ruiyu Ni Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek --- 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|0x3000= 0009 =20 + ## The maximum size of a single non-authenticated volatile variable. + # The default value is 0 for compatibility: in that case, the maximum =20 + # non-authenticated volatile variable size remains specified by #=20 + PcdMaxVariableSize. Only the=20 + MdeModulePkg/Universal/Variable/RuntimeDxe + # driver supports this PCD. + # @Prompt Maximum non-authenticated volatile variable size. + =20 + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x00|UINT32| + 0x3000000a + ## The maximum size of single hardware error record variable.

# In IA32/X64 platforms, this value should be larger than 1KB.
# In IA64 platforms, this value should be larger than 128KB.
diff --= git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index f= 3fa616438b0..080b8a62c045 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -94,6 +94,14 @@ #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxAuthVariableSize_HELP #l= anguage en-US "The maximum size of a single authenticated variable." = "The value is 0 as default for compatibility that maximum aut= henticated variable size is specified by PcdMaxVariableSize." =20 +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxVolatileVariableSize_PROM= PT #language en-US "The maximum size of a single non-authenticated volatil= e variable." + +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxVolatileVariableSize_HELP= #language en-US "The maximum size of a single non-authenticated volatile = variable.

\n" + = "The default value is 0 for compatibility: in that case, = the maximum " + = "non-authenticated volatile variable size remains specifi= ed by " + = "PcdMaxVariableSize.
\n" + = "Only the MdeModulePkg/Universal/Variable/RuntimeDxe driv= er supports this PCD.
" + #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxHardwareErrorVariableSize= _PROMPT #language en-US "Maximum HwErr variable size" =20 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaxHardwareErrorVariableSize= _HELP #language en-US "The maximum size of single hardware error record va= riable.

\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 ## CON= SUMES gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CON= SUMES gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CON= SUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CON= SUMES gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize ## CON= SUMES gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize ## CON= SUMES gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize ## CON= SUMES diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf b/M= deModulePkg/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 ## CO= NSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize ## CO= NSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize ## CO= NSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize ## CO= NSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize ## CO= NSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize ## CO= NSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize ## CO= NSUMES diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h b/MdeMod= ulePkg/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 ); =20 +/** + Get maximum variable size, covering both non-volatile and volatile varia= bles. + + @return Maximum variable size. + +**/ +UINTN +GetMaxVariableSize ( + VOID + ); + /** Initializes variable write service after FVB was ready. =20 diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeMod= ulePkg/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)); =20 // - // Set Max Common/Auth Variable Data Size as default MaxDataSize. + // Set Max Auth/Non-Volatile/Volatile Variable Data Size as defaul= t MaxDataSize. // if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) !=3D 0) { MaxDataSize =3D mVariableModuleGlobal->MaxAuthVariableSize - Dat= aOffset; - } else { + } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) !=3D 0) { MaxDataSize =3D mVariableModuleGlobal->MaxVariableSize - DataOff= set; + } else { + MaxDataSize =3D mVariableModuleGlobal->MaxVolatileVariableSize=20 + - DataOffset; } =20 // @@ -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 b= ytes. + // the DataSize is limited to maximum size of Max(Auth|Volatile)Varia= bleSize bytes. // if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) !=3D 0) { if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->Ma= xAuthVariableSize - GetVariableHeaderSize ()) { return EFI_INVALID_PARAMETER; } - } else { + } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) !=3D 0) { if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->Ma= xVariableSize - GetVariableHeaderSize ()) { return EFI_INVALID_PARAMETER; } + } else { + if (StrSize (VariableName) + PayloadSize > mVariableModuleGlobal->Ma= xVolatileVariableSize - GetVariableHeaderSize ()) { + return EFI_INVALID_PARAMETER; + } } } =20 @@ -3399,12 +3405,14 @@ VariableServiceQueryVariableInfoInternal ( } =20 // - // Let *MaximumVariableSize be Max(Auth)VariableSize with the exceptio= n 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) !=3D 0) { *MaximumVariableSize =3D mVariableModuleGlobal->MaxAuthVariableSize = - GetVariableHeaderSize (); - } else { + } else if ((Attributes & EFI_VARIABLE_NON_VOLATILE) !=3D 0) { *MaximumVariableSize =3D mVariableModuleGlobal->MaxVariableSize - Ge= tVariableHeaderSize (); + } else { + *MaximumVariableSize =3D=20 + mVariableModuleGlobal->MaxVolatileVariableSize - GetVariableHeaderSize=20 + (); } } =20 @@ -3657,6 +3665,30 @@ GetNonVolatileMaxVariableSize ( } } =20 +/** + Get maximum variable size, covering both non-volatile and volatile varia= bles. + + @return Maximum variable size. + +**/ +UINTN +GetMaxVariableSize ( + VOID + ) +{ + UINTN MaxVariableSize; + + MaxVariableSize =3D GetNonVolatileMaxVariableSize(); + // + // The condition below fails implicitly if PcdMaxVolatileVariableSize=20 +equals + // the default zero value. + // + if (MaxVariableSize < PcdGet32 (PcdMaxVolatileVariableSize)) { + MaxVariableSize =3D PcdGet32 (PcdMaxVolatileVariableSize); + } + return MaxVariableSize; +} + /** Init non-volatile variable store. =20 @@ -3810,6 +3842,10 @@ InitNonVolatileVariableStore ( =20 mVariableModuleGlobal->MaxVariableSize =3D PcdGet32 (PcdMaxVariableSize)= ; mVariableModuleGlobal->MaxAuthVariableSize =3D ((PcdGet32 (PcdMaxAuthVar= iableSize) !=3D 0) ? PcdGet32 (PcdMaxAuthVariableSize) : mVariableModuleGlo= bal->MaxVariableSize); + mVariableModuleGlobal->MaxVolatileVariableSize =3D ((PcdGet32 (PcdMaxVol= atileVariableSize) !=3D 0) ? + PcdGet32 (PcdMaxVolati= leVariableSize) : + mVariableModuleGlobal-= >MaxVariableSize + ); =20 // // 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 scr= atch space to store scratch data. // - ScratchSize =3D GetNonVolatileMaxVariableSize (); + ScratchSize =3D GetMaxVariableSize (); mVariableModuleGlobal->ScratchBufferSize =3D ScratchSize; VolatileVariableStore =3D AllocateRuntimePool (PcdGet32 (PcdVariableStor= eSize) + ScratchSize); if (VolatileVariableStore =3D=3D NULL) { diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/Mde= ModulePkg/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); =20 - mVariableBufferPayloadSize =3D GetNonVolatileMaxVariableSize () + + mVariableBufferPayloadSize =3D GetMaxVariableSize () + OFFSET_OF (SMM_VARIABLE_COMMUNICATE_VAR_CHE= CK_VARIABLE_PROPERTY, Name) - GetVariableHeaderSize (); =20 Status =3D gSmst->SmmAllocatePool ( -- 2.14.1.3.gb7cf6e02401b