[edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy
Yao, Jiewen
jiewen.yao at intel.com
Wed May 3 01:21:03 UTC 2023
That only proves that you did change the interface. But that cannot prove you change it right.
Have you done any function test? For example:
1) The HDD password feature still works?
2) The variable is really locked?
> -----Original Message-----
> From: Liu, Linus <linus.liu at intel.com>
> Sent: Wednesday, May 3, 2023 8:40 AM
> To: Yao, Jiewen <jiewen.yao at intel.com>; devel at edk2.groups.io
> Cc: FST-FIR-PRC <fst-fir-prc at intel.com>; FST FIR Server
> <fst.fir.server at intel.com>; Chu, Maggie <maggie.chu at intel.com>
> Subject: RE: [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit
> to use Variable Policy
>
> Hi Jiewen
> I add this patch into MTLS platform and collect the log.
> The below is before adding patch and after adding patch. There is no warring
> message.
>
>
> Before
>
> InstallProtocolInterface: 09576E91-6D3F-11D2-8E39-00A0C969723B
> 67E4C490
> InstallProtocolInterface: 330D4706-F2A0-4E4F-A369-B66FA8D54385
> 68180030
> !!! DEPRECATED INTERFACE !!! VariableLockRequestToLock() will go away
> soon!
> !!! DEPRECATED INTERFACE !!! Please move to use Variable Policy!
> !!! DEPRECATED INTERFACE !!! Variable: 737CDED7-448B-4801-B57D-
> B19483EC606F HddPassword
> HddPasswordDxeInit(): Lock HddPassword variable (Success)
>
>
> After
>
> InstallProtocolInterface: 09576E91-6D3F-11D2-8E39-00A0C969723B
> 67EA1370
> InstallProtocolInterface: 330D4706-F2A0-4E4F-A369-B66FA8D54385
> 68153DB0
> HddPasswordDxeInit(): Lock HddPassword variable (Success)
>
>
> Thanks
>
>
>
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao at intel.com>
> Sent: Wednesday, May 3, 2023 12:11 AM
> To: Liu, Linus <linus.liu at intel.com>; devel at edk2.groups.io
> Cc: FST-FIR-PRC <fst-fir-prc at intel.com>; FST FIR Server
> <fst.fir.server at intel.com>; Chu, Maggie <maggie.chu at intel.com>
> Subject: RE: [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit
> to use Variable Policy
>
> Thanks. The patch loos good to me.
>
> Would you please share with us, how you validate the patch?
>
>
>
> > -----Original Message-----
> > From: Liu, Linus <linus.liu at intel.com>
> > Sent: Tuesday, April 11, 2023 5:55 PM
> > To: devel at edk2.groups.io
> > Cc: Yao, Jiewen <jiewen.yao at intel.com>; FST-FIR-PRC <fst-fir-
> > prc at intel.com>; FST FIR Server <fst.fir.server at intel.com>; Chu, Maggie
> > <maggie.chu at intel.com>
> > Subject: [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit
> to
> > use Variable Policy
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4408
> >
> > Change-Id: I3c4b466ef318766d6d70c9f73e36b94b5f10832c
> > Cc: Jiewen Yao <jiewen.yao at intel.com>
> > Cc: FST-FIR-PRC <fst-fir-prc at intel.com>
> > Cc: FST FIR Server C <fst.fir.server at intel.com>
> > Cc: Maggie Chu <maggie.chu at intel.com>
> > Signed-off-by: Linus Liu <linus.liu at intel.com>
> > ---
> > SecurityPkg/HddPassword/HddPasswordDxe.c | 16 +++++++++++-----
> > SecurityPkg/HddPassword/HddPasswordDxe.h | 1 -
> > SecurityPkg/HddPassword/HddPasswordDxe.inf | 3 ++-
> > SecurityPkg/SecurityPkg.dsc | 1 +
> > 4 files changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.c
> > b/SecurityPkg/HddPassword/HddPasswordDxe.c
> > index a1a63b67a4..c20fdbe83f 100644
> > --- a/SecurityPkg/HddPassword/HddPasswordDxe.c
> > +++ b/SecurityPkg/HddPassword/HddPasswordDxe.c
> > @@ -9,6 +9,7 @@
> > **/
> >
> >
> >
> > #include "HddPasswordDxe.h"
> >
> > +#include <Library/VariablePolicyHelperLib.h>
> >
> >
> >
> > EFI_GUID mHddPasswordVendorGuid =
> > HDD_PASSWORD_CONFIG_GUID;
> >
> > CHAR16 mHddPasswordVendorStorageName[] =
> > L"HDD_PASSWORD_CONFIG";
> >
> > @@ -2822,7 +2823,7 @@ HddPasswordDxeInit (
> > HDD_PASSWORD_DXE_PRIVATE_DATA *Private;
> >
> > VOID *Registration;
> >
> > EFI_EVENT EndOfDxeEvent;
> >
> > - EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock;
> >
> > + EDKII_VARIABLE_POLICY_PROTOCOL *VariablePolicy;
> >
> >
> >
> > Private = NULL;
> >
> >
> >
> > @@ -2858,12 +2859,17 @@ HddPasswordDxeInit (
> > //
> >
> > // Make HDD_PASSWORD_VARIABLE_NAME variable read-only.
> >
> > //
> >
> > - Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid,
> > NULL, (VOID **)&VariableLock);
> >
> > + Status = gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid,
> > + NULL,
> > (VOID **)&VariablePolicy);
> >
> > if (!EFI_ERROR (Status)) {
> >
> > - Status = VariableLock->RequestToLock (
> >
> > - VariableLock,
> >
> > + Status = RegisterBasicVariablePolicy (
> >
> > + VariablePolicy,
> >
> > + &mHddPasswordVendorGuid,
> >
> > HDD_PASSWORD_VARIABLE_NAME,
> >
> > - &mHddPasswordVendorGuid
> >
> > + VARIABLE_POLICY_NO_MIN_SIZE,
> >
> > + VARIABLE_POLICY_NO_MAX_SIZE,
> >
> > + VARIABLE_POLICY_NO_MUST_ATTR,
> >
> > + VARIABLE_POLICY_NO_CANT_ATTR,
> >
> > + VARIABLE_POLICY_TYPE_LOCK_NOW
> >
> > );
> >
> > DEBUG ((DEBUG_INFO, "%a(): Lock %s variable (%r)\n",
> > __FUNCTION__, HDD_PASSWORD_VARIABLE_NAME, Status));
> >
> > ASSERT_EFI_ERROR (Status);
> >
> > diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.h
> > b/SecurityPkg/HddPassword/HddPasswordDxe.h
> > index 231533e737..049a208794 100644
> > --- a/SecurityPkg/HddPassword/HddPasswordDxe.h
> > +++ b/SecurityPkg/HddPassword/HddPasswordDxe.h
> > @@ -17,7 +17,6 @@
> > #include <Protocol/AtaPassThru.h>
> >
> > #include <Protocol/PciIo.h>
> >
> > #include <Protocol/HiiConfigAccess.h>
> >
> > -#include <Protocol/VariableLock.h>
> >
> >
> >
> > #include <Guid/MdeModuleHii.h>
> >
> > #include <Guid/EventGroup.h>
> >
> > diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.inf
> > b/SecurityPkg/HddPassword/HddPasswordDxe.inf
> > index 06e8755ffc..2c0ebbcc78 100644
> > --- a/SecurityPkg/HddPassword/HddPasswordDxe.inf
> > +++ b/SecurityPkg/HddPassword/HddPasswordDxe.inf
> > @@ -50,6 +50,7 @@
> > PrintLib
> >
> > UefiLib
> >
> > LockBoxLib
> >
> > + VariablePolicyHelperLib
> >
> > S3BootScriptLib
> >
> > PciLib
> >
> > BaseCryptLib
> >
> > @@ -63,7 +64,7 @@
> > gEfiHiiConfigAccessProtocolGuid ## PRODUCES
> >
> > gEfiAtaPassThruProtocolGuid ## CONSUMES
> >
> > gEfiPciIoProtocolGuid ## CONSUMES
> >
> > - gEdkiiVariableLockProtocolGuid ## CONSUMES
> >
> > + gEdkiiVariablePolicyProtocolGuid ## CONSUMES
> >
> >
> >
> > [Pcd]
> >
> > gEfiSecurityPkgTokenSpaceGuid.PcdSkipHddPasswordPrompt ##
> CONSUMES
> >
> > diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
> > index 3bad5375c0..3c62205162 100644
> > --- a/SecurityPkg/SecurityPkg.dsc
> > +++ b/SecurityPkg/SecurityPkg.dsc
> > @@ -74,6 +74,7 @@
> >
> > PlatformPKProtectionLib|SecurityPkg/Library/PlatformPKProtectionLibVar
> > PlatformPKProtectionLib|Po
> > licy/PlatformPKProtectionLibVarPolicy.inf
> >
> >
> > SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariableP
> > SecureBootVariableProvisionLib|ro
> > visionLib/SecureBootVariableProvisionLib.inf
> >
> > TdxLib|MdePkg/Library/TdxLib/TdxLib.inf
> >
> > +
> > VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/V
> > VariablePolicyHelperLib|ar
> > iablePolicyHelperLib.inf
> >
> >
> >
> > [LibraryClasses.ARM, LibraryClasses.AARCH64]
> >
> > #
> >
> > --
> > 2.33.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103871): https://edk2.groups.io/g/devel/message/103871
Mute This Topic: https://groups.io/mt/98224856/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list