* [PATCH 1/4] MdePkg/UefiLib: don't special-case EFI_FILE_MODE_CREATE in OpenMode
2018-08-17 14:35 [PATCH 0/4] {Mde, IntelFramework, Network, UefiCpu}Pkg: roundup for BZs 1074, 1090, 1092 Laszlo Ersek
@ 2018-08-17 14:35 ` Laszlo Ersek
2018-08-17 20:35 ` Gao, Liming
2018-08-17 14:35 ` [PATCH 2/4] IntelFrameworkPkg/FrameworkUefiLib: don't special-case EFI_FILE_MODE_CREATE Laszlo Ersek
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Laszlo Ersek @ 2018-08-17 14:35 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Liming Gao, Michael D Kinney, Ruiyu Ni
While reviewing the patch that would land as 768b611136d0
("MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()", 2018-08-16), Ray
pointed out that distinguishing EFI_FILE_MODE_CREATE was wasteful. Per
spec, if the file to create exists, then EFI_FILE_MODE_CREATE is ignored
by EFI_FILE_PROTOCOL.Open(), and the existent file is opened.
Therefore we don't need an attempt to "open-but-not-create" first, and a
fallback to "open-and-create-too" second -- that behavior is internal to
EFI_FILE_PROTOCOL.Open(). Remove the special-casing of
EFI_FILE_MODE_CREATE.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Suggested-by: Ruiyu Ni <ruiyu.ni@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1074
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
Tested with MdeModulePkg/RamDiskDxe: created a virtual disk from an ISO
file, using the HII form; browsed the disk contents from the UEFI shell.
MdePkg/Include/Library/UefiLib.h | 19 ++--------
MdePkg/Library/UefiLib/UefiLib.c | 40 +++-----------------
2 files changed, 9 insertions(+), 50 deletions(-)
diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index f950f1c9c648..f80067f11103 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -1533,12 +1533,7 @@ EfiLocateProtocolBuffer (
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.
+ EFI_FILE_PROTOCOL.Open().
(As a consequence, if OpenMode includes EFI_FILE_MODE_CREATE, and Attributes
includes EFI_FILE_DIRECTORY, and each FILEPATH_DEVICE_PATH specifies a single
@@ -1570,18 +1565,10 @@ EfiLocateProtocolBuffer (
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.
+ EFI_FILE_PROTOCOL.Open().
@param[in] Attributes The Attributes parameter to pass to
- EFI_FILE_PROTOCOL.Open(), when EFI_FILE_MODE_CREATE
- is propagated unmasked in OpenMode.
+ EFI_FILE_PROTOCOL.Open().
@retval EFI_SUCCESS The file or directory has been opened or
created.
diff --git a/MdePkg/Library/UefiLib/UefiLib.c b/MdePkg/Library/UefiLib/UefiLib.c
index 7bcac5613768..39d0ce2b21b3 100644
--- a/MdePkg/Library/UefiLib/UefiLib.c
+++ b/MdePkg/Library/UefiLib/UefiLib.c
@@ -1730,12 +1730,7 @@ EfiLocateProtocolBuffer (
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.
+ EFI_FILE_PROTOCOL.Open().
(As a consequence, if OpenMode includes EFI_FILE_MODE_CREATE, and Attributes
includes EFI_FILE_DIRECTORY, and each FILEPATH_DEVICE_PATH specifies a single
@@ -1767,18 +1762,10 @@ EfiLocateProtocolBuffer (
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.
+ EFI_FILE_PROTOCOL.Open().
@param[in] Attributes The Attributes parameter to pass to
- EFI_FILE_PROTOCOL.Open(), when EFI_FILE_MODE_CREATE
- is propagated unmasked in OpenMode.
+ EFI_FILE_PROTOCOL.Open().
@retval EFI_SUCCESS The file or directory has been opened or
created.
@@ -1889,31 +1876,16 @@ EfiOpenFileByDevicePath (
}
//
- // Open the next pathname fragment with EFI_FILE_MODE_CREATE masked out and
- // with Attributes set to 0.
+ // Open or create the file corresponding to the next pathname fragment.
//
Status = LastFile->Open (
LastFile,
&NextFile,
PathName,
- OpenMode & ~(UINT64)EFI_FILE_MODE_CREATE,
- 0
+ OpenMode,
+ Attributes
);
- //
- // 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.
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] MdePkg/UefiLib: don't special-case EFI_FILE_MODE_CREATE in OpenMode
2018-08-17 14:35 ` [PATCH 1/4] MdePkg/UefiLib: don't special-case EFI_FILE_MODE_CREATE in OpenMode Laszlo Ersek
@ 2018-08-17 20:35 ` Gao, Liming
0 siblings, 0 replies; 13+ messages in thread
From: Gao, Liming @ 2018-08-17 20:35 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Kinney, Michael D, Ni, Ruiyu
Reviewed-by: Liming Gao <liming.gao@intel.com>
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, August 17, 2018 7:36 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [PATCH 1/4] MdePkg/UefiLib: don't special-case EFI_FILE_MODE_CREATE in OpenMode
>
> While reviewing the patch that would land as 768b611136d0
> ("MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()", 2018-08-16), Ray
> pointed out that distinguishing EFI_FILE_MODE_CREATE was wasteful. Per
> spec, if the file to create exists, then EFI_FILE_MODE_CREATE is ignored
> by EFI_FILE_PROTOCOL.Open(), and the existent file is opened.
>
> Therefore we don't need an attempt to "open-but-not-create" first, and a
> fallback to "open-and-create-too" second -- that behavior is internal to
> EFI_FILE_PROTOCOL.Open(). Remove the special-casing of
> EFI_FILE_MODE_CREATE.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Suggested-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1074
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> Tested with MdeModulePkg/RamDiskDxe: created a virtual disk from an ISO
> file, using the HII form; browsed the disk contents from the UEFI shell.
>
> MdePkg/Include/Library/UefiLib.h | 19 ++--------
> MdePkg/Library/UefiLib/UefiLib.c | 40 +++-----------------
> 2 files changed, 9 insertions(+), 50 deletions(-)
>
> diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
> index f950f1c9c648..f80067f11103 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -1533,12 +1533,7 @@ EfiLocateProtocolBuffer (
>
> 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.
> + EFI_FILE_PROTOCOL.Open().
>
> (As a consequence, if OpenMode includes EFI_FILE_MODE_CREATE, and Attributes
> includes EFI_FILE_DIRECTORY, and each FILEPATH_DEVICE_PATH specifies a single
> @@ -1570,18 +1565,10 @@ EfiLocateProtocolBuffer (
> 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.
> + EFI_FILE_PROTOCOL.Open().
>
> @param[in] Attributes The Attributes parameter to pass to
> - EFI_FILE_PROTOCOL.Open(), when EFI_FILE_MODE_CREATE
> - is propagated unmasked in OpenMode.
> + EFI_FILE_PROTOCOL.Open().
>
> @retval EFI_SUCCESS The file or directory has been opened or
> created.
> diff --git a/MdePkg/Library/UefiLib/UefiLib.c b/MdePkg/Library/UefiLib/UefiLib.c
> index 7bcac5613768..39d0ce2b21b3 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.c
> +++ b/MdePkg/Library/UefiLib/UefiLib.c
> @@ -1730,12 +1730,7 @@ EfiLocateProtocolBuffer (
>
> 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.
> + EFI_FILE_PROTOCOL.Open().
>
> (As a consequence, if OpenMode includes EFI_FILE_MODE_CREATE, and Attributes
> includes EFI_FILE_DIRECTORY, and each FILEPATH_DEVICE_PATH specifies a single
> @@ -1767,18 +1762,10 @@ EfiLocateProtocolBuffer (
> 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.
> + EFI_FILE_PROTOCOL.Open().
>
> @param[in] Attributes The Attributes parameter to pass to
> - EFI_FILE_PROTOCOL.Open(), when EFI_FILE_MODE_CREATE
> - is propagated unmasked in OpenMode.
> + EFI_FILE_PROTOCOL.Open().
>
> @retval EFI_SUCCESS The file or directory has been opened or
> created.
> @@ -1889,31 +1876,16 @@ EfiOpenFileByDevicePath (
> }
>
> //
> - // Open the next pathname fragment with EFI_FILE_MODE_CREATE masked out and
> - // with Attributes set to 0.
> + // Open or create the file corresponding to the next pathname fragment.
> //
> Status = LastFile->Open (
> LastFile,
> &NextFile,
> PathName,
> - OpenMode & ~(UINT64)EFI_FILE_MODE_CREATE,
> - 0
> + OpenMode,
> + Attributes
> );
>
> - //
> - // 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.
> --
> 2.14.1.3.gb7cf6e02401b
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] IntelFrameworkPkg/FrameworkUefiLib: don't special-case EFI_FILE_MODE_CREATE
2018-08-17 14:35 [PATCH 0/4] {Mde, IntelFramework, Network, UefiCpu}Pkg: roundup for BZs 1074, 1090, 1092 Laszlo Ersek
2018-08-17 14:35 ` [PATCH 1/4] MdePkg/UefiLib: don't special-case EFI_FILE_MODE_CREATE in OpenMode Laszlo Ersek
@ 2018-08-17 14:35 ` Laszlo Ersek
2018-08-17 20:35 ` Gao, Liming
2018-08-17 14:35 ` [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval Laszlo Ersek
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Laszlo Ersek @ 2018-08-17 14:35 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Liming Gao, Michael D Kinney, Ruiyu Ni
Synchronize EfiOpenFileByDevicePath() with the MdePkg/UefiLib instance, as
described in the previous patch.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Suggested-by: Ruiyu Ni <ruiyu.ni@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1074
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
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/UefiLib.c | 40 +++-----------------
1 file changed, 6 insertions(+), 34 deletions(-)
diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c b/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
index b283d775b470..45bf4b17c5c3 100644
--- a/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
+++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
@@ -1702,12 +1702,7 @@ EfiLocateProtocolBuffer (
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.
+ EFI_FILE_PROTOCOL.Open().
(As a consequence, if OpenMode includes EFI_FILE_MODE_CREATE, and Attributes
includes EFI_FILE_DIRECTORY, and each FILEPATH_DEVICE_PATH specifies a single
@@ -1739,18 +1734,10 @@ EfiLocateProtocolBuffer (
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.
+ EFI_FILE_PROTOCOL.Open().
@param[in] Attributes The Attributes parameter to pass to
- EFI_FILE_PROTOCOL.Open(), when EFI_FILE_MODE_CREATE
- is propagated unmasked in OpenMode.
+ EFI_FILE_PROTOCOL.Open().
@retval EFI_SUCCESS The file or directory has been opened or
created.
@@ -1861,31 +1848,16 @@ EfiOpenFileByDevicePath (
}
//
- // Open the next pathname fragment with EFI_FILE_MODE_CREATE masked out and
- // with Attributes set to 0.
+ // Open or create the file corresponding to the next pathname fragment.
//
Status = LastFile->Open (
LastFile,
&NextFile,
PathName,
- OpenMode & ~(UINT64)EFI_FILE_MODE_CREATE,
- 0
+ OpenMode,
+ Attributes
);
- //
- // 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.
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] IntelFrameworkPkg/FrameworkUefiLib: don't special-case EFI_FILE_MODE_CREATE
2018-08-17 14:35 ` [PATCH 2/4] IntelFrameworkPkg/FrameworkUefiLib: don't special-case EFI_FILE_MODE_CREATE Laszlo Ersek
@ 2018-08-17 20:35 ` Gao, Liming
0 siblings, 0 replies; 13+ messages in thread
From: Gao, Liming @ 2018-08-17 20:35 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Kinney, Michael D, Ni, Ruiyu
Reviewed-by: Liming Gao <liming.gao@intel.com>
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, August 17, 2018 7:36 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [PATCH 2/4] IntelFrameworkPkg/FrameworkUefiLib: don't special-case EFI_FILE_MODE_CREATE
>
> Synchronize EfiOpenFileByDevicePath() with the MdePkg/UefiLib instance, as
> described in the previous patch.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Suggested-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1074
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> 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/UefiLib.c | 40 +++-----------------
> 1 file changed, 6 insertions(+), 34 deletions(-)
>
> diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c b/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
> index b283d775b470..45bf4b17c5c3 100644
> --- a/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
> +++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
> @@ -1702,12 +1702,7 @@ EfiLocateProtocolBuffer (
>
> 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.
> + EFI_FILE_PROTOCOL.Open().
>
> (As a consequence, if OpenMode includes EFI_FILE_MODE_CREATE, and Attributes
> includes EFI_FILE_DIRECTORY, and each FILEPATH_DEVICE_PATH specifies a single
> @@ -1739,18 +1734,10 @@ EfiLocateProtocolBuffer (
> 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.
> + EFI_FILE_PROTOCOL.Open().
>
> @param[in] Attributes The Attributes parameter to pass to
> - EFI_FILE_PROTOCOL.Open(), when EFI_FILE_MODE_CREATE
> - is propagated unmasked in OpenMode.
> + EFI_FILE_PROTOCOL.Open().
>
> @retval EFI_SUCCESS The file or directory has been opened or
> created.
> @@ -1861,31 +1848,16 @@ EfiOpenFileByDevicePath (
> }
>
> //
> - // Open the next pathname fragment with EFI_FILE_MODE_CREATE masked out and
> - // with Attributes set to 0.
> + // Open or create the file corresponding to the next pathname fragment.
> //
> Status = LastFile->Open (
> LastFile,
> &NextFile,
> PathName,
> - OpenMode & ~(UINT64)EFI_FILE_MODE_CREATE,
> - 0
> + OpenMode,
> + Attributes
> );
>
> - //
> - // 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.
> --
> 2.14.1.3.gb7cf6e02401b
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval
2018-08-17 14:35 [PATCH 0/4] {Mde, IntelFramework, Network, UefiCpu}Pkg: roundup for BZs 1074, 1090, 1092 Laszlo Ersek
2018-08-17 14:35 ` [PATCH 1/4] MdePkg/UefiLib: don't special-case EFI_FILE_MODE_CREATE in OpenMode Laszlo Ersek
2018-08-17 14:35 ` [PATCH 2/4] IntelFrameworkPkg/FrameworkUefiLib: don't special-case EFI_FILE_MODE_CREATE Laszlo Ersek
@ 2018-08-17 14:35 ` Laszlo Ersek
2018-08-20 6:29 ` Li, Songpeng
2018-08-22 0:31 ` Fu, Siyuan
2018-08-17 14:35 ` [PATCH 4/4] UefiCpuPkg/PiSmmCpuDxeSmm: clear exec file mode bits on "PiSmmCpuDxeSmm.c" Laszlo Ersek
2018-08-22 8:41 ` [PATCH 0/4] {Mde, IntelFramework, Network, UefiCpu}Pkg: roundup for BZs 1074, 1090, 1092 Laszlo Ersek
4 siblings, 2 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-08-17 14:35 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Jiaxin Wu, Siyuan Fu, Songpeng Li
Per spec, the GetVariable() runtime service is not required to populate
(*Attributes) on output when it fails with EFI_BUFFER_TOO_SMALL.
Therefore we have to fetch the full contents of the TlsCaCertificate
variable temporarily, just so we can (a) get the current attributes, and
(b) add EFI_VARIABLE_APPEND_WRITE to them for the subsequent SetVariable()
call.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Songpeng Li <songpeng.li@intel.com>
Reported-by: Songpeng Li <songpeng.li@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1090
Fixes: b90c335fbbb674470fbf09601cc522bf61564c30
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
Tested via loading the same CA cert .pem file twice in a row, using the
HII form, first without any pre-existent TlsCaCertificate variable.
Songpeng, can you please test this patch as well, and confirm if it
works on your end? Thanks!
NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c | 27 +++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
index 7259c5e82f61..0780b03bbab4 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
@@ -663,6 +663,7 @@ EnrollX509toVariable (
EFI_SIGNATURE_LIST *CACert;
EFI_SIGNATURE_DATA *CACertData;
VOID *Data;
+ VOID *CurrentData;
UINTN DataSize;
UINTN SigDataSize;
UINT32 Attr;
@@ -674,6 +675,7 @@ EnrollX509toVariable (
CACert = NULL;
CACertData = NULL;
Data = NULL;
+ CurrentData = NULL;
Attr = 0;
Status = ReadFileContent (
@@ -716,11 +718,30 @@ EnrollX509toVariable (
Status = gRT->GetVariable(
VariableName,
&gEfiTlsCaCertificateGuid,
- &Attr,
+ NULL,
&DataSize,
NULL
);
if (Status == EFI_BUFFER_TOO_SMALL) {
+ //
+ // Per spec, we have to fetch the variable's contents, even though we're
+ // only interested in the variable's attributes.
+ //
+ CurrentData = AllocatePool (DataSize);
+ if (CurrentData == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto ON_EXIT;
+ }
+ Status = gRT->GetVariable(
+ VariableName,
+ &gEfiTlsCaCertificateGuid,
+ &Attr,
+ &DataSize,
+ CurrentData
+ );
+ if (EFI_ERROR (Status)) {
+ goto ON_EXIT;
+ }
Attr |= EFI_VARIABLE_APPEND_WRITE;
} else if (Status == EFI_NOT_FOUND) {
Attr = TLS_AUTH_CONFIG_VAR_BASE_ATTR;
@@ -751,6 +772,10 @@ ON_EXIT:
FreePool (Data);
}
+ if (CurrentData != NULL) {
+ FreePool (CurrentData);
+ }
+
if (X509Data != NULL) {
FreePool (X509Data);
}
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval
2018-08-17 14:35 ` [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval Laszlo Ersek
@ 2018-08-20 6:29 ` Li, Songpeng
2018-08-21 13:30 ` Laszlo Ersek
2018-08-22 0:36 ` Wu, Jiaxin
2018-08-22 0:31 ` Fu, Siyuan
1 sibling, 2 replies; 13+ messages in thread
From: Li, Songpeng @ 2018-08-20 6:29 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Wu, Jiaxin, Fu, Siyuan
It worked on my end.
Tested-by: Songpeng Li <songpeng.li@intel.com>
Thanks & Best Regards,
Songpeng
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, August 17, 2018 10:36 PM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Li,
> Songpeng <songpeng.li@intel.com>
> Subject: [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate
> attributes retrieval
>
> Per spec, the GetVariable() runtime service is not required to populate
> (*Attributes) on output when it fails with EFI_BUFFER_TOO_SMALL.
>
> Therefore we have to fetch the full contents of the TlsCaCertificate
> variable temporarily, just so we can (a) get the current attributes, and
> (b) add EFI_VARIABLE_APPEND_WRITE to them for the subsequent
> SetVariable()
> call.
>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Songpeng Li <songpeng.li@intel.com>
> Reported-by: Songpeng Li <songpeng.li@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1090
> Fixes: b90c335fbbb674470fbf09601cc522bf61564c30
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> Tested via loading the same CA cert .pem file twice in a row, using the
> HII form, first without any pre-existent TlsCaCertificate variable.
>
> Songpeng, can you please test this patch as well, and confirm if it
> works on your end? Thanks!
>
> NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c | 27
> +++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> index 7259c5e82f61..0780b03bbab4 100644
> --- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> +++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> @@ -663,6 +663,7 @@ EnrollX509toVariable (
> EFI_SIGNATURE_LIST *CACert;
> EFI_SIGNATURE_DATA *CACertData;
> VOID *Data;
> + VOID *CurrentData;
> UINTN DataSize;
> UINTN SigDataSize;
> UINT32 Attr;
> @@ -674,6 +675,7 @@ EnrollX509toVariable (
> CACert = NULL;
> CACertData = NULL;
> Data = NULL;
> + CurrentData = NULL;
> Attr = 0;
>
> Status = ReadFileContent (
> @@ -716,11 +718,30 @@ EnrollX509toVariable (
> Status = gRT->GetVariable(
> VariableName,
> &gEfiTlsCaCertificateGuid,
> - &Attr,
> + NULL,
> &DataSize,
> NULL
> );
> if (Status == EFI_BUFFER_TOO_SMALL) {
> + //
> + // Per spec, we have to fetch the variable's contents, even though we're
> + // only interested in the variable's attributes.
> + //
> + CurrentData = AllocatePool (DataSize);
> + if (CurrentData == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + goto ON_EXIT;
> + }
> + Status = gRT->GetVariable(
> + VariableName,
> + &gEfiTlsCaCertificateGuid,
> + &Attr,
> + &DataSize,
> + CurrentData
> + );
> + if (EFI_ERROR (Status)) {
> + goto ON_EXIT;
> + }
> Attr |= EFI_VARIABLE_APPEND_WRITE;
> } else if (Status == EFI_NOT_FOUND) {
> Attr = TLS_AUTH_CONFIG_VAR_BASE_ATTR;
> @@ -751,6 +772,10 @@ ON_EXIT:
> FreePool (Data);
> }
>
> + if (CurrentData != NULL) {
> + FreePool (CurrentData);
> + }
> +
> if (X509Data != NULL) {
> FreePool (X509Data);
> }
> --
> 2.14.1.3.gb7cf6e02401b
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval
2018-08-20 6:29 ` Li, Songpeng
@ 2018-08-21 13:30 ` Laszlo Ersek
2018-08-22 0:36 ` Wu, Jiaxin
1 sibling, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-08-21 13:30 UTC (permalink / raw)
To: Li, Songpeng, edk2-devel-01; +Cc: Fu, Siyuan, Wu, Jiaxin
On 08/20/18 08:29, Li, Songpeng wrote:
> It worked on my end.
>
> Tested-by: Songpeng Li <songpeng.li@intel.com>
Thank you!
Jiaxin, Siyuan, are you guys OK with this patch?
Thanks
Laszlo
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, August 17, 2018 10:36 PM
>> To: edk2-devel-01 <edk2-devel@lists.01.org>
>> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Li,
>> Songpeng <songpeng.li@intel.com>
>> Subject: [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate
>> attributes retrieval
>>
>> Per spec, the GetVariable() runtime service is not required to populate
>> (*Attributes) on output when it fails with EFI_BUFFER_TOO_SMALL.
>>
>> Therefore we have to fetch the full contents of the TlsCaCertificate
>> variable temporarily, just so we can (a) get the current attributes, and
>> (b) add EFI_VARIABLE_APPEND_WRITE to them for the subsequent
>> SetVariable()
>> call.
>>
>> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
>> Cc: Siyuan Fu <siyuan.fu@intel.com>
>> Cc: Songpeng Li <songpeng.li@intel.com>
>> Reported-by: Songpeng Li <songpeng.li@intel.com>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1090
>> Fixes: b90c335fbbb674470fbf09601cc522bf61564c30
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>> Tested via loading the same CA cert .pem file twice in a row, using the
>> HII form, first without any pre-existent TlsCaCertificate variable.
>>
>> Songpeng, can you please test this patch as well, and confirm if it
>> works on your end? Thanks!
>>
>> NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c | 27
>> +++++++++++++++++++-
>> 1 file changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
>> b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
>> index 7259c5e82f61..0780b03bbab4 100644
>> --- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
>> +++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
>> @@ -663,6 +663,7 @@ EnrollX509toVariable (
>> EFI_SIGNATURE_LIST *CACert;
>> EFI_SIGNATURE_DATA *CACertData;
>> VOID *Data;
>> + VOID *CurrentData;
>> UINTN DataSize;
>> UINTN SigDataSize;
>> UINT32 Attr;
>> @@ -674,6 +675,7 @@ EnrollX509toVariable (
>> CACert = NULL;
>> CACertData = NULL;
>> Data = NULL;
>> + CurrentData = NULL;
>> Attr = 0;
>>
>> Status = ReadFileContent (
>> @@ -716,11 +718,30 @@ EnrollX509toVariable (
>> Status = gRT->GetVariable(
>> VariableName,
>> &gEfiTlsCaCertificateGuid,
>> - &Attr,
>> + NULL,
>> &DataSize,
>> NULL
>> );
>> if (Status == EFI_BUFFER_TOO_SMALL) {
>> + //
>> + // Per spec, we have to fetch the variable's contents, even though we're
>> + // only interested in the variable's attributes.
>> + //
>> + CurrentData = AllocatePool (DataSize);
>> + if (CurrentData == NULL) {
>> + Status = EFI_OUT_OF_RESOURCES;
>> + goto ON_EXIT;
>> + }
>> + Status = gRT->GetVariable(
>> + VariableName,
>> + &gEfiTlsCaCertificateGuid,
>> + &Attr,
>> + &DataSize,
>> + CurrentData
>> + );
>> + if (EFI_ERROR (Status)) {
>> + goto ON_EXIT;
>> + }
>> Attr |= EFI_VARIABLE_APPEND_WRITE;
>> } else if (Status == EFI_NOT_FOUND) {
>> Attr = TLS_AUTH_CONFIG_VAR_BASE_ATTR;
>> @@ -751,6 +772,10 @@ ON_EXIT:
>> FreePool (Data);
>> }
>>
>> + if (CurrentData != NULL) {
>> + FreePool (CurrentData);
>> + }
>> +
>> if (X509Data != NULL) {
>> FreePool (X509Data);
>> }
>> --
>> 2.14.1.3.gb7cf6e02401b
>>
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval
2018-08-20 6:29 ` Li, Songpeng
2018-08-21 13:30 ` Laszlo Ersek
@ 2018-08-22 0:36 ` Wu, Jiaxin
1 sibling, 0 replies; 13+ messages in thread
From: Wu, Jiaxin @ 2018-08-22 0:36 UTC (permalink / raw)
To: Li, Songpeng, Laszlo Ersek, edk2-devel-01; +Cc: Fu, Siyuan
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
Thanks,
Jiaxin
> -----Original Message-----
> From: Li, Songpeng
> Sent: Monday, August 20, 2018 2:29 PM
> To: Laszlo Ersek <lersek@redhat.com>; edk2-devel-01 <edk2-
> devel@lists.01.org>
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: RE: [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate
> attributes retrieval
>
> It worked on my end.
>
> Tested-by: Songpeng Li <songpeng.li@intel.com>
>
>
> Thanks & Best Regards,
> Songpeng
>
> > -----Original Message-----
> > From: Laszlo Ersek [mailto:lersek@redhat.com]
> > Sent: Friday, August 17, 2018 10:36 PM
> > To: edk2-devel-01 <edk2-devel@lists.01.org>
> > Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Li,
> > Songpeng <songpeng.li@intel.com>
> > Subject: [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate
> > attributes retrieval
> >
> > Per spec, the GetVariable() runtime service is not required to populate
> > (*Attributes) on output when it fails with EFI_BUFFER_TOO_SMALL.
> >
> > Therefore we have to fetch the full contents of the TlsCaCertificate
> > variable temporarily, just so we can (a) get the current attributes, and
> > (b) add EFI_VARIABLE_APPEND_WRITE to them for the subsequent
> > SetVariable()
> > call.
> >
> > Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > Cc: Songpeng Li <songpeng.li@intel.com>
> > Reported-by: Songpeng Li <songpeng.li@intel.com>
> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1090
> > Fixes: b90c335fbbb674470fbf09601cc522bf61564c30
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> > ---
> >
> > Notes:
> > Tested via loading the same CA cert .pem file twice in a row, using the
> > HII form, first without any pre-existent TlsCaCertificate variable.
> >
> > Songpeng, can you please test this patch as well, and confirm if it
> > works on your end? Thanks!
> >
> > NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c | 27
> > +++++++++++++++++++-
> > 1 file changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> > b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> > index 7259c5e82f61..0780b03bbab4 100644
> > --- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> > +++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> > @@ -663,6 +663,7 @@ EnrollX509toVariable (
> > EFI_SIGNATURE_LIST *CACert;
> > EFI_SIGNATURE_DATA *CACertData;
> > VOID *Data;
> > + VOID *CurrentData;
> > UINTN DataSize;
> > UINTN SigDataSize;
> > UINT32 Attr;
> > @@ -674,6 +675,7 @@ EnrollX509toVariable (
> > CACert = NULL;
> > CACertData = NULL;
> > Data = NULL;
> > + CurrentData = NULL;
> > Attr = 0;
> >
> > Status = ReadFileContent (
> > @@ -716,11 +718,30 @@ EnrollX509toVariable (
> > Status = gRT->GetVariable(
> > VariableName,
> > &gEfiTlsCaCertificateGuid,
> > - &Attr,
> > + NULL,
> > &DataSize,
> > NULL
> > );
> > if (Status == EFI_BUFFER_TOO_SMALL) {
> > + //
> > + // Per spec, we have to fetch the variable's contents, even though
> we're
> > + // only interested in the variable's attributes.
> > + //
> > + CurrentData = AllocatePool (DataSize);
> > + if (CurrentData == NULL) {
> > + Status = EFI_OUT_OF_RESOURCES;
> > + goto ON_EXIT;
> > + }
> > + Status = gRT->GetVariable(
> > + VariableName,
> > + &gEfiTlsCaCertificateGuid,
> > + &Attr,
> > + &DataSize,
> > + CurrentData
> > + );
> > + if (EFI_ERROR (Status)) {
> > + goto ON_EXIT;
> > + }
> > Attr |= EFI_VARIABLE_APPEND_WRITE;
> > } else if (Status == EFI_NOT_FOUND) {
> > Attr = TLS_AUTH_CONFIG_VAR_BASE_ATTR;
> > @@ -751,6 +772,10 @@ ON_EXIT:
> > FreePool (Data);
> > }
> >
> > + if (CurrentData != NULL) {
> > + FreePool (CurrentData);
> > + }
> > +
> > if (X509Data != NULL) {
> > FreePool (X509Data);
> > }
> > --
> > 2.14.1.3.gb7cf6e02401b
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval
2018-08-17 14:35 ` [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval Laszlo Ersek
2018-08-20 6:29 ` Li, Songpeng
@ 2018-08-22 0:31 ` Fu, Siyuan
1 sibling, 0 replies; 13+ messages in thread
From: Fu, Siyuan @ 2018-08-22 0:31 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Wu, Jiaxin, Li, Songpeng
The patch is ok with me.
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, August 17, 2018 10:36 PM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Li,
> Songpeng <songpeng.li@intel.com>
> Subject: [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate
> attributes retrieval
>
> Per spec, the GetVariable() runtime service is not required to populate
> (*Attributes) on output when it fails with EFI_BUFFER_TOO_SMALL.
>
> Therefore we have to fetch the full contents of the TlsCaCertificate
> variable temporarily, just so we can (a) get the current attributes, and
> (b) add EFI_VARIABLE_APPEND_WRITE to them for the subsequent SetVariable()
> call.
>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Songpeng Li <songpeng.li@intel.com>
> Reported-by: Songpeng Li <songpeng.li@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1090
> Fixes: b90c335fbbb674470fbf09601cc522bf61564c30
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> Tested via loading the same CA cert .pem file twice in a row, using
> the
> HII form, first without any pre-existent TlsCaCertificate variable.
>
> Songpeng, can you please test this patch as well, and confirm if it
> works on your end? Thanks!
>
> NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c | 27 +++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> index 7259c5e82f61..0780b03bbab4 100644
> --- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> +++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> @@ -663,6 +663,7 @@ EnrollX509toVariable (
> EFI_SIGNATURE_LIST *CACert;
> EFI_SIGNATURE_DATA *CACertData;
> VOID *Data;
> + VOID *CurrentData;
> UINTN DataSize;
> UINTN SigDataSize;
> UINT32 Attr;
> @@ -674,6 +675,7 @@ EnrollX509toVariable (
> CACert = NULL;
> CACertData = NULL;
> Data = NULL;
> + CurrentData = NULL;
> Attr = 0;
>
> Status = ReadFileContent (
> @@ -716,11 +718,30 @@ EnrollX509toVariable (
> Status = gRT->GetVariable(
> VariableName,
> &gEfiTlsCaCertificateGuid,
> - &Attr,
> + NULL,
> &DataSize,
> NULL
> );
> if (Status == EFI_BUFFER_TOO_SMALL) {
> + //
> + // Per spec, we have to fetch the variable's contents, even though
> we're
> + // only interested in the variable's attributes.
> + //
> + CurrentData = AllocatePool (DataSize);
> + if (CurrentData == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + goto ON_EXIT;
> + }
> + Status = gRT->GetVariable(
> + VariableName,
> + &gEfiTlsCaCertificateGuid,
> + &Attr,
> + &DataSize,
> + CurrentData
> + );
> + if (EFI_ERROR (Status)) {
> + goto ON_EXIT;
> + }
> Attr |= EFI_VARIABLE_APPEND_WRITE;
> } else if (Status == EFI_NOT_FOUND) {
> Attr = TLS_AUTH_CONFIG_VAR_BASE_ATTR;
> @@ -751,6 +772,10 @@ ON_EXIT:
> FreePool (Data);
> }
>
> + if (CurrentData != NULL) {
> + FreePool (CurrentData);
> + }
> +
> if (X509Data != NULL) {
> FreePool (X509Data);
> }
> --
> 2.14.1.3.gb7cf6e02401b
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] UefiCpuPkg/PiSmmCpuDxeSmm: clear exec file mode bits on "PiSmmCpuDxeSmm.c"
2018-08-17 14:35 [PATCH 0/4] {Mde, IntelFramework, Network, UefiCpu}Pkg: roundup for BZs 1074, 1090, 1092 Laszlo Ersek
` (2 preceding siblings ...)
2018-08-17 14:35 ` [PATCH 3/4] NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval Laszlo Ersek
@ 2018-08-17 14:35 ` Laszlo Ersek
2018-08-20 1:31 ` Zeng, Star
2018-08-22 8:41 ` [PATCH 0/4] {Mde, IntelFramework, Network, UefiCpu}Pkg: roundup for BZs 1074, 1090, 1092 Laszlo Ersek
4 siblings, 1 reply; 13+ messages in thread
From: Laszlo Ersek @ 2018-08-17 14:35 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Eric Dong
Commit 241f914975d5 ("UefiCpuPkg/PiSmmCpuDxeSmm: Add support for PCD
PcdPteMemoryEncryptionAddressOrMask", 2017-03-01) unintentionally set the
executable file mode bits on "PiSmmCpuDxeSmm.c"; clear them now.
Cc: Eric Dong <eric.dong@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1092
Fixes: 241f914975d50e34f6da57d1e5ac60eedb5d52de
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 0
1 file changed, 0 insertions(+), 0 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
old mode 100755
new mode 100644
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] UefiCpuPkg/PiSmmCpuDxeSmm: clear exec file mode bits on "PiSmmCpuDxeSmm.c"
2018-08-17 14:35 ` [PATCH 4/4] UefiCpuPkg/PiSmmCpuDxeSmm: clear exec file mode bits on "PiSmmCpuDxeSmm.c" Laszlo Ersek
@ 2018-08-20 1:31 ` Zeng, Star
0 siblings, 0 replies; 13+ messages in thread
From: Zeng, Star @ 2018-08-20 1:31 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Dong, Eric, Zeng, Star
Reviewed-by: Star Zeng <star.zeng@intel.com>
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
Sent: Friday, August 17, 2018 10:36 PM
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Dong, Eric <eric.dong@intel.com>
Subject: [edk2] [PATCH 4/4] UefiCpuPkg/PiSmmCpuDxeSmm: clear exec file mode bits on "PiSmmCpuDxeSmm.c"
Commit 241f914975d5 ("UefiCpuPkg/PiSmmCpuDxeSmm: Add support for PCD PcdPteMemoryEncryptionAddressOrMask", 2017-03-01) unintentionally set the executable file mode bits on "PiSmmCpuDxeSmm.c"; clear them now.
Cc: Eric Dong <eric.dong@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1092
Fixes: 241f914975d50e34f6da57d1e5ac60eedb5d52de
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 0
1 file changed, 0 insertions(+), 0 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
old mode 100755
new mode 100644
--
2.14.1.3.gb7cf6e02401b
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] {Mde, IntelFramework, Network, UefiCpu}Pkg: roundup for BZs 1074, 1090, 1092
2018-08-17 14:35 [PATCH 0/4] {Mde, IntelFramework, Network, UefiCpu}Pkg: roundup for BZs 1074, 1090, 1092 Laszlo Ersek
` (3 preceding siblings ...)
2018-08-17 14:35 ` [PATCH 4/4] UefiCpuPkg/PiSmmCpuDxeSmm: clear exec file mode bits on "PiSmmCpuDxeSmm.c" Laszlo Ersek
@ 2018-08-22 8:41 ` Laszlo Ersek
4 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-08-22 8:41 UTC (permalink / raw)
To: edk2-devel-01
Cc: Ruiyu Ni, Eric Dong, Liming Gao, Jiaxin Wu, Michael D Kinney,
Siyuan Fu, Songpeng Li
On 08/17/18 16:35, Laszlo Ersek wrote:
> Repo: https://github.com/lersek/edk2.git
> Branch: bz_1074_1090_1092_roundup
>
> This series addresses three independent BZs. Patches #1 and #2 belong to
> BZ#1074; patch #3 to BZ#1090; patch #4 to BZ#1092. The patches are
> small, and it's easier for me to send out a round-up series than three
> separate postings.
>
> The testing I've done is listed on the patches in git notes (they won't
> be part of the commit log).
>
> I'd like to highlight here that on patch #3, I'm asking Songpeng for
> additional testing.
Thanks to everyone for the feedback; I've pushed the series: commit
range e0c93c9b7615..0442a9a9f42a.
Laszlo
^ permalink raw reply [flat|nested] 13+ messages in thread