From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 EF36F81E18 for ; Mon, 7 Nov 2016 06:07:39 -0800 (PST) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 73C497AE92; Mon, 7 Nov 2016 14:07:42 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-129.phx2.redhat.com [10.3.116.129]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uA7E7eBt015567; Mon, 7 Nov 2016 09:07:41 -0500 To: Ruiyu Ni References: <20161104005942.345832-1-ruiyu.ni@intel.com> <20161104005942.345832-5-ruiyu.ni@intel.com> Cc: edk2-devel@ml01.01.org, Jiewen Yao From: Laszlo Ersek Message-ID: Date: Mon, 7 Nov 2016 15:07:40 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161104005942.345832-5-ruiyu.ni@intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 07 Nov 2016 14:07:42 +0000 (UTC) Subject: Re: [PATCH 4/4] MdeModulePkg/SecurityStubDxe: Report failure if image is load earlier X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Nov 2016 14:07:40 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 11/04/16 01:59, Ruiyu Ni wrote: > The 3rd party image should be loaded after EndOfDxe event signal and > DxeSmmReadyToLock protocol installation. But non-SMM platform doesn't > published DxeSmmReadyToLock protocol. > So the SecurityStubDxe can only depend on EndOfDxe event. > > This patch enhances the SecurityStubDxe to listen on > DxeSmmReadyToLock protocol installation and if any 3rd party image > is loaded before DxeSmmReadyToLock, it reports failure. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni > Cc: Jiewen Yao > --- > .../SecurityStubDxe/Defer3rdPartyImageLoad.c | 57 ++++++++++++++++++++++ > .../SecurityStubDxe/Defer3rdPartyImageLoad.h | 5 +- > .../Universal/SecurityStubDxe/SecurityStubDxe.inf | 3 ++ > 3 files changed, 64 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c b/MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c > index ca45d56..84d573b 100644 > --- a/MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c > +++ b/MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c > @@ -30,6 +30,7 @@ typedef struct { > DEFERRED_3RD_PARTY_IMAGE_INFO *ImageInfo; ///< deferred 3rd party image item > } DEFERRED_3RD_PARTY_IMAGE_TABLE; > > +BOOLEAN mImageLoadedAfterEndOfDxe = FALSE; > BOOLEAN mEndOfDxe = FALSE; > DEFERRED_3RD_PARTY_IMAGE_TABLE mDeferred3rdPartyImage = { > 0, // Deferred image count > @@ -257,6 +258,52 @@ EndOfDxe ( > } > > /** > + Event notification for gEfiDxeSmmReadyToLockProtocolGuid event. > + > + This function reports failure if any deferred image is loaded before > + this callback. > + Platform should publish ReadyToLock protocol immediately after signaling > + of the End of DXE Event. > + > + @param Event The Event that is being processed, not used. > + @param Context Event Context, not used. > + > +**/ > +VOID > +EFIAPI > +DxdSmmReadyToLock ( "Dxd" is a typo, it should be "Dxe". > + IN EFI_EVENT Event, > + IN VOID *Context > + ) > +{ > + EFI_STATUS Status; > + VOID *Interface; > + > + Status = gBS->LocateProtocol (&gEfiDxeSmmReadyToLockProtocolGuid, NULL, &Interface); > + if (EFI_ERROR (Status)) { > + return; > + } > + > + gBS->CloseEvent (Event); > + > + if (mImageLoadedAfterEndOfDxe) { > + // > + // Platform should not dispatch the 3rd party images after signaling EndOfDxe event > + // but before publishing DxeSmmReadyToLock protocol. > + // > + DEBUG (( > + DEBUG_ERROR, > + "[Security] 3rd party images must be dispatched after DxeSmmReadyToLock Protocol installation!\n" > + )); > + REPORT_STATUS_CODE ( > + EFI_ERROR_CODE | EFI_ERROR_UNRECOVERED, > + (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_EC_ILLEGAL_SOFTWARE_STATE) > + ); > + ASSERT (FALSE); I suggest to add an explicit CpuDeadLoop() call here, after the ASSERT(FALSE). I learned this pattern from Mike: https://www.mail-archive.com/edk2-devel@lists.01.org/msg04590.html Thanks! Laszlo > + } > +} > + > +/** > Defer the 3rd party image load and installs Deferred Image Load Protocol. > > @param[in] File This is a pointer to the device path of the file that > @@ -303,6 +350,7 @@ Defer3rdPartyImageLoad ( > ); > > if (mEndOfDxe) { > + mImageLoadedAfterEndOfDxe = TRUE; > // > // The image might be first time loaded after EndOfDxe, > // So ImageInfo can be NULL. > @@ -334,6 +382,7 @@ Defer3rdPartyImageLoadInitialize ( > EFI_STATUS Status; > EFI_HANDLE Handle; > EFI_EVENT Event; > + VOID *Registration; > > Handle = NULL; > Status = gBS->InstallMultipleProtocolInterfaces ( > @@ -353,4 +402,12 @@ Defer3rdPartyImageLoadInitialize ( > &Event > ); > ASSERT_EFI_ERROR (Status); > + > + EfiCreateProtocolNotifyEvent ( > + &gEfiDxeSmmReadyToLockProtocolGuid, > + TPL_CALLBACK, > + DxdSmmReadyToLock, > + NULL, > + &Registration > + ); > } > diff --git a/MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.h b/MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.h > index 3fab258..75553ba 100644 > --- a/MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.h > +++ b/MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.h > @@ -15,16 +15,19 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > #ifndef _DEFER_3RD_PARTY_IMAGE_LOAD_H_ > #define _DEFER_3RD_PARTY_IMAGE_LOAD_H_ > > -#include > +#include > #include > #include > #include > +#include > > #include > #include > #include > #include > #include > +#include > +#include > > /** > Returns information about a deferred image. > diff --git a/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf b/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf > index be6ce6c..7f8f6cb 100644 > --- a/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf > +++ b/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf > @@ -41,6 +41,8 @@ [LibraryClasses] > UefiBootServicesTableLib > DebugLib > SecurityManagementLib > + ReportStatusCodeLib > + UefiLib > > [Guids] > gEfiEndOfDxeEventGroupGuid ## CONSUMES ## Event > @@ -49,6 +51,7 @@ [Protocols] > gEfiSecurityArchProtocolGuid ## PRODUCES > gEfiSecurity2ArchProtocolGuid ## PRODUCES > gEfiDeferredImageLoadProtocolGuid ## PRODUCES > + gEfiDxeSmmReadyToLockProtocolGuid ## CONSUMES > > [Depex] > TRUE >