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

[Roman Bacik <roman.bacik@broadcom.com>]

Signed-off-by: Roman Bacik <roman.bacik@broadcom.com>

On Tue, Jul 10, 2018 at 3:51 PM, <rbacik@gmail.com> wrote:

> 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/SecureBootConfigFileExplorer.c
> | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/
> SecureBootConfigFileExplorer.c
> index 1b6f88804275..19b13a5569a6 100644
> --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/
> SecureBootConfigFileExplorer.c
> +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/
> SecureBootConfigFileExplorer.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
>