* [PATCH 0/4] {Mde, IntelFramework, Network, UefiCpu}Pkg: roundup for BZs 1074, 1090, 1092
@ 2018-08-17 14:35 Laszlo Ersek
2018-08-17 14:35 ` [PATCH 1/4] MdePkg/UefiLib: don't special-case EFI_FILE_MODE_CREATE in OpenMode Laszlo Ersek
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-08-17 14:35 UTC (permalink / raw)
To: edk2-devel-01
Cc: Eric Dong, Jiaxin Wu, Liming Gao, Michael D Kinney, Ruiyu Ni,
Siyuan Fu, Songpeng Li
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.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Songpeng Li <songpeng.li@intel.com>
Thanks,
Laszlo
Laszlo Ersek (4):
MdePkg/UefiLib: don't special-case EFI_FILE_MODE_CREATE in OpenMode
IntelFrameworkPkg/FrameworkUefiLib: don't special-case
EFI_FILE_MODE_CREATE
NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval
UefiCpuPkg/PiSmmCpuDxeSmm: clear exec file mode bits on
"PiSmmCpuDxeSmm.c"
IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c | 40 +++-----------------
MdePkg/Include/Library/UefiLib.h | 19 ++--------
MdePkg/Library/UefiLib/UefiLib.c | 40 +++-----------------
NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c | 27 ++++++++++++-
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 0
5 files changed, 41 insertions(+), 85 deletions(-)
mode change 100755 => 100644 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply [flat|nested] 13+ messages in thread
* [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
* [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
* [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
* [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 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
* 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
* 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 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-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
* 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 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
end of thread, other threads:[~2018-08-22 8:41 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 20:35 ` Gao, Liming
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
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
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
2018-08-22 8:41 ` [PATCH 0/4] {Mde, IntelFramework, Network, UefiCpu}Pkg: roundup for BZs 1074, 1090, 1092 Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox