From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.100, mailfrom: hao.a.wu@intel.com) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by groups.io with SMTP; Thu, 03 Oct 2019 01:04:12 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Oct 2019 01:04:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.67,251,1566889200"; d="scan'208";a="221692287" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga002.fm.intel.com with ESMTP; 03 Oct 2019 01:04:09 -0700 Received: from fmsmsx114.amr.corp.intel.com (10.18.116.8) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 3 Oct 2019 01:04:09 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX114.amr.corp.intel.com (10.18.116.8) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 3 Oct 2019 01:04:08 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.166]) by shsmsx102.ccr.corp.intel.com ([169.254.2.176]) with mapi id 14.03.0439.000; Thu, 3 Oct 2019 16:04:06 +0800 From: "Wu, Hao A" To: "devel@edk2.groups.io" , "Kubacki, Michael A" CC: "Bi, Dandan" , Ard Biesheuvel , "Dong, Eric" , Laszlo Ersek , "Gao, Liming" , "Kinney, Michael D" , "Ni, Ray" , "Wang, Jian J" , "Yao, Jiewen" Subject: Re: [edk2-devel] [PATCH V2 4/9] MdeModulePkg/Variable: Add local auth status in VariableParsing Thread-Topic: [edk2-devel] [PATCH V2 4/9] MdeModulePkg/Variable: Add local auth status in VariableParsing Thread-Index: AQHVdZ6zvg1BoZXtVUOkq6ImrvN3bKdCQ9cg Date: Thu, 3 Oct 2019 08:04:05 +0000 Message-ID: References: <20190928014717.31372-1-michael.a.kubacki@intel.com> <20190928014717.31372-5-michael.a.kubacki@intel.com> In-Reply-To: <20190928014717.31372-5-michael.a.kubacki@intel.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 Return-Path: hao.a.wu@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Inline comments below: > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Kubacki, Michael A > Sent: Saturday, September 28, 2019 9:47 AM > To: devel@edk2.groups.io > Cc: Bi, Dandan; Ard Biesheuvel; Dong, Eric; Laszlo Ersek; Gao, Liming; K= inney, > Michael D; Ni, Ray; Wang, Jian J; Wu, Hao A; Yao, Jiewen > Subject: [edk2-devel] [PATCH V2 4/9] MdeModulePkg/Variable: Add local > auth status in VariableParsing >=20 > The file VariableParsing.c provides generic functionality related > to parsing variable related structures and information. In order to > calculate offsets for certain operations, the functions must know if > authenticated variables are enabled as this increases the size of > variable headers. >=20 > This change removes linking against a global variable in an external fil= e > in favor of a statically scoped variable in VariableParsing.c Because th= is > file is unaware of how the authenticated variable status is determined, = the > variable is set through a function interface invoked during variable dri= ver > initialization. >=20 > Cc: Dandan Bi > Cc: Ard Biesheuvel > Cc: Eric Dong > Cc: Laszlo Ersek > Cc: Liming Gao > Cc: Michael D Kinney > Cc: Ray Ni > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Jiewen Yao > Signed-off-by: Michael Kubacki > --- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h | 14 > +++++++++ > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 10 +++--= - > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c | 33 > ++++++++++++++++---- > 3 files changed, 45 insertions(+), 12 deletions(-) >=20 > diff --git > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h > index 6f2000f3ee..3eba590634 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h > @@ -308,4 +308,18 @@ UpdateVariableInfo ( > IN OUT VARIABLE_INFO_ENTRY **VariableInfo > ); >=20 > +/** > + Initializes context needed for variable parsing functions. > + > + @param[in] AuthFormat If true then indicates authentic= ated > variables are supported > + > + @retval EFI_SUCCESS Initialized successfully > + @retval Others An error occurred during initial= ization > +**/ > +EFI_STATUS > +EFIAPI > +InitVariableParsing ( InitVariableParsing() seems an internal function, the 'EFIAPI' keyword can= be dropped. Please help to update the function definition in .C file as well. > + IN BOOLEAN AuthFormat > + ); > + > #endif > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > index 1a57d7e1ba..53d797152c 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > @@ -3326,6 +3326,9 @@ InitNonVolatileVariableStore ( > mVariableModuleGlobal->MaxVariableSize =3D PcdGet32 > (PcdMaxVariableSize); > mVariableModuleGlobal->MaxAuthVariableSize =3D ((PcdGet32 > (PcdMaxAuthVariableSize) !=3D 0) ? PcdGet32 (PcdMaxAuthVariableSize) : > mVariableModuleGlobal->MaxVariableSize); >=20 > + Status =3D InitVariableParsing (mVariableModuleGlobal- > >VariableGlobal.AuthFormat); > + ASSERT_EFI_ERROR (Status); > + After the above initialization, mVariableModuleGlobal->VariableGlobal.Auth= Format will be changed temporarily within ConvertNormalVarStorageToAuthVarStorage= () if normal HOB variable store will be converted to the auth format: VOID * ConvertNormalVarStorageToAuthVarStorage ( VARIABLE_STORE_HEADER *NormalVarStorage ) { ... // // Set AuthFormat as FALSE for normal variable storage // mVariableModuleGlobal->VariableGlobal.AuthFormat =3D FALSE; ... // // Restore AuthFormat // mVariableModuleGlobal->VariableGlobal.AuthFormat =3D TRUE; return AuthVarStorage; } I think there will be issues in such converting, since I found that at lea= st GetVariableHeaderSize() and NameSizeOfVariable() get called during the execution of ConvertNormalVarStorageToAuthVarStorage(). And they are check= ing 'mAuthFormat' rather than 'mVariableModuleGlobal->VariableGlobal.AuthForma= t'. > // > // Parse non-volatile variable data and get last variable offset. > // > @@ -3756,18 +3759,13 @@ VariableCommonInitialize ( >=20 > // > // mVariableModuleGlobal->VariableGlobal.AuthFormat > - // has been initialized in InitNonVolatileVariableStore(). > + // is initialized in InitNonVolatileVariableStore(). > // > if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > DEBUG ((EFI_D_INFO, "Variable driver will work with auth variable > format!\n")); > - // > - // Set AuthSupport to FALSE first, VariableWriteServiceInitialize()= will > initialize it. > - // > - mVariableModuleGlobal->VariableGlobal.AuthSupport =3D FALSE; > VariableGuid =3D &gEfiAuthenticatedVariableGuid; > } else { > DEBUG ((EFI_D_INFO, "Variable driver will work without auth variabl= e > support!\n")); > - mVariableModuleGlobal->VariableGlobal.AuthSupport =3D FALSE; Not sure why the above changes belong to this patch. Could you help to double confirm? Best Regards, Hao Wu > VariableGuid =3D &gEfiVariableGuid; > } >=20 > diff --git > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c > index 394195342d..0a47f6d10d 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c > @@ -9,6 +9,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > #include "VariableParsing.h" >=20 > +STATIC BOOLEAN mAuthFormat; > + > /** >=20 > This code checks if variable header is valid or not. > @@ -88,7 +90,7 @@ GetVariableHeaderSize ( > { > UINTN Value; >=20 > - if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > + if (mAuthFormat) { > Value =3D sizeof (AUTHENTICATED_VARIABLE_HEADER); > } else { > Value =3D sizeof (VARIABLE_HEADER); > @@ -114,7 +116,7 @@ NameSizeOfVariable ( > AUTHENTICATED_VARIABLE_HEADER *AuthVariable; >=20 > AuthVariable =3D (AUTHENTICATED_VARIABLE_HEADER *) Variable; > - if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > + if (mAuthFormat) { > if (AuthVariable->State =3D=3D (UINT8) (-1) || > AuthVariable->DataSize =3D=3D (UINT32) (-1) || > AuthVariable->NameSize =3D=3D (UINT32) (-1) || > @@ -149,7 +151,7 @@ SetNameSizeOfVariable ( > AUTHENTICATED_VARIABLE_HEADER *AuthVariable; >=20 > AuthVariable =3D (AUTHENTICATED_VARIABLE_HEADER *) Variable; > - if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > + if (mAuthFormat) { > AuthVariable->NameSize =3D (UINT32) NameSize; > } else { > Variable->NameSize =3D (UINT32) NameSize; > @@ -173,7 +175,7 @@ DataSizeOfVariable ( > AUTHENTICATED_VARIABLE_HEADER *AuthVariable; >=20 > AuthVariable =3D (AUTHENTICATED_VARIABLE_HEADER *) Variable; > - if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > + if (mAuthFormat) { > if (AuthVariable->State =3D=3D (UINT8) (-1) || > AuthVariable->DataSize =3D=3D (UINT32) (-1) || > AuthVariable->NameSize =3D=3D (UINT32) (-1) || > @@ -208,7 +210,7 @@ SetDataSizeOfVariable ( > AUTHENTICATED_VARIABLE_HEADER *AuthVariable; >=20 > AuthVariable =3D (AUTHENTICATED_VARIABLE_HEADER *) Variable; > - if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > + if (mAuthFormat) { > AuthVariable->DataSize =3D (UINT32) DataSize; > } else { > Variable->DataSize =3D (UINT32) DataSize; > @@ -248,7 +250,7 @@ GetVendorGuidPtr ( > AUTHENTICATED_VARIABLE_HEADER *AuthVariable; >=20 > AuthVariable =3D (AUTHENTICATED_VARIABLE_HEADER *) Variable; > - if (mVariableModuleGlobal->VariableGlobal.AuthFormat) { > + if (mAuthFormat) { > return &AuthVariable->VendorGuid; > } else { > return &Variable->VendorGuid; > @@ -746,3 +748,22 @@ UpdateVariableInfo ( > } > } > } > + > +/** > + Initializes context needed for variable parsing functions. > + > + @param[in] AuthFormat If true then indicates authentic= ated > variables are supported > + > + @retval EFI_SUCCESS Initialized successfully > + @retval Others An error occurred during initial= ization > +**/ > +EFI_STATUS > +EFIAPI > +InitVariableParsing ( > + IN BOOLEAN AuthFormat > + ) > +{ > + mAuthFormat =3D AuthFormat; > + > + return EFI_SUCCESS; > +} > -- > 2.16.2.windows.1 >=20 >=20 >=20