Re: [PATCH v2] SecurityPkg: Fix assert when setting key from eMMC/SD/USB

From: Roman Bacik <roman.bacik@broadcom.com>
To: "Yao, Jiewen" <jiewen.yao@intel.com>
Cc: "Zhang, Chao B" <chao.b.zhang@intel.com>,
	"rbacik@gmail.com" <rbacik@gmail.com>,
	 "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	Laszlo Ersek <lersek@redhat.com>,
	 Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
Subject: Re: [PATCH v2] SecurityPkg: Fix assert when setting key from eMMC/SD/USB
Date: Mon, 16 Jul 2018 21:30:51 -0700	[thread overview]
Message-ID: <CAGQAs7w56BBa1KcVw3ob+_KOrTE15ocBWdGd_66ZexV+70MtHg@mail.gmail.com> (raw)
In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503ACAC712@shsmsx102.ccr.corp.intel.com>

Yes, it is being taking care of.

On Mon, Jul 16, 2018 at 8:50 AM, Yao, Jiewen <jiewen.yao@intel.com> wrote:

> Laszlo already filed one - https://bugzilla.tianocore.
> org/show_bug.cgi?id=1008
>
> I suggest we add to UefiLib instead of fixing all individual driver.
>
> Thank you
> Yao Jiewen
>
>
> > -----Original Message-----
> > From: Zhang, Chao B
> > Sent: Monday, July 16, 2018 11:10 PM
> > To: rbacik@gmail.com; edk2-devel@lists.01.org
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Laszlo Ersek <lersek@redhat.com
> >;
> > Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> > Subject: RE: [PATCH v2] SecurityPkg: Fix assert when setting key from
> > eMMC/SD/USB
> >
> > Hi Bacik:
> >    Tks for the fix. Would you please file another report in Bugzilla for
> RamDisk
> > & Tls Configuration driver? They have same issue as SecureBootConfig
> driver
> >
> > -----Original Message-----
> > From: rbacik@gmail.com [mailto:rbacik@gmail.com]
> > Sent: Wednesday, July 11, 2018 6:51 AM
> > To: edk2-devel@lists.01.org
> > Cc: Zhang, Chao B <chao.b.zhang@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>; Laszlo Ersek <lersek@redhat.com>; Vladimir
> > Olovyannikov <vladimir.olovyannikov@broadcom.com>
> > Subject: [PATCH v2] SecurityPkg: Fix assert when setting key from
> > eMMC/SD/USB
> >
> > From: Roman Bacik <roman.bacik@broadcom.com>
> >
> > When secure boot is enabled, if one loads keys from a FAT formatted
> > eMMC/SD/USB when trying to provision PK/KEK/DB keys via the menu, an
> > assert in StrLen() occurs.
> > This is because the filename starts on odd address, which is not a uint16
> > aligned boundary: https://bugzilla.tianocore.org/show_bug.cgi?id=1003
> >
> > Cc: Chao Zhang <chao.b.zhang@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Roman Bacik <roman.bacik@broadcom.com>
> > ---
> >
> > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/
> SecureBootConfigFil
> > eExplorer.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> > FileExplorer.c
> > b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> > FileExplorer.c
> > index 1b6f88804275..19b13a5569a6 100644
> > ---
> > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> > FileExplorer.c
> > +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
> > +++ nfigFileExplorer.c
> > @@ -123,6 +123,8 @@ OpenFileByDevicePath(
> >    EFI_FILE_PROTOCOL               *Handle1;
> >    EFI_FILE_PROTOCOL               *Handle2;
> >    EFI_HANDLE                      DeviceHandle;
> > +  CHAR16                          *PathName;
> > +  UINTN                           PathLength;
> >
> >    if ((FilePath == NULL || FileHandle == NULL)) {
> >      return EFI_INVALID_PARAMETER;
> > @@ -173,6 +175,11 @@ OpenFileByDevicePath(
> >      //
> >      Handle2  = Handle1;
> >      Handle1 = NULL;
> > +    PathLength = DevicePathNodeLength(*FilePath) -
> > sizeof(EFI_DEVICE_PATH_PROTOCOL);
> > +    PathName = AllocateCopyPool(PathLength,
> > ((FILEPATH_DEVICE_PATH*)*FilePath)->PathName);
> > +    if (PathName == NULL) {
> > +      return EFI_OUT_OF_RESOURCES;
> > +    }
> >
> >      //
> >      // Try to test opening an existing file @@ -180,7 +187,7 @@
> > OpenFileByDevicePath(
> >      Status = Handle2->Open (
> >                            Handle2,
> >                            &Handle1,
> > -
> > ((FILEPATH_DEVICE_PATH*)*FilePath)->PathName,
> > +                          PathName,
> >                            OpenMode &~EFI_FILE_MODE_CREATE,
> >                            0
> >                           );
> > @@ -192,7 +199,7 @@ OpenFileByDevicePath(
> >        Status = Handle2->Open (
> >                              Handle2,
> >                              &Handle1,
> > -
> > ((FILEPATH_DEVICE_PATH*)*FilePath)->PathName,
> > +                            PathName,
> >                              OpenMode,
> >                              Attributes
> >                             );
> > @@ -202,6 +209,8 @@ OpenFileByDevicePath(
> >      //
> >      Handle2->Close (Handle2);
> >
> > +    FreePool (PathName);
> > +
> >      if (EFI_ERROR(Status)) {
> >        return (Status);
> >      }
> > --
> > 2.17.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>