Re: [PATCH v2 1/7] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()

["Ni, Ruiyu" <ruiyu.ni@Intel.com>]

On 8/3/2018 8:15 PM, Laszlo Ersek wrote:
> The EfiOpenFileByDevicePath() function centralizes functionality from
> 
> - MdeModulePkg/Universal/Disk/RamDiskDxe
> - NetworkPkg/TlsAuthConfigDxe
> - SecurityPkg/VariableAuthenticated/SecureBootConfigDxe
> - ShellPkg/Library/UefiShellLib
> 
> unifying the implementation and fixing various bugs.
> 
> (Ray suggested that we eliminate the special handling of
> EFI_FILE_MODE_CREATE in the "OpenMode" input parameter as well. We plan to
> implement that separately, under
> <https://bugzilla.tianocore.org/show_bug.cgi?id=1074>.)
> 
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Roman Bacik <roman.bacik@broadcom.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>      v2:
>      
>      - add the following sentence to the FilePath parameter's documentation:
>        "The caller is responsible for ensuring that the device path
>        pointed-to by FilePath is well-formed." [Jiewen]
>      
>      - lift the definition of the local variables that relate to the current
>        device path node from the loop to the top of the function
>        ("FilePathNode", "AlignedPathName", "PathName", "NextFile") [Ray]
>      
>      - report new TianoCore BZ
>        <https://bugzilla.tianocore.org/show_bug.cgi?id=1074> -- "don't
>        distinguish EFI_FILE_MODE_CREATE in EfiOpenFileByDevicePath() /
>        OpenMode" --, and reference it in the commit message, as future work
>        [Ray]
>      
>      - explain ASSERT (EFI_ERROR (Status)) -- as opposed to
>        ASSERT_EFI_ERROR (Status) -- with a code comment [Jaben, Ray]
>      
>      - pick up none of the received Reviewed-by tags (namely from Jaben,
>        Liming, and Ray) due to the *sum* of changes above
> 
>   MdePkg/Library/UefiLib/UefiLib.inf |   1 +
>   MdePkg/Include/Library/UefiLib.h   |  88 ++++++++
>   MdePkg/Library/UefiLib/UefiLib.c   | 227 ++++++++++++++++++++
>   3 files changed, 316 insertions(+)
> 
> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf b/MdePkg/Library/UefiLib/UefiLib.inf
> index f69f0a43b576..a6c739ef3d6d 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> @@ -68,6 +68,7 @@ [Protocols]
>     gEfiSimpleTextOutProtocolGuid                   ## SOMETIMES_CONSUMES
>     gEfiGraphicsOutputProtocolGuid                  ## SOMETIMES_CONSUMES
>     gEfiHiiFontProtocolGuid                         ## SOMETIMES_CONSUMES
> +  gEfiSimpleFileSystemProtocolGuid                ## SOMETIMES_CONSUMES
>     gEfiUgaDrawProtocolGuid | gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport                 ## SOMETIMES_CONSUMES # Consumes if gEfiGraphicsOutputProtocolGuid uninstalled
>     gEfiComponentNameProtocolGuid  | NOT gEfiMdePkgTokenSpaceGuid.PcdComponentNameDisable   ## SOMETIMES_PRODUCES # User chooses to produce it
>     gEfiComponentName2ProtocolGuid | NOT gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable  ## SOMETIMES_PRODUCES # User chooses to produce it
> diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
> index 7c6fde620c74..f950f1c9c648 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -33,6 +33,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>   #include <Protocol/DriverDiagnostics.h>
>   #include <Protocol/DriverDiagnostics2.h>
>   #include <Protocol/GraphicsOutput.h>
> +#include <Protocol/DevicePath.h>
> +#include <Protocol/SimpleFileSystem.h>
>   
>   #include <Library/BaseLib.h>
>   
> @@ -1520,4 +1522,90 @@ EfiLocateProtocolBuffer (
>     OUT UINTN     *NoProtocols,
>     OUT VOID      ***Buffer
>     );
> +
> +/**
> +  Open or create a file or directory, possibly creating the chain of
> +  directories leading up to the directory.
> +
> +  EfiOpenFileByDevicePath() first locates EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on
> +  FilePath, and opens the root directory of that filesystem with
> +  EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.OpenVolume().
> +
> +  On the remaining device path, the longest initial sequence of
> +  FILEPATH_DEVICE_PATH nodes is node-wise traversed with
> +  EFI_FILE_PROTOCOL.Open(). For the pathname fragment specified by each
> +  traversed FILEPATH_DEVICE_PATH node, EfiOpenFileByDevicePath() first masks
> +  EFI_FILE_MODE_CREATE out of OpenMode, and passes 0 for Attributes. If
> +  EFI_FILE_PROTOCOL.Open() fails, and OpenMode includes EFI_FILE_MODE_CREATE,
> +  then the operation is retried with the caller's OpenMode and Attributes
> +  unmodified.
> +
> +  (As a consequence, if OpenMode includes EFI_FILE_MODE_CREATE, and Attributes
> +  includes EFI_FILE_DIRECTORY, and each FILEPATH_DEVICE_PATH specifies a single
> +  pathname component, then EfiOpenFileByDevicePath() ensures that the specified
> +  series of subdirectories exist on return.)
> +
> +  The EFI_FILE_PROTOCOL identified by the last FILEPATH_DEVICE_PATH node is
> +  output to the caller; intermediate EFI_FILE_PROTOCOL instances are closed. If
> +  there are no FILEPATH_DEVICE_PATH nodes past the node that identifies the
> +  filesystem, then the EFI_FILE_PROTOCOL of the root directory of the
> +  filesystem is output to the caller. If a device path node that is different
> +  from FILEPATH_DEVICE_PATH is encountered relative to the filesystem, the
> +  traversal is stopped with an error, and a NULL EFI_FILE_PROTOCOL is output.
> +
> +  @param[in,out] FilePath  On input, the device path to the file or directory
> +                           to open or create. The caller is responsible for
> +                           ensuring that the device path pointed-to by FilePath
> +                           is well-formed. On output, FilePath points one past
> +                           the last node in the original device path that has
> +                           been successfully processed. FilePath is set on
> +                           output even if EfiOpenFileByDevicePath() returns an
> +                           error.
> +
> +  @param[out] File         On error, File is set to NULL. On success, File is
> +                           set to the EFI_FILE_PROTOCOL of the root directory
> +                           of the filesystem, if there are no
> +                           FILEPATH_DEVICE_PATH nodes in FilePath; otherwise,
> +                           File is set to the EFI_FILE_PROTOCOL identified by
> +                           the last node in FilePath.
> +
> +  @param[in] OpenMode      The OpenMode parameter to pass to
> +                           EFI_FILE_PROTOCOL.Open(). For each
> +                           FILEPATH_DEVICE_PATH node in FilePath,
> +                           EfiOpenFileByDevicePath() first opens the specified
> +                           pathname fragment with EFI_FILE_MODE_CREATE masked
> +                           out of OpenMode and with Attributes set to 0, and
> +                           only retries the operation with EFI_FILE_MODE_CREATE
> +                           unmasked and Attributes propagated if the first open
> +                           attempt fails.
> +
> +  @param[in] Attributes    The Attributes parameter to pass to
> +                           EFI_FILE_PROTOCOL.Open(), when EFI_FILE_MODE_CREATE
> +                           is propagated unmasked in OpenMode.
> +
> +  @retval EFI_SUCCESS            The file or directory has been opened or
> +                                 created.
> +
> +  @retval EFI_INVALID_PARAMETER  FilePath is NULL; or File is NULL; or FilePath
> +                                 contains a device path node, past the node
> +                                 that identifies
> +                                 EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, that is not a
> +                                 FILEPATH_DEVICE_PATH node.
> +
> +  @retval EFI_OUT_OF_RESOURCES   Memory allocation failed.
> +
> +  @return                        Error codes propagated from the
> +                                 LocateDevicePath() and OpenProtocol() boot
> +                                 services, and from the
> +                                 EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.OpenVolume()
> +                                 and EFI_FILE_PROTOCOL.Open() member functions.
> +**/
> +EFI_STATUS
> +EFIAPI
> +EfiOpenFileByDevicePath (
> +  IN OUT EFI_DEVICE_PATH_PROTOCOL  **FilePath,
> +  OUT    EFI_FILE_PROTOCOL         **File,
> +  IN     UINT64                    OpenMode,
> +  IN     UINT64                    Attributes
> +  );
>   #endif
> diff --git a/MdePkg/Library/UefiLib/UefiLib.c b/MdePkg/Library/UefiLib/UefiLib.c
> index 828a54ce7a97..7bcac5613768 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.c
> +++ b/MdePkg/Library/UefiLib/UefiLib.c
> @@ -1719,3 +1719,230 @@ EfiLocateProtocolBuffer (
>   
>     return EFI_SUCCESS;
>   }
> +
> +/**
> +  Open or create a file or directory, possibly creating the chain of
> +  directories leading up to the directory.
> +
> +  EfiOpenFileByDevicePath() first locates EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on
> +  FilePath, and opens the root directory of that filesystem with
> +  EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.OpenVolume().
> +
> +  On the remaining device path, the longest initial sequence of
> +  FILEPATH_DEVICE_PATH nodes is node-wise traversed with
> +  EFI_FILE_PROTOCOL.Open(). For the pathname fragment specified by each
> +  traversed FILEPATH_DEVICE_PATH node, EfiOpenFileByDevicePath() first masks
> +  EFI_FILE_MODE_CREATE out of OpenMode, and passes 0 for Attributes. If
> +  EFI_FILE_PROTOCOL.Open() fails, and OpenMode includes EFI_FILE_MODE_CREATE,
> +  then the operation is retried with the caller's OpenMode and Attributes
> +  unmodified.
> +
> +  (As a consequence, if OpenMode includes EFI_FILE_MODE_CREATE, and Attributes
> +  includes EFI_FILE_DIRECTORY, and each FILEPATH_DEVICE_PATH specifies a single
> +  pathname component, then EfiOpenFileByDevicePath() ensures that the specified
> +  series of subdirectories exist on return.)
> +
> +  The EFI_FILE_PROTOCOL identified by the last FILEPATH_DEVICE_PATH node is
> +  output to the caller; intermediate EFI_FILE_PROTOCOL instances are closed. If
> +  there are no FILEPATH_DEVICE_PATH nodes past the node that identifies the
> +  filesystem, then the EFI_FILE_PROTOCOL of the root directory of the
> +  filesystem is output to the caller. If a device path node that is different
> +  from FILEPATH_DEVICE_PATH is encountered relative to the filesystem, the
> +  traversal is stopped with an error, and a NULL EFI_FILE_PROTOCOL is output.
> +
> +  @param[in,out] FilePath  On input, the device path to the file or directory
> +                           to open or create. The caller is responsible for
> +                           ensuring that the device path pointed-to by FilePath
> +                           is well-formed. On output, FilePath points one past
> +                           the last node in the original device path that has
> +                           been successfully processed. FilePath is set on
> +                           output even if EfiOpenFileByDevicePath() returns an
> +                           error.
> +
> +  @param[out] File         On error, File is set to NULL. On success, File is
> +                           set to the EFI_FILE_PROTOCOL of the root directory
> +                           of the filesystem, if there are no
> +                           FILEPATH_DEVICE_PATH nodes in FilePath; otherwise,
> +                           File is set to the EFI_FILE_PROTOCOL identified by
> +                           the last node in FilePath.
> +
> +  @param[in] OpenMode      The OpenMode parameter to pass to
> +                           EFI_FILE_PROTOCOL.Open(). For each
> +                           FILEPATH_DEVICE_PATH node in FilePath,
> +                           EfiOpenFileByDevicePath() first opens the specified
> +                           pathname fragment with EFI_FILE_MODE_CREATE masked
> +                           out of OpenMode and with Attributes set to 0, and
> +                           only retries the operation with EFI_FILE_MODE_CREATE
> +                           unmasked and Attributes propagated if the first open
> +                           attempt fails.
> +
> +  @param[in] Attributes    The Attributes parameter to pass to
> +                           EFI_FILE_PROTOCOL.Open(), when EFI_FILE_MODE_CREATE
> +                           is propagated unmasked in OpenMode.
> +
> +  @retval EFI_SUCCESS            The file or directory has been opened or
> +                                 created.
> +
> +  @retval EFI_INVALID_PARAMETER  FilePath is NULL; or File is NULL; or FilePath
> +                                 contains a device path node, past the node
> +                                 that identifies
> +                                 EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, that is not a
> +                                 FILEPATH_DEVICE_PATH node.
> +
> +  @retval EFI_OUT_OF_RESOURCES   Memory allocation failed.
> +
> +  @return                        Error codes propagated from the
> +                                 LocateDevicePath() and OpenProtocol() boot
> +                                 services, and from the
> +                                 EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.OpenVolume()
> +                                 and EFI_FILE_PROTOCOL.Open() member functions.
> +**/
> +EFI_STATUS
> +EFIAPI
> +EfiOpenFileByDevicePath (
> +  IN OUT EFI_DEVICE_PATH_PROTOCOL  **FilePath,
> +  OUT    EFI_FILE_PROTOCOL         **File,
> +  IN     UINT64                    OpenMode,
> +  IN     UINT64                    Attributes
> +  )
> +{
> +  EFI_STATUS                      Status;
> +  EFI_HANDLE                      FileSystemHandle;
> +  EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *FileSystem;
> +  EFI_FILE_PROTOCOL               *LastFile;
> +  FILEPATH_DEVICE_PATH            *FilePathNode;
> +  CHAR16                          *AlignedPathName;
> +  CHAR16                          *PathName;
> +  EFI_FILE_PROTOCOL               *NextFile;
> +
> +  if (File == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  *File = NULL;
> +
> +  if (FilePath == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // Look up the filesystem.
> +  //
> +  Status = gBS->LocateDevicePath (
> +                  &gEfiSimpleFileSystemProtocolGuid,
> +                  FilePath,
> +                  &FileSystemHandle
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +  Status = gBS->OpenProtocol (
> +                  FileSystemHandle,
> +                  &gEfiSimpleFileSystemProtocolGuid,
> +                  (VOID **)&FileSystem,
> +                  gImageHandle,
> +                  NULL,
> +                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  //
> +  // Open the root directory of the filesystem. After this operation succeeds,
> +  // we have to release LastFile on error.
> +  //
> +  Status = FileSystem->OpenVolume (FileSystem, &LastFile);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  //
> +  // Traverse the device path nodes relative to the filesystem.
> +  //
> +  while (!IsDevicePathEnd (*FilePath)) {
> +    if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
> +        DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP) {
> +      Status = EFI_INVALID_PARAMETER;
> +      goto CloseLastFile;
> +    }
> +    FilePathNode = (FILEPATH_DEVICE_PATH *)*FilePath;
> +
> +    //
> +    // FilePathNode->PathName may be unaligned, and the UEFI specification
> +    // requires pointers that are passed to protocol member functions to be
> +    // aligned. Create an aligned copy of the pathname if necessary.
> +    //
> +    if ((UINTN)FilePathNode->PathName % sizeof *FilePathNode->PathName == 0) {
> +      AlignedPathName = NULL;
> +      PathName = FilePathNode->PathName;
> +    } else {
> +      AlignedPathName = AllocateCopyPool (
> +                          (DevicePathNodeLength (FilePathNode) -
> +                           SIZE_OF_FILEPATH_DEVICE_PATH),
> +                          FilePathNode->PathName
> +                          );
> +      if (AlignedPathName == NULL) {
> +        Status = EFI_OUT_OF_RESOURCES;
> +        goto CloseLastFile;
> +      }
> +      PathName = AlignedPathName;
> +    }
> +
> +    //
> +    // Open the next pathname fragment with EFI_FILE_MODE_CREATE masked out and
> +    // with Attributes set to 0.
> +    //
> +    Status = LastFile->Open (
> +                         LastFile,
> +                         &NextFile,
> +                         PathName,
> +                         OpenMode & ~(UINT64)EFI_FILE_MODE_CREATE,
> +                         0
> +                         );
> +
> +    //
> +    // Retry with EFI_FILE_MODE_CREATE and the original Attributes if the first
> +    // attempt failed, and the caller specified EFI_FILE_MODE_CREATE.
> +    //
> +    if (EFI_ERROR (Status) && (OpenMode & EFI_FILE_MODE_CREATE) != 0) {
> +      Status = LastFile->Open (
> +                           LastFile,
> +                           &NextFile,
> +                           PathName,
> +                           OpenMode,
> +                           Attributes
> +                           );
> +    }
> +
> +    //
> +    // Release any AlignedPathName on both error and success paths; PathName is
> +    // no longer needed.
> +    //
> +    if (AlignedPathName != NULL) {
> +      FreePool (AlignedPathName);
> +    }
> +    if (EFI_ERROR (Status)) {
> +      goto CloseLastFile;
> +    }
> +
> +    //
> +    // Advance to the next device path node.
> +    //
> +    LastFile->Close (LastFile);
> +    LastFile = NextFile;
> +    *FilePath = NextDevicePathNode (FilePathNode);
> +  }
> +
> +  *File = LastFile;
> +  return EFI_SUCCESS;
> +
> +CloseLastFile:
> +  LastFile->Close (LastFile);
> +
> +  //
> +  // We are on the error path; we must have set an error Status for returning
> +  // to the caller.
> +  //
> +  ASSERT (EFI_ERROR (Status));
> +  return Status;
> +}
> 
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

-- 
Thanks,
Ray