Re: [edk2-devel] [PATCH v2 1/1] SecurityPkg/DxeImageVerificationLib: Check result of GetEfiGlobalVariable2

["Yao, Jiewen" <jiewen.yao@intel.com>]

Sounds good. Thanks.

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: kraxel@redhat.com <kraxel@redhat.com>
> Sent: Monday, March 20, 2023 11:00 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Pawel Polawski <ppolawsk@redhat.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Oliver Steffen <osteffen@redhat.com>; Xu, Min M
> <min.m.xu@intel.com>; Marvin Häuser <mhaeuser@posteo.de>;
> jmaloy@redhat.com
> Subject: Re: [edk2-devel] [PATCH v2 1/1]
> SecurityPkg/DxeImageVerificationLib: Check result of GetEfiGlobalVariable2
> 
> On Mon, Mar 20, 2023 at 01:20:29PM +0000, Yao, Jiewen wrote:
> > Would you please share with us what test has been done for this patch?
> 
> Usual regression testing, including booting images with and without
> secure boot.  Additionally checked images with the wrong signature
> are rejected (try boot grub.efi directly instead of using the
> shim.efi -> grub.efi chain).
> 
> take care,
>   Gerd
> 
> >
> > Thank you
> > Yao, Jiewen
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> > > Hoffmann
> > > Sent: Monday, March 20, 2023 6:02 PM
> > > To: devel@edk2.groups.io
> > > Cc: Pawel Polawski <ppolawsk@redhat.com>; Wang, Jian J
> > > <jian.j.wang@intel.com>; Oliver Steffen <osteffen@redhat.com>; Xu,
> Min M
> > > <min.m.xu@intel.com>; Marvin Häuser <mhaeuser@posteo.de>; Yao,
> > > Jiewen <jiewen.yao@intel.com>; jmaloy@redhat.com
> > > Subject: Re: [edk2-devel] [PATCH v2 1/1]
> > > SecurityPkg/DxeImageVerificationLib: Check result of
> GetEfiGlobalVariable2
> > >
> > > On Fri, Mar 03, 2023 at 11:35:53AM +0100, Gerd Hoffmann wrote:
> > > > Call gRT->GetVariable() directly to read the SecureBoot variable.  It is
> > > > one byte in size so we can easily place it on the stack instead of
> > > > having GetEfiGlobalVariable2() allocate it for us, which avoids a few
> > > > possible error cases.
> > > >
> > > > Skip secure boot checks if (and only if):
> > > >
> > > >  (a) the SecureBoot variable is not present (EFI_NOT_FOUND) according
> to
> > > >      the return value, or
> > > >  (b) the SecureBoot variable was read successfully and is set to
> > > >      SECURE_BOOT_MODE_DISABLE.
> > > >
> > > > Previously the code skipped the secure boot checks on *any*
> > > > gRT->GetVariable() error (GetEfiGlobalVariable2 sets the variable
> > > > value to NULL in that case) and also on memory allocation failures.
> > > >
> > > > Fixes: CVE-2019-14560
> > > > Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=2167
> > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > >
> > > Ping.  Any comments on this patch?
> > >
> > > take care,
> > >   Gerd
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> > 
> >
> >
> 
> --