* [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs
@ 2018-08-03 12:15 Laszlo Ersek
2018-08-03 12:15 ` [PATCH v2 1/7] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath() Laszlo Ersek
` (8 more replies)
0 siblings, 9 replies; 17+ messages in thread
From: Laszlo Ersek @ 2018-08-03 12:15 UTC (permalink / raw)
To: edk2-devel-01
Cc: Chao Zhang, Eric Dong, Jaben Carsey, Jiaxin Wu, Jiewen Yao,
Liming Gao, Michael D Kinney, Roman Bacik, Ruiyu Ni, Siyuan Fu,
Star Zeng
Repo: https://github.com/lersek/edk2.git
Branch: open_file_by_devpath_tiano_1008_v2
This is version 2 of the patch set that was originally posted at:
https://lists.01.org/pipermail/edk2-devel/2018-July/027253.html
for <https://bugzilla.tianocore.org/show_bug.cgi?id=1008>.
Changes are noted on every patch.
The cumulative code difference is very small (not counting the
FrameworkUefiLib copy of the function), so I'm including it here for
easier review:
> diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
> index 2468bf2aee80..f950f1c9c648 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -1554,9 +1554,11 @@ EfiLocateProtocolBuffer (
> 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. On output, FilePath points one
> - past the last node in the original device path that
> - has been successfully processed. FilePath is set on
> + 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.
>
> diff --git a/MdePkg/Library/UefiLib/UefiLib.c b/MdePkg/Library/UefiLib/UefiLib.c
> index d3e290178cd9..7bcac5613768 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.c
> +++ b/MdePkg/Library/UefiLib/UefiLib.c
> @@ -1751,9 +1751,11 @@ EfiLocateProtocolBuffer (
> 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. On output, FilePath points one
> - past the last node in the original device path that
> - has been successfully processed. FilePath is set on
> + 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.
>
> @@ -1808,6 +1810,10 @@ EfiOpenFileByDevicePath (
> 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;
> @@ -1854,15 +1860,6 @@ EfiOpenFileByDevicePath (
> // Traverse the device path nodes relative to the filesystem.
> //
> while (!IsDevicePathEnd (*FilePath)) {
> - //
> - // Keep local variables that relate to the current device path node tightly
> - // scoped.
> - //
> - FILEPATH_DEVICE_PATH *FilePathNode;
> - CHAR16 *AlignedPathName;
> - CHAR16 *PathName;
> - EFI_FILE_PROTOCOL *NextFile;
> -
> if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
> DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP) {
> Status = EFI_INVALID_PARAMETER;
> @@ -1942,6 +1939,10 @@ EfiOpenFileByDevicePath (
> 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;
> }
> diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
> index 312a92d7461a..aef85c470143 100644
> --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
> +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
> @@ -163,7 +163,7 @@ UpdatePage(
>
> gSecureBootPrivateData->FileContext->FileName = FileName;
>
> - EfiOpenFileByDevicePath(
> + EfiOpenFileByDevicePath (
> &FilePath,
> &gSecureBootPrivateData->FileContext->FHandle,
> EFI_FILE_MODE_READ,
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>
Thanks,
Laszlo
Laszlo Ersek (7):
MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
IntelFrameworkPkg/FrameworkUefiLib: introduce
EfiOpenFileByDevicePath()
MdeModulePkg/RamDiskDxe: replace OpenFileByDevicePath() with UefiLib
API
NetworkPkg/TlsAuthConfigDxe: replace OpenFileByDevicePath() with
UefiLib API
SecurityPkg/SecureBootConfigDxe: replace OpenFileByDevicePath() with
UefiLib API
ShellPkg/UefiShellLib: drop DeviceHandle param of
ShellOpenFileByDevicePath()
ShellPkg/UefiShellLib: rebase ShellOpenFileByDevicePath() to UefiLib
API
IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf | 1 +
IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c | 227 ++++++++++++++++++++
MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf | 1 -
MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c | 140 ------------
MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c | 2 +-
MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h | 39 ----
MdePkg/Include/Library/UefiLib.h | 88 ++++++++
MdePkg/Library/UefiLib/UefiLib.c | 227 ++++++++++++++++++++
MdePkg/Library/UefiLib/UefiLib.inf | 1 +
NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf | 1 -
NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c | 141 +-----------
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf | 1 -
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c | 151 +------------
ShellPkg/Include/Library/ShellLib.h | 2 -
ShellPkg/Library/UefiShellLib/UefiShellLib.c | 118 +---------
ShellPkg/Library/UefiShellLib/UefiShellLib.inf | 3 +-
16 files changed, 552 insertions(+), 591 deletions(-)
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/7] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
2018-08-03 12:15 [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs Laszlo Ersek
@ 2018-08-03 12:15 ` Laszlo Ersek
2018-08-06 2:03 ` Ni, Ruiyu
2018-08-03 12:15 ` [PATCH v2 2/7] IntelFrameworkPkg/FrameworkUefiLib: " Laszlo Ersek
` (7 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Laszlo Ersek @ 2018-08-03 12:15 UTC (permalink / raw)
To: edk2-devel-01
Cc: Chao Zhang, Eric Dong, Jaben Carsey, Jiaxin Wu, Jiewen Yao,
Liming Gao, Michael D Kinney, Roman Bacik, Ruiyu Ni, Siyuan Fu,
Star Zeng
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;
+}
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/7] IntelFrameworkPkg/FrameworkUefiLib: introduce EfiOpenFileByDevicePath()
2018-08-03 12:15 [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs Laszlo Ersek
2018-08-03 12:15 ` [PATCH v2 1/7] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath() Laszlo Ersek
@ 2018-08-03 12:15 ` Laszlo Ersek
2018-08-03 12:15 ` [PATCH v2 3/7] MdeModulePkg/RamDiskDxe: replace OpenFileByDevicePath() with UefiLib API Laszlo Ersek
` (6 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Laszlo Ersek @ 2018-08-03 12:15 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Liming Gao, Michael D Kinney
Copy the EfiOpenFileByDevicePath() implementation from the previous
(MdePkg/UefiLib) patch to FrameworkUefiLib.
(Note that the FrameworkUefiLib instance too will be updated for
<https://bugzilla.tianocore.org/show_bug.cgi?id=1074>.)
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@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:
- new patch: with the MdePkg instance finalized, include the
implementation in IntelFrameworkPkg too [Liming, Laszlo]
- build-tested with:
build -a IA32 -a X64 -b NOOPT -t GCC48 \
-p IntelFrameworkPkg/IntelFrameworkPkg.dsc \
-m IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf | 1 +
IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c | 227 ++++++++++++++++++++
2 files changed, 228 insertions(+)
diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
index 8cff19fa3124..182d20fca051 100644
--- a/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
+++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/FrameworkUefiLib.inf
@@ -61,6 +61,7 @@ [Protocols]
gEfiSimpleTextOutProtocolGuid ## SOMETIMES_CONSUMES
gEfiGraphicsOutputProtocolGuid ## SOMETIMES_CONSUMES
gEfiHiiFontProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiSimpleFileSystemProtocolGuid ## SOMETIMES_CONSUMES
gEfiComponentNameProtocolGuid ## SOMETIMES_PRODUCES
gEfiComponentName2ProtocolGuid ## SOMETIMES_PRODUCES
gEfiDriverConfigurationProtocolGuid ## SOMETIMES_PRODUCES
diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c b/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
index 0aa4506b8f4d..b283d775b470 100644
--- a/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
+++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
@@ -1691,3 +1691,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;
+}
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 3/7] MdeModulePkg/RamDiskDxe: replace OpenFileByDevicePath() with UefiLib API
2018-08-03 12:15 [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs Laszlo Ersek
2018-08-03 12:15 ` [PATCH v2 1/7] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath() Laszlo Ersek
2018-08-03 12:15 ` [PATCH v2 2/7] IntelFrameworkPkg/FrameworkUefiLib: " Laszlo Ersek
@ 2018-08-03 12:15 ` Laszlo Ersek
2018-08-03 12:15 ` [PATCH v2 4/7] NetworkPkg/TlsAuthConfigDxe: " Laszlo Ersek
` (5 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Laszlo Ersek @ 2018-08-03 12:15 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Eric Dong, Jiaxin Wu, Ruiyu Ni, Siyuan Fu, Star Zeng
Replace the OpenFileByDevicePath() function with EfiOpenFileByDevicePath()
from UefiLib, correcting the following issues:
- imprecise comments on OpenFileByDevicePath(),
- code duplication between this module and other modules,
- local variable name "EfiSimpleFileSystemProtocol" starting with "Efi"
prefix,
- bogus "FileHandle = NULL" assignments,
- passing a potentially unaligned "FILEPATH_DEVICE_PATH.PathName" field to
a protocol member function (forbidden by the UEFI spec),
- leaking "Handle1" when the device path type/subtype check fails in the
loop,
- stale SHELL_FILE_HANDLE reference in a comment.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.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>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
---
Notes:
v2:
- pick up Star's and Jaben's R-b's
MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf | 1 -
MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h | 39 ------
MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c | 140 --------------------
MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c | 2 +-
4 files changed, 1 insertion(+), 181 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
index cdd2da690411..da00e4a420e7 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
@@ -75,7 +75,6 @@ [Protocols]
gEfiDevicePathProtocolGuid ## PRODUCES
gEfiBlockIoProtocolGuid ## PRODUCES
gEfiBlockIo2ProtocolGuid ## PRODUCES
- gEfiSimpleFileSystemProtocolGuid ## SOMETIMES_CONSUMES
gEfiAcpiTableProtocolGuid ## SOMETIMES_CONSUMES
gEfiAcpiSdtProtocolGuid ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
index 077bb77b25bf..08a8ca94c9db 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
@@ -605,45 +605,6 @@ FileInfo (
);
-/**
- This function will open a file or directory referenced by DevicePath.
-
- This function opens a file with the open mode according to the file path. The
- Attributes is valid only for EFI_FILE_MODE_CREATE.
-
- @param[in, out] FilePath On input, the device path to the file.
- On output, the remaining device path.
- @param[out] FileHandle Pointer to the file handle.
- @param[in] OpenMode The mode to open the file with.
- @param[in] Attributes The file's file attributes.
-
- @retval EFI_SUCCESS The information was set.
- @retval EFI_INVALID_PARAMETER One of the parameters has an invalid value.
- @retval EFI_UNSUPPORTED Could not open the file path.
- @retval EFI_NOT_FOUND The specified file could not be found on the
- device or the file system could not be found
- on the device.
- @retval EFI_NO_MEDIA The device has no medium.
- @retval EFI_MEDIA_CHANGED The device has a different medium in it or
- the medium is no longer supported.
- @retval EFI_DEVICE_ERROR The device reported an error.
- @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
- @retval EFI_WRITE_PROTECTED The file or medium is write protected.
- @retval EFI_ACCESS_DENIED The file was opened read only.
- @retval EFI_OUT_OF_RESOURCES Not enough resources were available to open
- the file.
- @retval EFI_VOLUME_FULL The volume is full.
-**/
-EFI_STATUS
-EFIAPI
-OpenFileByDevicePath(
- IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
- OUT EFI_FILE_HANDLE *FileHandle,
- IN UINT64 OpenMode,
- IN UINT64 Attributes
- );
-
-
/**
Publish the RAM disk NVDIMM Firmware Interface Table (NFIT) to the ACPI
table.
diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c
index 2cfd4bbf6ce8..95d676fc3939 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c
@@ -111,143 +111,3 @@ FileInfo (
return Buffer;
}
-
-
-/**
- This function will open a file or directory referenced by DevicePath.
-
- This function opens a file with the open mode according to the file path. The
- Attributes is valid only for EFI_FILE_MODE_CREATE.
-
- @param[in, out] FilePath On input, the device path to the file.
- On output, the remaining device path.
- @param[out] FileHandle Pointer to the file handle.
- @param[in] OpenMode The mode to open the file with.
- @param[in] Attributes The file's file attributes.
-
- @retval EFI_SUCCESS The information was set.
- @retval EFI_INVALID_PARAMETER One of the parameters has an invalid value.
- @retval EFI_UNSUPPORTED Could not open the file path.
- @retval EFI_NOT_FOUND The specified file could not be found on the
- device or the file system could not be found
- on the device.
- @retval EFI_NO_MEDIA The device has no medium.
- @retval EFI_MEDIA_CHANGED The device has a different medium in it or
- the medium is no longer supported.
- @retval EFI_DEVICE_ERROR The device reported an error.
- @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
- @retval EFI_WRITE_PROTECTED The file or medium is write protected.
- @retval EFI_ACCESS_DENIED The file was opened read only.
- @retval EFI_OUT_OF_RESOURCES Not enough resources were available to open
- the file.
- @retval EFI_VOLUME_FULL The volume is full.
-**/
-EFI_STATUS
-EFIAPI
-OpenFileByDevicePath(
- IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
- OUT EFI_FILE_HANDLE *FileHandle,
- IN UINT64 OpenMode,
- IN UINT64 Attributes
- )
-{
- EFI_STATUS Status;
- EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *EfiSimpleFileSystemProtocol;
- EFI_FILE_PROTOCOL *Handle1;
- EFI_FILE_PROTOCOL *Handle2;
- EFI_HANDLE DeviceHandle;
-
- if ((FilePath == NULL || FileHandle == NULL)) {
- return EFI_INVALID_PARAMETER;
- }
-
- Status = gBS->LocateDevicePath (
- &gEfiSimpleFileSystemProtocolGuid,
- FilePath,
- &DeviceHandle
- );
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- Status = gBS->OpenProtocol(
- DeviceHandle,
- &gEfiSimpleFileSystemProtocolGuid,
- (VOID**)&EfiSimpleFileSystemProtocol,
- gImageHandle,
- NULL,
- EFI_OPEN_PROTOCOL_GET_PROTOCOL
- );
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- Status = EfiSimpleFileSystemProtocol->OpenVolume(EfiSimpleFileSystemProtocol, &Handle1);
- if (EFI_ERROR (Status)) {
- FileHandle = NULL;
- return Status;
- }
-
- //
- // go down directories one node at a time.
- //
- while (!IsDevicePathEnd (*FilePath)) {
- //
- // For file system access each node should be a file path component
- //
- if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
- DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP
- ) {
- FileHandle = NULL;
- return (EFI_INVALID_PARAMETER);
- }
- //
- // Open this file path node
- //
- Handle2 = Handle1;
- Handle1 = NULL;
-
- //
- // Try to test opening an existing file
- //
- Status = Handle2->Open (
- Handle2,
- &Handle1,
- ((FILEPATH_DEVICE_PATH*)*FilePath)->PathName,
- OpenMode &~EFI_FILE_MODE_CREATE,
- 0
- );
-
- //
- // see if the error was that it needs to be created
- //
- if ((EFI_ERROR (Status)) && (OpenMode != (OpenMode &~EFI_FILE_MODE_CREATE))) {
- Status = Handle2->Open (
- Handle2,
- &Handle1,
- ((FILEPATH_DEVICE_PATH*)*FilePath)->PathName,
- OpenMode,
- Attributes
- );
- }
- //
- // Close the last node
- //
- Handle2->Close (Handle2);
-
- if (EFI_ERROR(Status)) {
- return (Status);
- }
-
- //
- // Get the next node
- //
- *FilePath = NextDevicePathNode (*FilePath);
- }
-
- //
- // This is a weak spot since if the undefined SHELL_FILE_HANDLE format changes this must change also!
- //
- *FileHandle = (VOID*)Handle1;
- return EFI_SUCCESS;
-}
diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c
index 7ebd397fe68a..62933a2d6201 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c
@@ -656,7 +656,7 @@ RamDiskCallback (
//
// Open the file.
//
- Status = OpenFileByDevicePath (
+ Status = EfiOpenFileByDevicePath (
&FileDevPath,
&FileHandle,
EFI_FILE_MODE_READ,
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 4/7] NetworkPkg/TlsAuthConfigDxe: replace OpenFileByDevicePath() with UefiLib API
2018-08-03 12:15 [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs Laszlo Ersek
` (2 preceding siblings ...)
2018-08-03 12:15 ` [PATCH v2 3/7] MdeModulePkg/RamDiskDxe: replace OpenFileByDevicePath() with UefiLib API Laszlo Ersek
@ 2018-08-03 12:15 ` Laszlo Ersek
2018-08-03 12:15 ` [PATCH v2 5/7] SecurityPkg/SecureBootConfigDxe: " Laszlo Ersek
` (4 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Laszlo Ersek @ 2018-08-03 12:15 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Jiaxin Wu, Siyuan Fu
Replace the OpenFileByDevicePath() function with EfiOpenFileByDevicePath()
from UefiLib, correcting the following issues:
- imprecise comments on OpenFileByDevicePath(),
- code duplication between this module and other modules,
- local variable name "EfiSimpleFileSystemProtocol" starting with "Efi"
prefix,
- bogus "FileHandle = NULL" assignments,
- passing a potentially unaligned "FILEPATH_DEVICE_PATH.PathName" field to
a protocol member function (forbidden by the UEFI spec),
- leaking "Handle1" when the device path type/subtype check fails in the
loop,
- stale SHELL_FILE_HANDLE reference in a comment.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@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>
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
---
Notes:
v2:
- pick up Jiaxin's and Jaben's R-b's
NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf | 1 -
NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c | 141 +-------------------
2 files changed, 1 insertion(+), 141 deletions(-)
diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
index 3cfcdb9983f1..e5face7b4de5 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
@@ -57,7 +57,6 @@ [LibraryClasses]
[Protocols]
gEfiDevicePathProtocolGuid ## PRODUCES
gEfiHiiConfigAccessProtocolGuid ## PRODUCES
- gEfiSimpleFileSystemProtocolGuid ## SOMETIMES_CONSUMES
[Guids]
gTlsAuthConfigGuid ## PRODUCES ## GUID
diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
index 31450b3e4a1b..7259c5e82f61 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
@@ -574,145 +574,6 @@ ON_EXIT:
return Status;
}
-/**
- This function will open a file or directory referenced by DevicePath.
-
- This function opens a file with the open mode according to the file path. The
- Attributes is valid only for EFI_FILE_MODE_CREATE.
-
- @param[in, out] FilePath On input, the device path to the file.
- On output, the remaining device path.
- @param[out] FileHandle Pointer to the file handle.
- @param[in] OpenMode The mode to open the file with.
- @param[in] Attributes The file's file attributes.
-
- @retval EFI_SUCCESS The information was set.
- @retval EFI_INVALID_PARAMETER One of the parameters has an invalid value.
- @retval EFI_UNSUPPORTED Could not open the file path.
- @retval EFI_NOT_FOUND The specified file could not be found on the
- device or the file system could not be found on
- the device.
- @retval EFI_NO_MEDIA The device has no medium.
- @retval EFI_MEDIA_CHANGED The device has a different medium in it or the
- medium is no longer supported.
- @retval EFI_DEVICE_ERROR The device reported an error.
- @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
- @retval EFI_WRITE_PROTECTED The file or medium is write protected.
- @retval EFI_ACCESS_DENIED The file was opened read only.
- @retval EFI_OUT_OF_RESOURCES Not enough resources were available to open the
- file.
- @retval EFI_VOLUME_FULL The volume is full.
-**/
-EFI_STATUS
-EFIAPI
-OpenFileByDevicePath (
- IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
- OUT EFI_FILE_HANDLE *FileHandle,
- IN UINT64 OpenMode,
- IN UINT64 Attributes
- )
-{
- EFI_STATUS Status;
- EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *EfiSimpleFileSystemProtocol;
- EFI_FILE_PROTOCOL *Handle1;
- EFI_FILE_PROTOCOL *Handle2;
- EFI_HANDLE DeviceHandle;
-
- if ((FilePath == NULL || FileHandle == NULL)) {
- return EFI_INVALID_PARAMETER;
- }
-
- Status = gBS->LocateDevicePath (
- &gEfiSimpleFileSystemProtocolGuid,
- FilePath,
- &DeviceHandle
- );
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- Status = gBS->OpenProtocol(
- DeviceHandle,
- &gEfiSimpleFileSystemProtocolGuid,
- (VOID**)&EfiSimpleFileSystemProtocol,
- gImageHandle,
- NULL,
- EFI_OPEN_PROTOCOL_GET_PROTOCOL
- );
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- Status = EfiSimpleFileSystemProtocol->OpenVolume(EfiSimpleFileSystemProtocol, &Handle1);
- if (EFI_ERROR (Status)) {
- FileHandle = NULL;
- return Status;
- }
-
- //
- // go down directories one node at a time.
- //
- while (!IsDevicePathEnd (*FilePath)) {
- //
- // For file system access each node should be a file path component
- //
- if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
- DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP
- ) {
- FileHandle = NULL;
- return (EFI_INVALID_PARAMETER);
- }
- //
- // Open this file path node
- //
- Handle2 = Handle1;
- Handle1 = NULL;
-
- //
- // Try to test opening an existing file
- //
- Status = Handle2->Open (
- Handle2,
- &Handle1,
- ((FILEPATH_DEVICE_PATH*)*FilePath)->PathName,
- OpenMode &~EFI_FILE_MODE_CREATE,
- 0
- );
-
- //
- // see if the error was that it needs to be created
- //
- if ((EFI_ERROR (Status)) && (OpenMode != (OpenMode &~EFI_FILE_MODE_CREATE))) {
- Status = Handle2->Open (
- Handle2,
- &Handle1,
- ((FILEPATH_DEVICE_PATH*)*FilePath)->PathName,
- OpenMode,
- Attributes
- );
- }
- //
- // Close the last node
- //
- Handle2->Close (Handle2);
-
- if (EFI_ERROR(Status)) {
- return (Status);
- }
-
- //
- // Get the next node
- //
- *FilePath = NextDevicePathNode (*FilePath);
- }
-
- //
- // This is a weak spot since if the undefined SHELL_FILE_HANDLE format changes this must change also!
- //
- *FileHandle = (VOID*)Handle1;
- return EFI_SUCCESS;
-}
-
/**
This function converts an input device structure to a Unicode string.
@@ -1039,7 +900,7 @@ UpdatePage(
mTlsAuthPrivateData->FileContext->FileName = FileName;
- OpenFileByDevicePath (
+ EfiOpenFileByDevicePath (
&FilePath,
&mTlsAuthPrivateData->FileContext->FHandle,
EFI_FILE_MODE_READ,
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 5/7] SecurityPkg/SecureBootConfigDxe: replace OpenFileByDevicePath() with UefiLib API
2018-08-03 12:15 [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs Laszlo Ersek
` (3 preceding siblings ...)
2018-08-03 12:15 ` [PATCH v2 4/7] NetworkPkg/TlsAuthConfigDxe: " Laszlo Ersek
@ 2018-08-03 12:15 ` Laszlo Ersek
2018-08-07 12:16 ` Zhang, Chao B
2018-08-03 12:15 ` [PATCH v2 6/7] ShellPkg/UefiShellLib: drop DeviceHandle param of ShellOpenFileByDevicePath() Laszlo Ersek
` (3 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Laszlo Ersek @ 2018-08-03 12:15 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Chao Zhang, Jiewen Yao, Roman Bacik
Replace the OpenFileByDevicePath() function with EfiOpenFileByDevicePath()
from UefiLib, correcting the following issues:
- imprecise comments on OpenFileByDevicePath(),
- code duplication between this module and other modules,
- local variable name "EfiSimpleFileSystemProtocol" starting with "Efi"
prefix,
- bogus "FileHandle = NULL" assignments,
- leaking "Handle1" when the device path type/subtype check or the
realignment-motivated AllocateCopyPool() fails in the loop,
- stale SHELL_FILE_HANDLE reference in a comment.
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Roman Bacik <roman.bacik@broadcom.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>
Reviewed-by: Chao Zhang <chao.b.zhang@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
---
Notes:
v2:
- pick up Chao's and Jaben's R-b's
- insert a space character between "EfiOpenFileByDevicePath" and "(" --
it was missing from the pre-patch code too
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf | 1 -
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c | 151 +-------------------
2 files changed, 1 insertion(+), 151 deletions(-)
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
index 487fc8cda917..caf95ddac7d9 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
@@ -114,7 +114,6 @@ [Guids]
[Protocols]
gEfiHiiConfigAccessProtocolGuid ## PRODUCES
gEfiDevicePathProtocolGuid ## PRODUCES
- gEfiSimpleFileSystemProtocolGuid ## SOMETIMES_CONSUMES
gEfiBlockIoProtocolGuid ## SOMETIMES_CONSUMES
[Depex]
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
index 2a26c20f394c..aef85c470143 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
@@ -80,155 +80,6 @@ CleanUpPage (
);
}
-/**
- This function will open a file or directory referenced by DevicePath.
-
- This function opens a file with the open mode according to the file path. The
- Attributes is valid only for EFI_FILE_MODE_CREATE.
-
- @param[in, out] FilePath On input, the device path to the file.
- On output, the remaining device path.
- @param[out] FileHandle Pointer to the file handle.
- @param[in] OpenMode The mode to open the file with.
- @param[in] Attributes The file's file attributes.
-
- @retval EFI_SUCCESS The information was set.
- @retval EFI_INVALID_PARAMETER One of the parameters has an invalid value.
- @retval EFI_UNSUPPORTED Could not open the file path.
- @retval EFI_NOT_FOUND The specified file could not be found on the
- device or the file system could not be found on
- the device.
- @retval EFI_NO_MEDIA The device has no medium.
- @retval EFI_MEDIA_CHANGED The device has a different medium in it or the
- medium is no longer supported.
- @retval EFI_DEVICE_ERROR The device reported an error.
- @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
- @retval EFI_WRITE_PROTECTED The file or medium is write protected.
- @retval EFI_ACCESS_DENIED The file was opened read only.
- @retval EFI_OUT_OF_RESOURCES Not enough resources were available to open the
- file.
- @retval EFI_VOLUME_FULL The volume is full.
-**/
-EFI_STATUS
-EFIAPI
-OpenFileByDevicePath(
- IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
- OUT EFI_FILE_HANDLE *FileHandle,
- IN UINT64 OpenMode,
- IN UINT64 Attributes
- )
-{
- EFI_STATUS Status;
- EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *EfiSimpleFileSystemProtocol;
- EFI_FILE_PROTOCOL *Handle1;
- EFI_FILE_PROTOCOL *Handle2;
- EFI_HANDLE DeviceHandle;
- CHAR16 *PathName;
- UINTN PathLength;
-
- if ((FilePath == NULL || FileHandle == NULL)) {
- return EFI_INVALID_PARAMETER;
- }
-
- Status = gBS->LocateDevicePath (
- &gEfiSimpleFileSystemProtocolGuid,
- FilePath,
- &DeviceHandle
- );
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- Status = gBS->OpenProtocol(
- DeviceHandle,
- &gEfiSimpleFileSystemProtocolGuid,
- (VOID**)&EfiSimpleFileSystemProtocol,
- gImageHandle,
- NULL,
- EFI_OPEN_PROTOCOL_GET_PROTOCOL
- );
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- Status = EfiSimpleFileSystemProtocol->OpenVolume(EfiSimpleFileSystemProtocol, &Handle1);
- if (EFI_ERROR (Status)) {
- FileHandle = NULL;
- return Status;
- }
-
- //
- // go down directories one node at a time.
- //
- while (!IsDevicePathEnd (*FilePath)) {
- //
- // For file system access each node should be a file path component
- //
- if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
- DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP
- ) {
- FileHandle = NULL;
- return (EFI_INVALID_PARAMETER);
- }
- //
- // Open this file path node
- //
- 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
- //
- Status = Handle2->Open (
- Handle2,
- &Handle1,
- PathName,
- OpenMode &~EFI_FILE_MODE_CREATE,
- 0
- );
-
- //
- // see if the error was that it needs to be created
- //
- if ((EFI_ERROR (Status)) && (OpenMode != (OpenMode &~EFI_FILE_MODE_CREATE))) {
- Status = Handle2->Open (
- Handle2,
- &Handle1,
- PathName,
- OpenMode,
- Attributes
- );
- }
- //
- // Close the last node
- //
- Handle2->Close (Handle2);
-
- FreePool (PathName);
-
- if (EFI_ERROR(Status)) {
- return (Status);
- }
-
- //
- // Get the next node
- //
- *FilePath = NextDevicePathNode (*FilePath);
- }
-
- //
- // This is a weak spot since if the undefined SHELL_FILE_HANDLE format changes this must change also!
- //
- *FileHandle = (VOID*)Handle1;
- return EFI_SUCCESS;
-}
-
-
/**
Extract filename from device path. The returned buffer is allocated using AllocateCopyPool.
The caller is responsible for freeing the allocated buffer using FreePool(). If return NULL
@@ -312,7 +163,7 @@ UpdatePage(
gSecureBootPrivateData->FileContext->FileName = FileName;
- OpenFileByDevicePath(
+ EfiOpenFileByDevicePath (
&FilePath,
&gSecureBootPrivateData->FileContext->FHandle,
EFI_FILE_MODE_READ,
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 6/7] ShellPkg/UefiShellLib: drop DeviceHandle param of ShellOpenFileByDevicePath()
2018-08-03 12:15 [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs Laszlo Ersek
` (4 preceding siblings ...)
2018-08-03 12:15 ` [PATCH v2 5/7] SecurityPkg/SecureBootConfigDxe: " Laszlo Ersek
@ 2018-08-03 12:15 ` Laszlo Ersek
2018-08-06 2:04 ` Ni, Ruiyu
2018-08-03 12:15 ` [PATCH v2 7/7] ShellPkg/UefiShellLib: rebase ShellOpenFileByDevicePath() to UefiLib API Laszlo Ersek
` (2 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Laszlo Ersek @ 2018-08-03 12:15 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Jaben Carsey, Ruiyu Ni
The ShellOpenFileByDevicePath() API promises to set the DeviceHandle
output parameter to the handle of the filesystem identified by the
FilePath input parameter. However, this doesn't actually happen when the
UEFI Shell 2.0 method is used (which is basically "always" nowadays).
Accordingly, the only caller of ShellOpenFileByDevicePath(), namely
ShellOpenFileByName(), defines a (dummy) local DeviceHandle variable just
so it can call ShellOpenFileByDevicePath().
Remove the useless output parameter.
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@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>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
---
Notes:
v2:
- pick up Jaben's R-b
ShellPkg/Library/UefiShellLib/UefiShellLib.inf | 2 +-
ShellPkg/Include/Library/ShellLib.h | 2 --
ShellPkg/Library/UefiShellLib/UefiShellLib.c | 11 ++++-------
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
index 0df632378fe6..38d9a4b81f5f 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
@@ -19,7 +19,7 @@ [Defines]
BASE_NAME = UefiShellLib
FILE_GUID = 449D0F00-2148-4a43-9836-F10B3980ECF5
MODULE_TYPE = UEFI_DRIVER
- VERSION_STRING = 1.1
+ VERSION_STRING = 1.2
LIBRARY_CLASS = ShellLib|UEFI_APPLICATION UEFI_DRIVER DXE_RUNTIME_DRIVER DXE_DRIVER
CONSTRUCTOR = ShellLibConstructor
DESTRUCTOR = ShellLibDestructor
diff --git a/ShellPkg/Include/Library/ShellLib.h b/ShellPkg/Include/Library/ShellLib.h
index e360a67ac751..92fddc50f5dd 100644
--- a/ShellPkg/Include/Library/ShellLib.h
+++ b/ShellPkg/Include/Library/ShellLib.h
@@ -89,7 +89,6 @@ ShellSetFileInfo (
@param[in, out] FilePath On input, the device path to the file. On output,
the remaining device path.
- @param[out] DeviceHandle Pointer to the system device handle.
@param[out] FileHandle Pointer to the file handle.
@param[in] OpenMode The mode to open the file with.
@param[in] Attributes The file's file attributes.
@@ -115,7 +114,6 @@ EFI_STATUS
EFIAPI
ShellOpenFileByDevicePath(
IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
- OUT EFI_HANDLE *DeviceHandle,
OUT SHELL_FILE_HANDLE *FileHandle,
IN UINT64 OpenMode,
IN UINT64 Attributes
diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index 3c24ba1742bf..18c3be4a8bc7 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -472,7 +472,6 @@ ShellSetFileInfo (
@param FilePath on input the device path to the file. On output
the remaining device path.
- @param DeviceHandle pointer to the system device handle.
@param FileHandle pointer to the file handle.
@param OpenMode the mode to open the file with.
@param Attributes the file's file attributes.
@@ -498,7 +497,6 @@ EFI_STATUS
EFIAPI
ShellOpenFileByDevicePath(
IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
- OUT EFI_HANDLE *DeviceHandle,
OUT SHELL_FILE_HANDLE *FileHandle,
IN UINT64 OpenMode,
IN UINT64 Attributes
@@ -511,8 +509,9 @@ ShellOpenFileByDevicePath(
EFI_FILE_PROTOCOL *Handle2;
CHAR16 *FnafPathName;
UINTN PathLen;
+ EFI_HANDLE DeviceHandle;
- if (FilePath == NULL || FileHandle == NULL || DeviceHandle == NULL) {
+ if (FilePath == NULL || FileHandle == NULL) {
return (EFI_INVALID_PARAMETER);
}
@@ -538,11 +537,11 @@ ShellOpenFileByDevicePath(
//
Status = gBS->LocateDevicePath (&gEfiSimpleFileSystemProtocolGuid,
FilePath,
- DeviceHandle);
+ &DeviceHandle);
if (EFI_ERROR (Status)) {
return Status;
}
- Status = gBS->OpenProtocol(*DeviceHandle,
+ Status = gBS->OpenProtocol(DeviceHandle,
&gEfiSimpleFileSystemProtocolGuid,
(VOID**)&EfiSimpleFileSystemProtocol,
gImageHandle,
@@ -690,7 +689,6 @@ ShellOpenFileByName(
IN UINT64 Attributes
)
{
- EFI_HANDLE DeviceHandle;
EFI_DEVICE_PATH_PROTOCOL *FilePath;
EFI_STATUS Status;
EFI_FILE_INFO *FileInfo;
@@ -774,7 +772,6 @@ ShellOpenFileByName(
FilePath = mEfiShellEnvironment2->NameToPath ((CHAR16*)FileName);
if (FilePath != NULL) {
return (ShellOpenFileByDevicePath(&FilePath,
- &DeviceHandle,
FileHandle,
OpenMode,
Attributes));
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 7/7] ShellPkg/UefiShellLib: rebase ShellOpenFileByDevicePath() to UefiLib API
2018-08-03 12:15 [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs Laszlo Ersek
` (5 preceding siblings ...)
2018-08-03 12:15 ` [PATCH v2 6/7] ShellPkg/UefiShellLib: drop DeviceHandle param of ShellOpenFileByDevicePath() Laszlo Ersek
@ 2018-08-03 12:15 ` Laszlo Ersek
2018-08-06 2:04 ` Ni, Ruiyu
2018-08-03 16:09 ` [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs Laszlo Ersek
2018-08-09 13:30 ` Laszlo Ersek
8 siblings, 1 reply; 17+ messages in thread
From: Laszlo Ersek @ 2018-08-03 12:15 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Jaben Carsey, Ruiyu Ni
Replace the "old shell method" implementation in
ShellOpenFileByDevicePath() with EfiOpenFileByDevicePath() from UefiLib,
correcting the following issues:
- code duplication between this module and other modules,
- local variable name "EfiSimpleFileSystemProtocol" starting with "Efi"
prefix,
- bogus "FileHandle = NULL" assignments,
- leaking "Handle1" when the device path type/subtype check or the
realignment-motivated AllocateCopyPool() fails in the loop.
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@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>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
---
Notes:
v2:
- pick up Jaben's R-b
ShellPkg/Library/UefiShellLib/UefiShellLib.inf | 1 -
ShellPkg/Library/UefiShellLib/UefiShellLib.c | 113 +-------------------
2 files changed, 3 insertions(+), 111 deletions(-)
diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
index 38d9a4b81f5f..aacddbbf9765 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
@@ -51,7 +51,6 @@ [LibraryClasses]
SortLib
[Protocols]
- gEfiSimpleFileSystemProtocolGuid ## SOMETIMES_CONSUMES
gEfiUnicodeCollation2ProtocolGuid ## CONSUMES
# shell 2.0
diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index 18c3be4a8bc7..f04adbb63ffe 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -504,12 +504,7 @@ ShellOpenFileByDevicePath(
{
CHAR16 *FileName;
EFI_STATUS Status;
- EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *EfiSimpleFileSystemProtocol;
- EFI_FILE_PROTOCOL *Handle1;
- EFI_FILE_PROTOCOL *Handle2;
- CHAR16 *FnafPathName;
- UINTN PathLen;
- EFI_HANDLE DeviceHandle;
+ EFI_FILE_PROTOCOL *File;
if (FilePath == NULL || FileHandle == NULL) {
return (EFI_INVALID_PARAMETER);
@@ -535,117 +530,15 @@ ShellOpenFileByDevicePath(
//
// use old shell method.
//
- Status = gBS->LocateDevicePath (&gEfiSimpleFileSystemProtocolGuid,
- FilePath,
- &DeviceHandle);
+ Status = EfiOpenFileByDevicePath (FilePath, &File, OpenMode, Attributes);
if (EFI_ERROR (Status)) {
return Status;
}
- Status = gBS->OpenProtocol(DeviceHandle,
- &gEfiSimpleFileSystemProtocolGuid,
- (VOID**)&EfiSimpleFileSystemProtocol,
- gImageHandle,
- NULL,
- EFI_OPEN_PROTOCOL_GET_PROTOCOL);
- if (EFI_ERROR (Status)) {
- return Status;
- }
- Status = EfiSimpleFileSystemProtocol->OpenVolume(EfiSimpleFileSystemProtocol, &Handle1);
- if (EFI_ERROR (Status)) {
- FileHandle = NULL;
- return Status;
- }
-
- //
- // go down directories one node at a time.
- //
- while (!IsDevicePathEnd (*FilePath)) {
- //
- // For file system access each node should be a file path component
- //
- if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
- DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP
- ) {
- FileHandle = NULL;
- return (EFI_INVALID_PARAMETER);
- }
- //
- // Open this file path node
- //
- Handle2 = Handle1;
- Handle1 = NULL;
-
- //
- // File Name Alignment Fix (FNAF)
- // Handle2->Open may be incapable of handling a unaligned CHAR16 data.
- // The structure pointed to by FilePath may be not CHAR16 aligned.
- // This code copies the potentially unaligned PathName data from the
- // FilePath structure to the aligned FnafPathName for use in the
- // calls to Handl2->Open.
- //
-
- //
- // Determine length of PathName, in bytes.
- //
- PathLen = DevicePathNodeLength (*FilePath) - SIZE_OF_FILEPATH_DEVICE_PATH;
-
- //
- // Allocate memory for the aligned copy of the string Extra allocation is to allow for forced alignment
- // Copy bytes from possibly unaligned location to aligned location
- //
- FnafPathName = AllocateCopyPool(PathLen, (UINT8 *)((FILEPATH_DEVICE_PATH*)*FilePath)->PathName);
- if (FnafPathName == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
-
- //
- // Try to test opening an existing file
- //
- Status = Handle2->Open (
- Handle2,
- &Handle1,
- FnafPathName,
- OpenMode &~EFI_FILE_MODE_CREATE,
- 0
- );
-
- //
- // see if the error was that it needs to be created
- //
- if ((EFI_ERROR (Status)) && (OpenMode != (OpenMode &~EFI_FILE_MODE_CREATE))) {
- Status = Handle2->Open (
- Handle2,
- &Handle1,
- FnafPathName,
- OpenMode,
- Attributes
- );
- }
-
- //
- // Free the alignment buffer
- //
- FreePool(FnafPathName);
-
- //
- // Close the last node
- //
- Handle2->Close (Handle2);
-
- if (EFI_ERROR(Status)) {
- return (Status);
- }
-
- //
- // Get the next node
- //
- *FilePath = NextDevicePathNode (*FilePath);
- }
//
// This is a weak spot since if the undefined SHELL_FILE_HANDLE format changes this must change also!
//
- *FileHandle = (VOID*)Handle1;
+ *FileHandle = (VOID*)File;
return (EFI_SUCCESS);
}
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs
2018-08-03 12:15 [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs Laszlo Ersek
` (6 preceding siblings ...)
2018-08-03 12:15 ` [PATCH v2 7/7] ShellPkg/UefiShellLib: rebase ShellOpenFileByDevicePath() to UefiLib API Laszlo Ersek
@ 2018-08-03 16:09 ` Laszlo Ersek
2018-08-09 13:30 ` Laszlo Ersek
8 siblings, 0 replies; 17+ messages in thread
From: Laszlo Ersek @ 2018-08-03 16:09 UTC (permalink / raw)
To: edk2-devel-01
Cc: Ruiyu Ni, Eric Dong, Liming Gao, Michael D Kinney, Jiaxin Wu,
Jiewen Yao, Star Zeng, Jaben Carsey, Siyuan Fu, Chao Zhang,
Fam Zheng, Paolo Bonzini
On 08/03/18 14:15, Laszlo Ersek wrote:
> Repo: https://github.com/lersek/edk2.git
> Branch: open_file_by_devpath_tiano_1008_v2
>
> This is version 2 of the patch set that was originally posted at:
>
> https://lists.01.org/pipermail/edk2-devel/2018-July/027253.html
>
> for <https://bugzilla.tianocore.org/show_bug.cgi?id=1008>.
>
> Changes are noted on every patch.
>
> The cumulative code difference is very small (not counting the
> FrameworkUefiLib copy of the function), so I'm including it here for
> easier review:
>
>> [...]
I should use this opportunity to highlight an awesome community feature
from Paolo and Fam (CC'd): the patchew.org website has been tracking
edk2 patch submissions for a while:
https://patchew.org/
https://patchew.org/EDK2/
and now it offers side-by-side comparison between versions of the same
patch set ("interdiff"). For example, the link for this (v2) series is:
https://patchew.org/EDK2/20180803121537.32123-1-lersek@redhat.com/
and if you click the "Diff against v1" link, you get:
https://patchew.org/EDK2/20180718205043.17574-1-lersek@redhat.com/diff/20180803121537.32123-1-lersek@redhat.com/
I recommend that all edk2 reviewers make use of this feature, for
incrementally reviewing patch series.
(This is another good reason for keeping the source code lines limited
to 80 columns -- if you write 200 character long lines, you won't have a
good time looking at side-by-side diffs!)
Laszlo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/7] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
2018-08-03 12:15 ` [PATCH v2 1/7] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath() Laszlo Ersek
@ 2018-08-06 2:03 ` Ni, Ruiyu
0 siblings, 0 replies; 17+ messages in thread
From: Ni, Ruiyu @ 2018-08-06 2:03 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01
Cc: Eric Dong, Liming Gao, Michael D Kinney, Jiaxin Wu, Jiewen Yao,
Star Zeng, Jaben Carsey, Siyuan Fu, Chao Zhang
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 6/7] ShellPkg/UefiShellLib: drop DeviceHandle param of ShellOpenFileByDevicePath()
2018-08-03 12:15 ` [PATCH v2 6/7] ShellPkg/UefiShellLib: drop DeviceHandle param of ShellOpenFileByDevicePath() Laszlo Ersek
@ 2018-08-06 2:04 ` Ni, Ruiyu
0 siblings, 0 replies; 17+ messages in thread
From: Ni, Ruiyu @ 2018-08-06 2:04 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Jaben Carsey
On 8/3/2018 8:15 PM, Laszlo Ersek wrote:
> The ShellOpenFileByDevicePath() API promises to set the DeviceHandle
> output parameter to the handle of the filesystem identified by the
> FilePath input parameter. However, this doesn't actually happen when the
> UEFI Shell 2.0 method is used (which is basically "always" nowadays).
>
> Accordingly, the only caller of ShellOpenFileByDevicePath(), namely
> ShellOpenFileByName(), defines a (dummy) local DeviceHandle variable just
> so it can call ShellOpenFileByDevicePath().
>
> Remove the useless output parameter.
>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@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>
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> ---
>
> Notes:
> v2:
> - pick up Jaben's R-b
>
> ShellPkg/Library/UefiShellLib/UefiShellLib.inf | 2 +-
> ShellPkg/Include/Library/ShellLib.h | 2 --
> ShellPkg/Library/UefiShellLib/UefiShellLib.c | 11 ++++-------
> 3 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> index 0df632378fe6..38d9a4b81f5f 100644
> --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> @@ -19,7 +19,7 @@ [Defines]
> BASE_NAME = UefiShellLib
> FILE_GUID = 449D0F00-2148-4a43-9836-F10B3980ECF5
> MODULE_TYPE = UEFI_DRIVER
> - VERSION_STRING = 1.1
> + VERSION_STRING = 1.2
> LIBRARY_CLASS = ShellLib|UEFI_APPLICATION UEFI_DRIVER DXE_RUNTIME_DRIVER DXE_DRIVER
> CONSTRUCTOR = ShellLibConstructor
> DESTRUCTOR = ShellLibDestructor
> diff --git a/ShellPkg/Include/Library/ShellLib.h b/ShellPkg/Include/Library/ShellLib.h
> index e360a67ac751..92fddc50f5dd 100644
> --- a/ShellPkg/Include/Library/ShellLib.h
> +++ b/ShellPkg/Include/Library/ShellLib.h
> @@ -89,7 +89,6 @@ ShellSetFileInfo (
>
> @param[in, out] FilePath On input, the device path to the file. On output,
> the remaining device path.
> - @param[out] DeviceHandle Pointer to the system device handle.
> @param[out] FileHandle Pointer to the file handle.
> @param[in] OpenMode The mode to open the file with.
> @param[in] Attributes The file's file attributes.
> @@ -115,7 +114,6 @@ EFI_STATUS
> EFIAPI
> ShellOpenFileByDevicePath(
> IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
> - OUT EFI_HANDLE *DeviceHandle,
> OUT SHELL_FILE_HANDLE *FileHandle,
> IN UINT64 OpenMode,
> IN UINT64 Attributes
> diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> index 3c24ba1742bf..18c3be4a8bc7 100644
> --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> @@ -472,7 +472,6 @@ ShellSetFileInfo (
>
> @param FilePath on input the device path to the file. On output
> the remaining device path.
> - @param DeviceHandle pointer to the system device handle.
> @param FileHandle pointer to the file handle.
> @param OpenMode the mode to open the file with.
> @param Attributes the file's file attributes.
> @@ -498,7 +497,6 @@ EFI_STATUS
> EFIAPI
> ShellOpenFileByDevicePath(
> IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
> - OUT EFI_HANDLE *DeviceHandle,
> OUT SHELL_FILE_HANDLE *FileHandle,
> IN UINT64 OpenMode,
> IN UINT64 Attributes
> @@ -511,8 +509,9 @@ ShellOpenFileByDevicePath(
> EFI_FILE_PROTOCOL *Handle2;
> CHAR16 *FnafPathName;
> UINTN PathLen;
> + EFI_HANDLE DeviceHandle;
>
> - if (FilePath == NULL || FileHandle == NULL || DeviceHandle == NULL) {
> + if (FilePath == NULL || FileHandle == NULL) {
> return (EFI_INVALID_PARAMETER);
> }
>
> @@ -538,11 +537,11 @@ ShellOpenFileByDevicePath(
> //
> Status = gBS->LocateDevicePath (&gEfiSimpleFileSystemProtocolGuid,
> FilePath,
> - DeviceHandle);
> + &DeviceHandle);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - Status = gBS->OpenProtocol(*DeviceHandle,
> + Status = gBS->OpenProtocol(DeviceHandle,
> &gEfiSimpleFileSystemProtocolGuid,
> (VOID**)&EfiSimpleFileSystemProtocol,
> gImageHandle,
> @@ -690,7 +689,6 @@ ShellOpenFileByName(
> IN UINT64 Attributes
> )
> {
> - EFI_HANDLE DeviceHandle;
> EFI_DEVICE_PATH_PROTOCOL *FilePath;
> EFI_STATUS Status;
> EFI_FILE_INFO *FileInfo;
> @@ -774,7 +772,6 @@ ShellOpenFileByName(
> FilePath = mEfiShellEnvironment2->NameToPath ((CHAR16*)FileName);
> if (FilePath != NULL) {
> return (ShellOpenFileByDevicePath(&FilePath,
> - &DeviceHandle,
> FileHandle,
> OpenMode,
> Attributes));
>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
--
Thanks,
Ray
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 7/7] ShellPkg/UefiShellLib: rebase ShellOpenFileByDevicePath() to UefiLib API
2018-08-03 12:15 ` [PATCH v2 7/7] ShellPkg/UefiShellLib: rebase ShellOpenFileByDevicePath() to UefiLib API Laszlo Ersek
@ 2018-08-06 2:04 ` Ni, Ruiyu
0 siblings, 0 replies; 17+ messages in thread
From: Ni, Ruiyu @ 2018-08-06 2:04 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Jaben Carsey
On 8/3/2018 8:15 PM, Laszlo Ersek wrote:
> Replace the "old shell method" implementation in
> ShellOpenFileByDevicePath() with EfiOpenFileByDevicePath() from UefiLib,
> correcting the following issues:
>
> - code duplication between this module and other modules,
> - local variable name "EfiSimpleFileSystemProtocol" starting with "Efi"
> prefix,
> - bogus "FileHandle = NULL" assignments,
> - leaking "Handle1" when the device path type/subtype check or the
> realignment-motivated AllocateCopyPool() fails in the loop.
>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@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>
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> ---
>
> Notes:
> v2:
> - pick up Jaben's R-b
>
> ShellPkg/Library/UefiShellLib/UefiShellLib.inf | 1 -
> ShellPkg/Library/UefiShellLib/UefiShellLib.c | 113 +-------------------
> 2 files changed, 3 insertions(+), 111 deletions(-)
>
> diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> index 38d9a4b81f5f..aacddbbf9765 100644
> --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf
> @@ -51,7 +51,6 @@ [LibraryClasses]
> SortLib
>
> [Protocols]
> - gEfiSimpleFileSystemProtocolGuid ## SOMETIMES_CONSUMES
> gEfiUnicodeCollation2ProtocolGuid ## CONSUMES
>
> # shell 2.0
> diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> index 18c3be4a8bc7..f04adbb63ffe 100644
> --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> @@ -504,12 +504,7 @@ ShellOpenFileByDevicePath(
> {
> CHAR16 *FileName;
> EFI_STATUS Status;
> - EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *EfiSimpleFileSystemProtocol;
> - EFI_FILE_PROTOCOL *Handle1;
> - EFI_FILE_PROTOCOL *Handle2;
> - CHAR16 *FnafPathName;
> - UINTN PathLen;
> - EFI_HANDLE DeviceHandle;
> + EFI_FILE_PROTOCOL *File;
>
> if (FilePath == NULL || FileHandle == NULL) {
> return (EFI_INVALID_PARAMETER);
> @@ -535,117 +530,15 @@ ShellOpenFileByDevicePath(
> //
> // use old shell method.
> //
> - Status = gBS->LocateDevicePath (&gEfiSimpleFileSystemProtocolGuid,
> - FilePath,
> - &DeviceHandle);
> + Status = EfiOpenFileByDevicePath (FilePath, &File, OpenMode, Attributes);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - Status = gBS->OpenProtocol(DeviceHandle,
> - &gEfiSimpleFileSystemProtocolGuid,
> - (VOID**)&EfiSimpleFileSystemProtocol,
> - gImageHandle,
> - NULL,
> - EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> - if (EFI_ERROR (Status)) {
> - return Status;
> - }
> - Status = EfiSimpleFileSystemProtocol->OpenVolume(EfiSimpleFileSystemProtocol, &Handle1);
> - if (EFI_ERROR (Status)) {
> - FileHandle = NULL;
> - return Status;
> - }
> -
> - //
> - // go down directories one node at a time.
> - //
> - while (!IsDevicePathEnd (*FilePath)) {
> - //
> - // For file system access each node should be a file path component
> - //
> - if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
> - DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP
> - ) {
> - FileHandle = NULL;
> - return (EFI_INVALID_PARAMETER);
> - }
> - //
> - // Open this file path node
> - //
> - Handle2 = Handle1;
> - Handle1 = NULL;
> -
> - //
> - // File Name Alignment Fix (FNAF)
> - // Handle2->Open may be incapable of handling a unaligned CHAR16 data.
> - // The structure pointed to by FilePath may be not CHAR16 aligned.
> - // This code copies the potentially unaligned PathName data from the
> - // FilePath structure to the aligned FnafPathName for use in the
> - // calls to Handl2->Open.
> - //
> -
> - //
> - // Determine length of PathName, in bytes.
> - //
> - PathLen = DevicePathNodeLength (*FilePath) - SIZE_OF_FILEPATH_DEVICE_PATH;
> -
> - //
> - // Allocate memory for the aligned copy of the string Extra allocation is to allow for forced alignment
> - // Copy bytes from possibly unaligned location to aligned location
> - //
> - FnafPathName = AllocateCopyPool(PathLen, (UINT8 *)((FILEPATH_DEVICE_PATH*)*FilePath)->PathName);
> - if (FnafPathName == NULL) {
> - return EFI_OUT_OF_RESOURCES;
> - }
> -
> - //
> - // Try to test opening an existing file
> - //
> - Status = Handle2->Open (
> - Handle2,
> - &Handle1,
> - FnafPathName,
> - OpenMode &~EFI_FILE_MODE_CREATE,
> - 0
> - );
> -
> - //
> - // see if the error was that it needs to be created
> - //
> - if ((EFI_ERROR (Status)) && (OpenMode != (OpenMode &~EFI_FILE_MODE_CREATE))) {
> - Status = Handle2->Open (
> - Handle2,
> - &Handle1,
> - FnafPathName,
> - OpenMode,
> - Attributes
> - );
> - }
> -
> - //
> - // Free the alignment buffer
> - //
> - FreePool(FnafPathName);
> -
> - //
> - // Close the last node
> - //
> - Handle2->Close (Handle2);
> -
> - if (EFI_ERROR(Status)) {
> - return (Status);
> - }
> -
> - //
> - // Get the next node
> - //
> - *FilePath = NextDevicePathNode (*FilePath);
> - }
>
> //
> // This is a weak spot since if the undefined SHELL_FILE_HANDLE format changes this must change also!
> //
> - *FileHandle = (VOID*)Handle1;
> + *FileHandle = (VOID*)File;
> return (EFI_SUCCESS);
> }
>
>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
--
Thanks,
Ray
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/7] SecurityPkg/SecureBootConfigDxe: replace OpenFileByDevicePath() with UefiLib API
2018-08-03 12:15 ` [PATCH v2 5/7] SecurityPkg/SecureBootConfigDxe: " Laszlo Ersek
@ 2018-08-07 12:16 ` Zhang, Chao B
0 siblings, 0 replies; 17+ messages in thread
From: Zhang, Chao B @ 2018-08-07 12:16 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Yao, Jiewen, Roman Bacik
Reviewed-by : Chao Zhang <chao.b.zhang@intel.com>
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Friday, August 3, 2018 8:16 PM
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Zhang, Chao B <chao.b.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Roman Bacik <roman.bacik@broadcom.com>
Subject: [PATCH v2 5/7] SecurityPkg/SecureBootConfigDxe: replace OpenFileByDevicePath() with UefiLib API
Replace the OpenFileByDevicePath() function with EfiOpenFileByDevicePath() from UefiLib, correcting the following issues:
- imprecise comments on OpenFileByDevicePath(),
- code duplication between this module and other modules,
- local variable name "EfiSimpleFileSystemProtocol" starting with "Efi"
prefix,
- bogus "FileHandle = NULL" assignments,
- leaking "Handle1" when the device path type/subtype check or the
realignment-motivated AllocateCopyPool() fails in the loop,
- stale SHELL_FILE_HANDLE reference in a comment.
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Roman Bacik <roman.bacik@broadcom.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>
Reviewed-by: Chao Zhang <chao.b.zhang@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
---
Notes:
v2:
- pick up Chao's and Jaben's R-b's
- insert a space character between "EfiOpenFileByDevicePath" and "(" --
it was missing from the pre-patch code too
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf | 1 -
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c | 151 +-------------------
2 files changed, 1 insertion(+), 151 deletions(-)
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
index 487fc8cda917..caf95ddac7d9 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
+++ nfigDxe.inf
@@ -114,7 +114,6 @@ [Guids]
[Protocols]
gEfiHiiConfigAccessProtocolGuid ## PRODUCES
gEfiDevicePathProtocolGuid ## PRODUCES
- gEfiSimpleFileSystemProtocolGuid ## SOMETIMES_CONSUMES
gEfiBlockIoProtocolGuid ## SOMETIMES_CONSUMES
[Depex]
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
index 2a26c20f394c..aef85c470143 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
+++ nfigFileExplorer.c
@@ -80,155 +80,6 @@ CleanUpPage (
);
}
-/**
- This function will open a file or directory referenced by DevicePath.
-
- This function opens a file with the open mode according to the file path. The
- Attributes is valid only for EFI_FILE_MODE_CREATE.
-
- @param[in, out] FilePath On input, the device path to the file.
- On output, the remaining device path.
- @param[out] FileHandle Pointer to the file handle.
- @param[in] OpenMode The mode to open the file with.
- @param[in] Attributes The file's file attributes.
-
- @retval EFI_SUCCESS The information was set.
- @retval EFI_INVALID_PARAMETER One of the parameters has an invalid value.
- @retval EFI_UNSUPPORTED Could not open the file path.
- @retval EFI_NOT_FOUND The specified file could not be found on the
- device or the file system could not be found on
- the device.
- @retval EFI_NO_MEDIA The device has no medium.
- @retval EFI_MEDIA_CHANGED The device has a different medium in it or the
- medium is no longer supported.
- @retval EFI_DEVICE_ERROR The device reported an error.
- @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
- @retval EFI_WRITE_PROTECTED The file or medium is write protected.
- @retval EFI_ACCESS_DENIED The file was opened read only.
- @retval EFI_OUT_OF_RESOURCES Not enough resources were available to open the
- file.
- @retval EFI_VOLUME_FULL The volume is full.
-**/
-EFI_STATUS
-EFIAPI
-OpenFileByDevicePath(
- IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath,
- OUT EFI_FILE_HANDLE *FileHandle,
- IN UINT64 OpenMode,
- IN UINT64 Attributes
- )
-{
- EFI_STATUS Status;
- EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *EfiSimpleFileSystemProtocol;
- EFI_FILE_PROTOCOL *Handle1;
- EFI_FILE_PROTOCOL *Handle2;
- EFI_HANDLE DeviceHandle;
- CHAR16 *PathName;
- UINTN PathLength;
-
- if ((FilePath == NULL || FileHandle == NULL)) {
- return EFI_INVALID_PARAMETER;
- }
-
- Status = gBS->LocateDevicePath (
- &gEfiSimpleFileSystemProtocolGuid,
- FilePath,
- &DeviceHandle
- );
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- Status = gBS->OpenProtocol(
- DeviceHandle,
- &gEfiSimpleFileSystemProtocolGuid,
- (VOID**)&EfiSimpleFileSystemProtocol,
- gImageHandle,
- NULL,
- EFI_OPEN_PROTOCOL_GET_PROTOCOL
- );
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- Status = EfiSimpleFileSystemProtocol->OpenVolume(EfiSimpleFileSystemProtocol, &Handle1);
- if (EFI_ERROR (Status)) {
- FileHandle = NULL;
- return Status;
- }
-
- //
- // go down directories one node at a time.
- //
- while (!IsDevicePathEnd (*FilePath)) {
- //
- // For file system access each node should be a file path component
- //
- if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
- DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP
- ) {
- FileHandle = NULL;
- return (EFI_INVALID_PARAMETER);
- }
- //
- // Open this file path node
- //
- 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
- //
- Status = Handle2->Open (
- Handle2,
- &Handle1,
- PathName,
- OpenMode &~EFI_FILE_MODE_CREATE,
- 0
- );
-
- //
- // see if the error was that it needs to be created
- //
- if ((EFI_ERROR (Status)) && (OpenMode != (OpenMode &~EFI_FILE_MODE_CREATE))) {
- Status = Handle2->Open (
- Handle2,
- &Handle1,
- PathName,
- OpenMode,
- Attributes
- );
- }
- //
- // Close the last node
- //
- Handle2->Close (Handle2);
-
- FreePool (PathName);
-
- if (EFI_ERROR(Status)) {
- return (Status);
- }
-
- //
- // Get the next node
- //
- *FilePath = NextDevicePathNode (*FilePath);
- }
-
- //
- // This is a weak spot since if the undefined SHELL_FILE_HANDLE format changes this must change also!
- //
- *FileHandle = (VOID*)Handle1;
- return EFI_SUCCESS;
-}
-
-
/**
Extract filename from device path. The returned buffer is allocated using AllocateCopyPool.
The caller is responsible for freeing the allocated buffer using FreePool(). If return NULL @@ -312,7 +163,7 @@ UpdatePage(
gSecureBootPrivateData->FileContext->FileName = FileName;
- OpenFileByDevicePath(
+ EfiOpenFileByDevicePath (
&FilePath,
&gSecureBootPrivateData->FileContext->FHandle,
EFI_FILE_MODE_READ,
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs
2018-08-03 12:15 [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs Laszlo Ersek
` (7 preceding siblings ...)
2018-08-03 16:09 ` [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs Laszlo Ersek
@ 2018-08-09 13:30 ` Laszlo Ersek
2018-08-15 17:20 ` Laszlo Ersek
8 siblings, 1 reply; 17+ messages in thread
From: Laszlo Ersek @ 2018-08-09 13:30 UTC (permalink / raw)
To: Liming Gao, Michael D Kinney
Cc: edk2-devel-01, Ruiyu Ni, Eric Dong, Jiaxin Wu, Jiewen Yao,
Star Zeng, Jaben Carsey, Siyuan Fu, Chao Zhang
On 08/03/18 14:15, Laszlo Ersek wrote:
> Repo: https://github.com/lersek/edk2.git
> Branch: open_file_by_devpath_tiano_1008_v2
>
> This is version 2 of the patch set that was originally posted at:
>
> https://lists.01.org/pipermail/edk2-devel/2018-July/027253.html
>
> for <https://bugzilla.tianocore.org/show_bug.cgi?id=1008>.
>
> Changes are noted on every patch.
Liming, Mike, can you please check patches #1 and #2?
(Obviously I'll only push the series after the quiet period ends.)
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs
2018-08-09 13:30 ` Laszlo Ersek
@ 2018-08-15 17:20 ` Laszlo Ersek
2018-08-15 17:42 ` Gao, Liming
0 siblings, 1 reply; 17+ messages in thread
From: Laszlo Ersek @ 2018-08-15 17:20 UTC (permalink / raw)
To: Liming Gao, Michael D Kinney
Cc: Ruiyu Ni, Eric Dong, edk2-devel-01, Jiaxin Wu, Jiewen Yao,
Chao Zhang, Jaben Carsey, Siyuan Fu, Star Zeng
On 08/09/18 15:30, Laszlo Ersek wrote:
> On 08/03/18 14:15, Laszlo Ersek wrote:
>> Repo: https://github.com/lersek/edk2.git
>> Branch: open_file_by_devpath_tiano_1008_v2
>>
>> This is version 2 of the patch set that was originally posted at:
>>
>> https://lists.01.org/pipermail/edk2-devel/2018-July/027253.html
>>
>> for <https://bugzilla.tianocore.org/show_bug.cgi?id=1008>.
>>
>> Changes are noted on every patch.
>
> Liming, Mike, can you please check patches #1 and #2?
>
> (Obviously I'll only push the series after the quiet period ends.)
pinging this set again
Thanks
Laszlo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs
2018-08-15 17:20 ` Laszlo Ersek
@ 2018-08-15 17:42 ` Gao, Liming
2018-08-16 18:12 ` Laszlo Ersek
0 siblings, 1 reply; 17+ messages in thread
From: Gao, Liming @ 2018-08-15 17:42 UTC (permalink / raw)
To: Laszlo Ersek, Kinney, Michael D
Cc: Ni, Ruiyu, Dong, Eric, edk2-devel-01, Wu, Jiaxin, Yao, Jiewen,
Zhang, Chao B, Carsey, Jaben, Fu, Siyuan, Zeng, Star
Laszlo:
Sorry to miss the patch. Thank you to update IntelFrameworkPkg UefiLib. The change in MdePkg and IntelFrameworkPkg is good to me. Reviewed-by: Liming Gao <liming.gao@intel.com>
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, August 15, 2018 10:21 AM
> To: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; edk2-devel-01 <edk2-devel@lists.01.org>; Wu, Jiaxin
> <jiaxin.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>; Carsey, Jaben
> <jaben.carsey@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs
>
> On 08/09/18 15:30, Laszlo Ersek wrote:
> > On 08/03/18 14:15, Laszlo Ersek wrote:
> >> Repo: https://github.com/lersek/edk2.git
> >> Branch: open_file_by_devpath_tiano_1008_v2
> >>
> >> This is version 2 of the patch set that was originally posted at:
> >>
> >> https://lists.01.org/pipermail/edk2-devel/2018-July/027253.html
> >>
> >> for <https://bugzilla.tianocore.org/show_bug.cgi?id=1008>.
> >>
> >> Changes are noted on every patch.
> >
> > Liming, Mike, can you please check patches #1 and #2?
> >
> > (Obviously I'll only push the series after the quiet period ends.)
>
> pinging this set again
>
> Thanks
> Laszlo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs
2018-08-15 17:42 ` Gao, Liming
@ 2018-08-16 18:12 ` Laszlo Ersek
0 siblings, 0 replies; 17+ messages in thread
From: Laszlo Ersek @ 2018-08-16 18:12 UTC (permalink / raw)
To: Gao, Liming, Kinney, Michael D
Cc: Ni, Ruiyu, Dong, Eric, edk2-devel-01, Wu, Jiaxin, Yao, Jiewen,
Zeng, Star, Carsey, Jaben, Fu, Siyuan, Zhang, Chao B
On 08/15/18 19:42, Gao, Liming wrote:
> Laszlo:
> Sorry to miss the patch. Thank you to update IntelFrameworkPkg UefiLib. The change in MdePkg and IntelFrameworkPkg is good to me. Reviewed-by: Liming Gao <liming.gao@intel.com>
Thank you.
Series pushed as commit range 52047be02430..9becf2f0759e.
Laszlo
>
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Wednesday, August 15, 2018 10:21 AM
>> To: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
>> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; edk2-devel-01 <edk2-devel@lists.01.org>; Wu, Jiaxin
>> <jiaxin.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>; Carsey, Jaben
>> <jaben.carsey@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Zeng, Star <star.zeng@intel.com>
>> Subject: Re: [edk2] [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs
>>
>> On 08/09/18 15:30, Laszlo Ersek wrote:
>>> On 08/03/18 14:15, Laszlo Ersek wrote:
>>>> Repo: https://github.com/lersek/edk2.git
>>>> Branch: open_file_by_devpath_tiano_1008_v2
>>>>
>>>> This is version 2 of the patch set that was originally posted at:
>>>>
>>>> https://lists.01.org/pipermail/edk2-devel/2018-July/027253.html
>>>>
>>>> for <https://bugzilla.tianocore.org/show_bug.cgi?id=1008>.
>>>>
>>>> Changes are noted on every patch.
>>>
>>> Liming, Mike, can you please check patches #1 and #2?
>>>
>>> (Obviously I'll only push the series after the quiet period ends.)
>>
>> pinging this set again
>>
>> Thanks
>> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-08-16 18:12 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-03 12:15 [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs Laszlo Ersek
2018-08-03 12:15 ` [PATCH v2 1/7] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath() Laszlo Ersek
2018-08-06 2:03 ` Ni, Ruiyu
2018-08-03 12:15 ` [PATCH v2 2/7] IntelFrameworkPkg/FrameworkUefiLib: " Laszlo Ersek
2018-08-03 12:15 ` [PATCH v2 3/7] MdeModulePkg/RamDiskDxe: replace OpenFileByDevicePath() with UefiLib API Laszlo Ersek
2018-08-03 12:15 ` [PATCH v2 4/7] NetworkPkg/TlsAuthConfigDxe: " Laszlo Ersek
2018-08-03 12:15 ` [PATCH v2 5/7] SecurityPkg/SecureBootConfigDxe: " Laszlo Ersek
2018-08-07 12:16 ` Zhang, Chao B
2018-08-03 12:15 ` [PATCH v2 6/7] ShellPkg/UefiShellLib: drop DeviceHandle param of ShellOpenFileByDevicePath() Laszlo Ersek
2018-08-06 2:04 ` Ni, Ruiyu
2018-08-03 12:15 ` [PATCH v2 7/7] ShellPkg/UefiShellLib: rebase ShellOpenFileByDevicePath() to UefiLib API Laszlo Ersek
2018-08-06 2:04 ` Ni, Ruiyu
2018-08-03 16:09 ` [PATCH v2 0/7] UefiLib: centralize OpenFileByDevicePath() and fix its bugs Laszlo Ersek
2018-08-09 13:30 ` Laszlo Ersek
2018-08-15 17:20 ` Laszlo Ersek
2018-08-15 17:42 ` Gao, Liming
2018-08-16 18:12 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox