* [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL
[not found] <20200327215536.9556-1-michael.kubacki@outlook.com>
@ 2020-03-27 21:55 ` Michael Kubacki
2020-03-30 4:15 ` Wang, Jian J
2020-03-27 21:55 ` [PATCH v3 2/3] MdeModulePkg VariablePei: " Michael Kubacki
2020-03-27 21:55 ` [PATCH v3 3/3] Revert "NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval" Michael Kubacki
2 siblings, 1 reply; 10+ messages in thread
From: Michael Kubacki @ 2020-03-27 21:55 UTC (permalink / raw)
To: devel; +Cc: Bret Barkelew, Liming Gao, Michael D Kinney, Jian J Wang,
Hao A Wu
From: Michael Kubacki <michael.kubacki@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
"Attributes" parameter description states:
"If not NULL, a pointer to the memory location to return the
attributes bitmask for the variable. See 'Related Definitions.'
If not NULL, then Attributes is set on output both when
EFI_SUCCESS and when EFI_BUFFER_TOO_SMALL is returned."
The attributes were previously only returned from the implementation
in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS or
EFI_BUFFER_TOO_SMALL according to spec.
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 10 +++++++---
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 10 ++++++----
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index d23aea4bc712..1e71fc642c76 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -18,6 +18,8 @@
Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP<BR>
+Copyright (c) Microsoft Corporation.<BR>
+
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -2379,9 +2381,6 @@ VariableServiceGetVariable (
}
CopyMem (Data, GetVariableDataPtr (Variable.CurrPtr, mVariableModuleGlobal->VariableGlobal.AuthFormat), VarDataSize);
- if (Attributes != NULL) {
- *Attributes = Variable.CurrPtr->Attributes;
- }
*DataSize = VarDataSize;
UpdateVariableInfo (VariableName, VendorGuid, Variable.Volatile, TRUE, FALSE, FALSE, FALSE, &gVariableInfo);
@@ -2395,6 +2394,11 @@ VariableServiceGetVariable (
}
Done:
+ if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
+ if (Attributes != NULL && Variable.CurrPtr != NULL) {
+ *Attributes = Variable.CurrPtr->Attributes;
+ }
+ }
ReleaseLockOnlyAtBootTime (&mVariableModuleGlobal->VariableGlobal.VariableServicesLock);
return Status;
}
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 2cf0ed32ae55..ca833fb0244d 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -14,6 +14,7 @@
InitCommunicateBuffer() is really function to check the variable data size.
Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -642,10 +643,6 @@ FindVariableInRuntimeCache (
}
CopyMem (Data, GetVariableDataPtr (RtPtrTrack.CurrPtr, mVariableAuthFormat), TempDataSize);
- if (Attributes != NULL) {
- *Attributes = RtPtrTrack.CurrPtr->Attributes;
- }
-
*DataSize = TempDataSize;
UpdateVariableInfo (VariableName, VendorGuid, RtPtrTrack.Volatile, TRUE, FALSE, FALSE, TRUE, &mVariableInfo);
@@ -661,6 +658,11 @@ FindVariableInRuntimeCache (
}
Done:
+ if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
+ if (Attributes != NULL && RtPtrTrack.CurrPtr != NULL) {
+ *Attributes = RtPtrTrack.CurrPtr->Attributes;
+ }
+ }
mVariableRuntimeCacheReadLock = FALSE;
return Status;
--
2.16.3.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/3] MdeModulePkg VariablePei: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL
[not found] <20200327215536.9556-1-michael.kubacki@outlook.com>
2020-03-27 21:55 ` [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL Michael Kubacki
@ 2020-03-27 21:55 ` Michael Kubacki
2020-03-30 4:19 ` Wang, Jian J
2020-03-27 21:55 ` [PATCH v3 3/3] Revert "NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval" Michael Kubacki
2 siblings, 1 reply; 10+ messages in thread
From: Michael Kubacki @ 2020-03-27 21:55 UTC (permalink / raw)
To: devel; +Cc: Bret Barkelew, Liming Gao, Michael D Kinney, Jian J Wang,
Hao A Wu
From: Michael Kubacki <michael.kubacki@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
This commit makes the behavior for PeiGetVariable() match the following
specification-defined behavior. It is now consistent with the DXE/SMM
variable driver implementation.
The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
"Attributes" parameter description states:
"If not NULL, a pointer to the memory location to return the
attributes bitmask for the variable. See 'Related Definitions.'
If not NULL, then Attributes is set on output both when
EFI_SUCCESS and when EFI_BUFFER_TOO_SMALL is returned."
The attributes were previously only returned from the implementation
in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS or
EFI_BUFFER_TOO_SMALL according to spec.
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
MdeModulePkg/Universal/Variable/Pei/Variable.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c b/MdeModulePkg/Universal/Variable/Pei/Variable.c
index f61465fc3045..f420b58165b7 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
@@ -3,6 +3,7 @@
PEI ReadOnly Varaiable2 PPI. These services operates the non volatile storage space.
Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -1047,17 +1048,17 @@ PeiGetVariable (
}
GetVariableNameOrData (&StoreInfo, GetVariableDataPtr (Variable.CurrPtr, VariableHeader, StoreInfo.AuthFlag), VarDataSize, Data);
-
- if (Attributes != NULL) {
- *Attributes = VariableHeader->Attributes;
- }
-
- *DataSize = VarDataSize;
- return EFI_SUCCESS;
+ Status = EFI_SUCCESS;
} else {
- *DataSize = VarDataSize;
- return EFI_BUFFER_TOO_SMALL;
+ Status = EFI_BUFFER_TOO_SMALL;
}
+
+ if (Attributes != NULL) {
+ *Attributes = VariableHeader->Attributes;
+ }
+ *DataSize = VarDataSize;
+
+ return Status;
}
/**
--
2.16.3.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 3/3] Revert "NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval"
[not found] <20200327215536.9556-1-michael.kubacki@outlook.com>
2020-03-27 21:55 ` [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL Michael Kubacki
2020-03-27 21:55 ` [PATCH v3 2/3] MdeModulePkg VariablePei: " Michael Kubacki
@ 2020-03-27 21:55 ` Michael Kubacki
2020-04-01 0:24 ` Wu, Jiaxin
2 siblings, 1 reply; 10+ messages in thread
From: Michael Kubacki @ 2020-03-27 21:55 UTC (permalink / raw)
To: devel; +Cc: Laszlo Ersek, Siyuan Fu, Maciej Rabeda, Jiaxin Wu
From: Michael Kubacki <michael.kubacki@microsoft.com>
This reverts commit 6896efdec2709e530b23c688cf0f31706709a0c5.
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
GetVariable() now returns attributes when it fails with
EFI_BUFFER_TOO_SMALL. Therefore, commit 6896efdec270 is
reverted since it is no longer relevant.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Bret Barkelew <bret.barkelew@microsoft.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c | 27 +-------------------
1 file changed, 1 insertion(+), 26 deletions(-)
diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
index 715bc3a0a941..2481d1098fa3 100644
--- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
+++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
@@ -657,7 +657,6 @@ EnrollX509toVariable (
EFI_SIGNATURE_LIST *CACert;
EFI_SIGNATURE_DATA *CACertData;
VOID *Data;
- VOID *CurrentData;
UINTN DataSize;
UINTN SigDataSize;
UINT32 Attr;
@@ -669,7 +668,6 @@ EnrollX509toVariable (
CACert = NULL;
CACertData = NULL;
Data = NULL;
- CurrentData = NULL;
Attr = 0;
Status = ReadFileContent (
@@ -712,30 +710,11 @@ EnrollX509toVariable (
Status = gRT->GetVariable(
VariableName,
&gEfiTlsCaCertificateGuid,
- NULL,
+ &Attr,
&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;
@@ -766,10 +745,6 @@ ON_EXIT:
FreePool (Data);
}
- if (CurrentData != NULL) {
- FreePool (CurrentData);
- }
-
if (X509Data != NULL) {
FreePool (X509Data);
}
--
2.16.3.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL
2020-03-27 21:55 ` [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL Michael Kubacki
@ 2020-03-30 4:15 ` Wang, Jian J
2020-03-31 8:18 ` [edk2-devel] " Guomin Jiang
0 siblings, 1 reply; 10+ messages in thread
From: Wang, Jian J @ 2020-03-30 4:15 UTC (permalink / raw)
To: michael.kubacki@outlook.com, devel@edk2.groups.io
Cc: Bret Barkelew, Gao, Liming, Kinney, Michael D, Wu, Hao A
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Regards,
Jian
> -----Original Message-----
> From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
> Sent: Saturday, March 28, 2020 5:56 AM
> To: devel@edk2.groups.io
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> Subject: [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if
> EFI_BUFFER_TOO_SMALL
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
>
> The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
> "Attributes" parameter description states:
>
> "If not NULL, a pointer to the memory location to return the
> attributes bitmask for the variable. See 'Related Definitions.'
> If not NULL, then Attributes is set on output both when
> EFI_SUCCESS and when EFI_BUFFER_TOO_SMALL is returned."
>
> The attributes were previously only returned from the implementation
> in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS or
> EFI_BUFFER_TOO_SMALL according to spec.
>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 10
> +++++++---
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c |
> 10 ++++++----
> 2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index d23aea4bc712..1e71fc642c76 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -18,6 +18,8 @@
>
> Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>
> (C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> +
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -2379,9 +2381,6 @@ VariableServiceGetVariable (
> }
>
> CopyMem (Data, GetVariableDataPtr (Variable.CurrPtr,
> mVariableModuleGlobal->VariableGlobal.AuthFormat), VarDataSize);
> - if (Attributes != NULL) {
> - *Attributes = Variable.CurrPtr->Attributes;
> - }
>
> *DataSize = VarDataSize;
> UpdateVariableInfo (VariableName, VendorGuid, Variable.Volatile, TRUE,
> FALSE, FALSE, FALSE, &gVariableInfo);
> @@ -2395,6 +2394,11 @@ VariableServiceGetVariable (
> }
>
> Done:
> + if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
> + if (Attributes != NULL && Variable.CurrPtr != NULL) {
> + *Attributes = Variable.CurrPtr->Attributes;
> + }
> + }
> ReleaseLockOnlyAtBootTime (&mVariableModuleGlobal-
> >VariableGlobal.VariableServicesLock);
> return Status;
> }
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> index 2cf0ed32ae55..ca833fb0244d 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> @@ -14,6 +14,7 @@
> InitCommunicateBuffer() is really function to check the variable data size.
>
> Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -642,10 +643,6 @@ FindVariableInRuntimeCache (
> }
>
> CopyMem (Data, GetVariableDataPtr (RtPtrTrack.CurrPtr,
> mVariableAuthFormat), TempDataSize);
> - if (Attributes != NULL) {
> - *Attributes = RtPtrTrack.CurrPtr->Attributes;
> - }
> -
> *DataSize = TempDataSize;
>
> UpdateVariableInfo (VariableName, VendorGuid, RtPtrTrack.Volatile, TRUE,
> FALSE, FALSE, TRUE, &mVariableInfo);
> @@ -661,6 +658,11 @@ FindVariableInRuntimeCache (
> }
>
> Done:
> + if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
> + if (Attributes != NULL && RtPtrTrack.CurrPtr != NULL) {
> + *Attributes = RtPtrTrack.CurrPtr->Attributes;
> + }
> + }
> mVariableRuntimeCacheReadLock = FALSE;
>
> return Status;
> --
> 2.16.3.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] MdeModulePkg VariablePei: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL
2020-03-27 21:55 ` [PATCH v3 2/3] MdeModulePkg VariablePei: " Michael Kubacki
@ 2020-03-30 4:19 ` Wang, Jian J
2020-03-31 8:04 ` [edk2-devel] " Guomin Jiang
0 siblings, 1 reply; 10+ messages in thread
From: Wang, Jian J @ 2020-03-30 4:19 UTC (permalink / raw)
To: michael.kubacki@outlook.com, devel@edk2.groups.io
Cc: Bret Barkelew, Gao, Liming, Kinney, Michael D, Wu, Hao A
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Regards,
Jian
> -----Original Message-----
> From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
> Sent: Saturday, March 28, 2020 5:56 AM
> To: devel@edk2.groups.io
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> Subject: [PATCH v3 2/3] MdeModulePkg VariablePei: Return GetVariable() attr if
> EFI_BUFFER_TOO_SMALL
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
>
> This commit makes the behavior for PeiGetVariable() match the following
> specification-defined behavior. It is now consistent with the DXE/SMM
> variable driver implementation.
>
> The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
> "Attributes" parameter description states:
>
> "If not NULL, a pointer to the memory location to return the
> attributes bitmask for the variable. See 'Related Definitions.'
> If not NULL, then Attributes is set on output both when
> EFI_SUCCESS and when EFI_BUFFER_TOO_SMALL is returned."
>
> The attributes were previously only returned from the implementation
> in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS or
> EFI_BUFFER_TOO_SMALL according to spec.
>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
> MdeModulePkg/Universal/Variable/Pei/Variable.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c
> b/MdeModulePkg/Universal/Variable/Pei/Variable.c
> index f61465fc3045..f420b58165b7 100644
> --- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
> @@ -3,6 +3,7 @@
> PEI ReadOnly Varaiable2 PPI. These services operates the non volatile storage
> space.
>
> Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -1047,17 +1048,17 @@ PeiGetVariable (
> }
>
> GetVariableNameOrData (&StoreInfo, GetVariableDataPtr (Variable.CurrPtr,
> VariableHeader, StoreInfo.AuthFlag), VarDataSize, Data);
> -
> - if (Attributes != NULL) {
> - *Attributes = VariableHeader->Attributes;
> - }
> -
> - *DataSize = VarDataSize;
> - return EFI_SUCCESS;
> + Status = EFI_SUCCESS;
> } else {
> - *DataSize = VarDataSize;
> - return EFI_BUFFER_TOO_SMALL;
> + Status = EFI_BUFFER_TOO_SMALL;
> }
> +
> + if (Attributes != NULL) {
> + *Attributes = VariableHeader->Attributes;
> + }
> + *DataSize = VarDataSize;
> +
> + return Status;
> }
>
> /**
> --
> 2.16.3.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v3 2/3] MdeModulePkg VariablePei: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL
2020-03-30 4:19 ` Wang, Jian J
@ 2020-03-31 8:04 ` Guomin Jiang
0 siblings, 0 replies; 10+ messages in thread
From: Guomin Jiang @ 2020-03-31 8:04 UTC (permalink / raw)
To: devel@edk2.groups.io, Wang, Jian J, michael.kubacki@outlook.com
Cc: Bret Barkelew, Gao, Liming, Kinney, Michael D, Wu, Hao A
Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Wang, Jian J
> Sent: Monday, March 30, 2020 12:19 PM
> To: michael.kubacki@outlook.com; devel@edk2.groups.io
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Wu, Hao A <hao.a.wu@intel.com>
> Subject: Re: [edk2-devel] [PATCH v3 2/3] MdeModulePkg VariablePei:
> Return GetVariable() attr if EFI_BUFFER_TOO_SMALL
>
> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
>
> Regards,
> Jian
>
> > -----Original Message-----
> > From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
> > Sent: Saturday, March 28, 2020 5:56 AM
> > To: devel@edk2.groups.io
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
> > <liming.gao@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> > Wu, Hao A <hao.a.wu@intel.com>
> > Subject: [PATCH v3 2/3] MdeModulePkg VariablePei: Return GetVariable()
> > attr if EFI_BUFFER_TOO_SMALL
> >
> > From: Michael Kubacki <michael.kubacki@microsoft.com>
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
> >
> > This commit makes the behavior for PeiGetVariable() match the
> > following specification-defined behavior. It is now consistent with
> > the DXE/SMM variable driver implementation.
> >
> > The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
> > "Attributes" parameter description states:
> >
> > "If not NULL, a pointer to the memory location to return the
> > attributes bitmask for the variable. See 'Related Definitions.'
> > If not NULL, then Attributes is set on output both when EFI_SUCCESS
> > and when EFI_BUFFER_TOO_SMALL is returned."
> >
> > The attributes were previously only returned from the implementation
> > in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS or
> > EFI_BUFFER_TOO_SMALL according to spec.
> >
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> > ---
> > MdeModulePkg/Universal/Variable/Pei/Variable.c | 19
> > ++++++++++---------
> > 1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c
> > b/MdeModulePkg/Universal/Variable/Pei/Variable.c
> > index f61465fc3045..f420b58165b7 100644
> > --- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
> > +++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
> > @@ -3,6 +3,7 @@
> > PEI ReadOnly Varaiable2 PPI. These services operates the non
> > volatile storage space.
> >
> > Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > reserved.<BR>
> > +Copyright (c) Microsoft Corporation.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -1047,17 +1048,17 @@ PeiGetVariable (
> > }
> >
> > GetVariableNameOrData (&StoreInfo, GetVariableDataPtr
> > (Variable.CurrPtr, VariableHeader, StoreInfo.AuthFlag), VarDataSize,
> > Data);
> > -
> > - if (Attributes != NULL) {
> > - *Attributes = VariableHeader->Attributes;
> > - }
> > -
> > - *DataSize = VarDataSize;
> > - return EFI_SUCCESS;
> > + Status = EFI_SUCCESS;
> > } else {
> > - *DataSize = VarDataSize;
> > - return EFI_BUFFER_TOO_SMALL;
> > + Status = EFI_BUFFER_TOO_SMALL;
> > }
> > +
> > + if (Attributes != NULL) {
> > + *Attributes = VariableHeader->Attributes; } *DataSize =
> > + VarDataSize;
> > +
> > + return Status;
> > }
> >
> > /**
> > --
> > 2.16.3.windows.1
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL
2020-03-30 4:15 ` Wang, Jian J
@ 2020-03-31 8:18 ` Guomin Jiang
2020-03-31 17:11 ` Michael Kubacki
0 siblings, 1 reply; 10+ messages in thread
From: Guomin Jiang @ 2020-03-31 8:18 UTC (permalink / raw)
To: devel@edk2.groups.io, Wang, Jian J, michael.kubacki@outlook.com
Cc: Bret Barkelew, Gao, Liming, Kinney, Michael D, Wu, Hao A
There is a spell error in the comments of VariableServiceGetVariable() in Variable.c.
- @return EFI_BUFFER_TO_SMALL DataSize is too small for the result.
+ @return EFI_BUFFER_TOO_SMALL DataSize is too small for the result.
Need create new bugs for it or fix in this comment directly?
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Wang, Jian J
> Sent: Monday, March 30, 2020 12:16 PM
> To: michael.kubacki@outlook.com; devel@edk2.groups.io
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Wu, Hao A <hao.a.wu@intel.com>
> Subject: Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return
> GetVariable() attr if EFI_BUFFER_TOO_SMALL
>
>
> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
>
> Regards,
> Jian
>
> > -----Original Message-----
> > From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
> > Sent: Saturday, March 28, 2020 5:56 AM
> > To: devel@edk2.groups.io
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
> > <liming.gao@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> > Wu, Hao A <hao.a.wu@intel.com>
> > Subject: [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable()
> > attr if EFI_BUFFER_TOO_SMALL
> >
> > From: Michael Kubacki <michael.kubacki@microsoft.com>
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
> >
> > The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
> > "Attributes" parameter description states:
> >
> > "If not NULL, a pointer to the memory location to return the
> > attributes bitmask for the variable. See 'Related Definitions.'
> > If not NULL, then Attributes is set on output both when EFI_SUCCESS
> > and when EFI_BUFFER_TOO_SMALL is returned."
> >
> > The attributes were previously only returned from the implementation
> > in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS or
> > EFI_BUFFER_TOO_SMALL according to spec.
> >
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> > ---
> > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 10
> > +++++++---
> >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
> c |
> > 10 ++++++----
> > 2 files changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > index d23aea4bc712..1e71fc642c76 100644
> > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > @@ -18,6 +18,8 @@
> >
> > Copyright (c) 2006 - 2020, Intel Corporation. All rights
> > reserved.<BR>
> > (C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP<BR>
> > +Copyright (c) Microsoft Corporation.<BR>
> > +
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -2379,9 +2381,6 @@ VariableServiceGetVariable (
> > }
> >
> > CopyMem (Data, GetVariableDataPtr (Variable.CurrPtr,
> > mVariableModuleGlobal->VariableGlobal.AuthFormat), VarDataSize);
> > - if (Attributes != NULL) {
> > - *Attributes = Variable.CurrPtr->Attributes;
> > - }
> >
> > *DataSize = VarDataSize;
> > UpdateVariableInfo (VariableName, VendorGuid, Variable.Volatile,
> > TRUE, FALSE, FALSE, FALSE, &gVariableInfo); @@ -2395,6 +2394,11 @@
> > VariableServiceGetVariable (
> > }
> >
> > Done:
> > + if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
> > + if (Attributes != NULL && Variable.CurrPtr != NULL) {
> > + *Attributes = Variable.CurrPtr->Attributes;
> > + }
> > + }
> > ReleaseLockOnlyAtBootTime (&mVariableModuleGlobal-
> > >VariableGlobal.VariableServicesLock);
> > return Status;
> > }
> > diff --git
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> > index 2cf0ed32ae55..ca833fb0244d 100644
> > ---
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> > +++
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> > @@ -14,6 +14,7 @@
> > InitCommunicateBuffer() is really function to check the variable data size.
> >
> > Copyright (c) 2010 - 2019, Intel Corporation. All rights
> > reserved.<BR>
> > +Copyright (c) Microsoft Corporation.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -642,10 +643,6 @@ FindVariableInRuntimeCache (
> > }
> >
> > CopyMem (Data, GetVariableDataPtr (RtPtrTrack.CurrPtr,
> > mVariableAuthFormat), TempDataSize);
> > - if (Attributes != NULL) {
> > - *Attributes = RtPtrTrack.CurrPtr->Attributes;
> > - }
> > -
> > *DataSize = TempDataSize;
> >
> > UpdateVariableInfo (VariableName, VendorGuid,
> > RtPtrTrack.Volatile, TRUE, FALSE, FALSE, TRUE, &mVariableInfo); @@
> > -661,6 +658,11 @@ FindVariableInRuntimeCache (
> > }
> >
> > Done:
> > + if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
> > + if (Attributes != NULL && RtPtrTrack.CurrPtr != NULL) {
> > + *Attributes = RtPtrTrack.CurrPtr->Attributes;
> > + }
> > + }
> > mVariableRuntimeCacheReadLock = FALSE;
> >
> > return Status;
> > --
> > 2.16.3.windows.1
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL
2020-03-31 8:18 ` [edk2-devel] " Guomin Jiang
@ 2020-03-31 17:11 ` Michael Kubacki
2020-04-01 2:37 ` Guomin Jiang
0 siblings, 1 reply; 10+ messages in thread
From: Michael Kubacki @ 2020-03-31 17:11 UTC (permalink / raw)
To: Jiang, Guomin, devel@edk2.groups.io, Wang, Jian J
Cc: Bret Barkelew, Gao, Liming, Kinney, Michael D, Wu, Hao A
That has been spelled incorrectly for about 9 years. The file (like many
others) also has other spelling errors such as the following. I suggest
this be fixed in a separate commit/series focused on fixing spelling errors.
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -102,7 +102,7 @@ AUTH_VAR_LIB_CONTEXT_OUT mAuthContextOut;
This function writes data to the FWH at the correct LBA even if the LBAs
are fragmented.
- @param Global Pointer to VARAIBLE_GLOBAL structure.
+ @param Global Pointer to VARIABLE_GLOBAL structure.
@param Volatile Point out the Variable is Volatile or
Non-Volatile.
@param SetByIndex TRUE if target pointer is given as index.
FALSE if target pointer is absolute.
@@ -504,7 +504,7 @@ InitializeVariableQuota (
@return EFI_SUCCESS Reclaim operation has finished
successfully.
@return EFI_OUT_OF_RESOURCES No enough memory resources or
variable space.
- @return Others Unexpect error happened during
reclaim operation.
+ @return Others Unexpected error happened during
reclaim operation.
**/
EFI_STATUS
@@ -2561,7 +2561,7 @@ VariableServiceSetVariable (
}
//
- // Check for reserverd bit in variable attribute.
+ // Check for reserved bit in variable attribute.
// EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated but we
still allow
// the delete operation of common authenticated variable at user
physical presence.
// So leave EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS attribute check
to AuthVariableLib
@@ -3381,7 +3381,7 @@ ConvertNormalVarStorageToAuthVarStorage (
VARIABLE_HEADER *StartPtr;
UINT8 *NextPtr;
VARIABLE_HEADER *EndPtr;
- UINTN AuthVarStroageSize;
+ UINTN AuthVarStorageSize;
AUTHENTICATED_VARIABLE_HEADER *AuthStartPtr;
VARIABLE_STORE_HEADER *AuthVarStorage;
On 3/31/2020 1:18 AM, Jiang, Guomin wrote:
> There is a spell error in the comments of VariableServiceGetVariable() in Variable.c.
> - @return EFI_BUFFER_TO_SMALL DataSize is too small for the result.
> + @return EFI_BUFFER_TOO_SMALL DataSize is too small for the result.
>
> Need create new bugs for it or fix in this comment directly?
>
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Wang, Jian J
>> Sent: Monday, March 30, 2020 12:16 PM
>> To: michael.kubacki@outlook.com; devel@edk2.groups.io
>> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
>> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
>> Wu, Hao A <hao.a.wu@intel.com>
>> Subject: Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return
>> GetVariable() attr if EFI_BUFFER_TOO_SMALL
>>
>>
>> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
>>
>> Regards,
>> Jian
>>
>>> -----Original Message-----
>>> From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
>>> Sent: Saturday, March 28, 2020 5:56 AM
>>> To: devel@edk2.groups.io
>>> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
>>> <liming.gao@intel.com>; Kinney, Michael D
>>> <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
>>> Wu, Hao A <hao.a.wu@intel.com>
>>> Subject: [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable()
>>> attr if EFI_BUFFER_TOO_SMALL
>>>
>>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>>
>>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
>>>
>>> The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
>>> "Attributes" parameter description states:
>>>
>>> "If not NULL, a pointer to the memory location to return the
>>> attributes bitmask for the variable. See 'Related Definitions.'
>>> If not NULL, then Attributes is set on output both when EFI_SUCCESS
>>> and when EFI_BUFFER_TOO_SMALL is returned."
>>>
>>> The attributes were previously only returned from the implementation
>>> in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS or
>>> EFI_BUFFER_TOO_SMALL according to spec.
>>>
>>> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>> Cc: Hao A Wu <hao.a.wu@intel.com>
>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>>> ---
>>> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 10
>>> +++++++---
>>>
>> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
>> c |
>>> 10 ++++++----
>>> 2 files changed, 13 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>>> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>>> index d23aea4bc712..1e71fc642c76 100644
>>> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>>> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>>> @@ -18,6 +18,8 @@
>>>
>>> Copyright (c) 2006 - 2020, Intel Corporation. All rights
>>> reserved.<BR>
>>> (C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP<BR>
>>> +Copyright (c) Microsoft Corporation.<BR>
>>> +
>>> SPDX-License-Identifier: BSD-2-Clause-Patent
>>>
>>> **/
>>> @@ -2379,9 +2381,6 @@ VariableServiceGetVariable (
>>> }
>>>
>>> CopyMem (Data, GetVariableDataPtr (Variable.CurrPtr,
>>> mVariableModuleGlobal->VariableGlobal.AuthFormat), VarDataSize);
>>> - if (Attributes != NULL) {
>>> - *Attributes = Variable.CurrPtr->Attributes;
>>> - }
>>>
>>> *DataSize = VarDataSize;
>>> UpdateVariableInfo (VariableName, VendorGuid, Variable.Volatile,
>>> TRUE, FALSE, FALSE, FALSE, &gVariableInfo); @@ -2395,6 +2394,11 @@
>>> VariableServiceGetVariable (
>>> }
>>>
>>> Done:
>>> + if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
>>> + if (Attributes != NULL && Variable.CurrPtr != NULL) {
>>> + *Attributes = Variable.CurrPtr->Attributes;
>>> + }
>>> + }
>>> ReleaseLockOnlyAtBootTime (&mVariableModuleGlobal-
>>>> VariableGlobal.VariableServicesLock);
>>> return Status;
>>> }
>>> diff --git
>>>
>> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
>> e.c
>>>
>> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
>> e.c
>>> index 2cf0ed32ae55..ca833fb0244d 100644
>>> ---
>>>
>> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
>> e.c
>>> +++
>>>
>> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
>> e.c
>>> @@ -14,6 +14,7 @@
>>> InitCommunicateBuffer() is really function to check the variable data size.
>>>
>>> Copyright (c) 2010 - 2019, Intel Corporation. All rights
>>> reserved.<BR>
>>> +Copyright (c) Microsoft Corporation.<BR>
>>> SPDX-License-Identifier: BSD-2-Clause-Patent
>>>
>>> **/
>>> @@ -642,10 +643,6 @@ FindVariableInRuntimeCache (
>>> }
>>>
>>> CopyMem (Data, GetVariableDataPtr (RtPtrTrack.CurrPtr,
>>> mVariableAuthFormat), TempDataSize);
>>> - if (Attributes != NULL) {
>>> - *Attributes = RtPtrTrack.CurrPtr->Attributes;
>>> - }
>>> -
>>> *DataSize = TempDataSize;
>>>
>>> UpdateVariableInfo (VariableName, VendorGuid,
>>> RtPtrTrack.Volatile, TRUE, FALSE, FALSE, TRUE, &mVariableInfo); @@
>>> -661,6 +658,11 @@ FindVariableInRuntimeCache (
>>> }
>>>
>>> Done:
>>> + if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
>>> + if (Attributes != NULL && RtPtrTrack.CurrPtr != NULL) {
>>> + *Attributes = RtPtrTrack.CurrPtr->Attributes;
>>> + }
>>> + }
>>> mVariableRuntimeCacheReadLock = FALSE;
>>>
>>> return Status;
>>> --
>>> 2.16.3.windows.1
>>
>>
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/3] Revert "NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval"
2020-03-27 21:55 ` [PATCH v3 3/3] Revert "NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval" Michael Kubacki
@ 2020-04-01 0:24 ` Wu, Jiaxin
0 siblings, 0 replies; 10+ messages in thread
From: Wu, Jiaxin @ 2020-04-01 0:24 UTC (permalink / raw)
To: michael.kubacki@outlook.com, devel@edk2.groups.io
Cc: Laszlo Ersek, Fu, Siyuan, Maciej Rabeda
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
> -----Original Message-----
> From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
> Sent: Saturday, March 28, 2020 5:56 AM
> To: devel@edk2.groups.io
> Cc: Laszlo Ersek <lersek@redhat.com>; Fu, Siyuan <siyuan.fu@intel.com>;
> Maciej Rabeda <maciej.rabeda@linux.intel.com>; Wu, Jiaxin
> <jiaxin.wu@intel.com>
> Subject: [PATCH v3 3/3] Revert "NetworkPkg/TlsAuthConfigDxe: fix
> TlsCaCertificate attributes retrieval"
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> This reverts commit 6896efdec2709e530b23c688cf0f31706709a0c5.
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
>
> GetVariable() now returns attributes when it fails with
> EFI_BUFFER_TOO_SMALL. Therefore, commit 6896efdec270 is
> reverted since it is no longer relevant.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> Reviewed-by: Bret Barkelew <bret.barkelew@microsoft.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c | 27 +-------------------
> 1 file changed, 1 insertion(+), 26 deletions(-)
>
> diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> index 715bc3a0a941..2481d1098fa3 100644
> --- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> +++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigImpl.c
> @@ -657,7 +657,6 @@ EnrollX509toVariable (
> EFI_SIGNATURE_LIST *CACert;
> EFI_SIGNATURE_DATA *CACertData;
> VOID *Data;
> - VOID *CurrentData;
> UINTN DataSize;
> UINTN SigDataSize;
> UINT32 Attr;
> @@ -669,7 +668,6 @@ EnrollX509toVariable (
> CACert = NULL;
> CACertData = NULL;
> Data = NULL;
> - CurrentData = NULL;
> Attr = 0;
>
> Status = ReadFileContent (
> @@ -712,30 +710,11 @@ EnrollX509toVariable (
> Status = gRT->GetVariable(
> VariableName,
> &gEfiTlsCaCertificateGuid,
> - NULL,
> + &Attr,
> &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;
> @@ -766,10 +745,6 @@ ON_EXIT:
> FreePool (Data);
> }
>
> - if (CurrentData != NULL) {
> - FreePool (CurrentData);
> - }
> -
> if (X509Data != NULL) {
> FreePool (X509Data);
> }
> --
> 2.16.3.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL
2020-03-31 17:11 ` Michael Kubacki
@ 2020-04-01 2:37 ` Guomin Jiang
0 siblings, 0 replies; 10+ messages in thread
From: Guomin Jiang @ 2020-04-01 2:37 UTC (permalink / raw)
To: Michael Kubacki, devel@edk2.groups.io, Wang, Jian J
Cc: Bret Barkelew, Gao, Liming, Kinney, Michael D, Wu, Hao A
It is ok, I have no others confusion.
Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
> -----Original Message-----
> From: Michael Kubacki [mailto:michael.kubacki@outlook.com]
> Sent: Wednesday, April 1, 2020 1:12 AM
> To: Jiang, Guomin <guomin.jiang@intel.com>; devel@edk2.groups.io; Wang,
> Jian J <jian.j.wang@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Wu, Hao A <hao.a.wu@intel.com>
> Subject: Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return
> GetVariable() attr if EFI_BUFFER_TOO_SMALL
>
> That has been spelled incorrectly for about 9 years. The file (like many
> others) also has other spelling errors such as the following. I suggest this be
> fixed in a separate commit/series focused on fixing spelling errors.
>
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -102,7 +102,7 @@ AUTH_VAR_LIB_CONTEXT_OUT mAuthContextOut;
> This function writes data to the FWH at the correct LBA even if the LBAs
> are fragmented.
>
> - @param Global Pointer to VARAIBLE_GLOBAL structure.
> + @param Global Pointer to VARIABLE_GLOBAL structure.
> @param Volatile Point out the Variable is Volatile or
> Non-Volatile.
> @param SetByIndex TRUE if target pointer is given as index.
> FALSE if target pointer is absolute.
> @@ -504,7 +504,7 @@ InitializeVariableQuota (
>
> @return EFI_SUCCESS Reclaim operation has finished
> successfully.
> @return EFI_OUT_OF_RESOURCES No enough memory resources or
> variable space.
> - @return Others Unexpect error happened during
> reclaim operation.
> + @return Others Unexpected error happened during
> reclaim operation.
>
> **/
> EFI_STATUS
> @@ -2561,7 +2561,7 @@ VariableServiceSetVariable (
> }
>
> //
> - // Check for reserverd bit in variable attribute.
> + // Check for reserved bit in variable attribute.
> // EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated but we
> still allow
> // the delete operation of common authenticated variable at user physical
> presence.
> // So leave EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS attribute
> check to AuthVariableLib @@ -3381,7 +3381,7 @@
> ConvertNormalVarStorageToAuthVarStorage (
> VARIABLE_HEADER *StartPtr;
> UINT8 *NextPtr;
> VARIABLE_HEADER *EndPtr;
> - UINTN AuthVarStroageSize;
> + UINTN AuthVarStorageSize;
> AUTHENTICATED_VARIABLE_HEADER *AuthStartPtr;
> VARIABLE_STORE_HEADER *AuthVarStorage;
>
>
> On 3/31/2020 1:18 AM, Jiang, Guomin wrote:
> > There is a spell error in the comments of VariableServiceGetVariable() in
> Variable.c.
> > - @return EFI_BUFFER_TO_SMALL DataSize is too small for the result.
> > + @return EFI_BUFFER_TOO_SMALL DataSize is too small for the result.
> >
> > Need create new bugs for it or fix in this comment directly?
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> >> Wang, Jian J
> >> Sent: Monday, March 30, 2020 12:16 PM
> >> To: michael.kubacki@outlook.com; devel@edk2.groups.io
> >> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
> >> <liming.gao@intel.com>; Kinney, Michael D
> >> <michael.d.kinney@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
> >> Subject: Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable:
> >> Return
> >> GetVariable() attr if EFI_BUFFER_TOO_SMALL
> >>
> >>
> >> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> >>
> >> Regards,
> >> Jian
> >>
> >>> -----Original Message-----
> >>> From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
> >>> Sent: Saturday, March 28, 2020 5:56 AM
> >>> To: devel@edk2.groups.io
> >>> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
> >>> <liming.gao@intel.com>; Kinney, Michael D
> >>> <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> >>> Wu, Hao A <hao.a.wu@intel.com>
> >>> Subject: [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable()
> >>> attr if EFI_BUFFER_TOO_SMALL
> >>>
> >>> From: Michael Kubacki <michael.kubacki@microsoft.com>
> >>>
> >>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062
> >>>
> >>> The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
> >>> "Attributes" parameter description states:
> >>>
> >>> "If not NULL, a pointer to the memory location to return the
> >>> attributes bitmask for the variable. See 'Related Definitions.'
> >>> If not NULL, then Attributes is set on output both when
> >>> EFI_SUCCESS and when EFI_BUFFER_TOO_SMALL is returned."
> >>>
> >>> The attributes were previously only returned from the implementation
> >>> in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS
> >>> or EFI_BUFFER_TOO_SMALL according to spec.
> >>>
> >>> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> >>> Cc: Liming Gao <liming.gao@intel.com>
> >>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >>> Cc: Jian J Wang <jian.j.wang@intel.com>
> >>> Cc: Hao A Wu <hao.a.wu@intel.com>
> >>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> >>> ---
> >>> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 10
> >>> +++++++---
> >>>
> >>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
> >> c |
> >>> 10 ++++++----
> >>> 2 files changed, 13 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> >>> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> >>> index d23aea4bc712..1e71fc642c76 100644
> >>> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> >>> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> >>> @@ -18,6 +18,8 @@
> >>>
> >>> Copyright (c) 2006 - 2020, Intel Corporation. All rights
> >>> reserved.<BR>
> >>> (C) Copyright 2015-2018 Hewlett Packard Enterprise Development
> >>> LP<BR>
> >>> +Copyright (c) Microsoft Corporation.<BR>
> >>> +
> >>> SPDX-License-Identifier: BSD-2-Clause-Patent
> >>>
> >>> **/
> >>> @@ -2379,9 +2381,6 @@ VariableServiceGetVariable (
> >>> }
> >>>
> >>> CopyMem (Data, GetVariableDataPtr (Variable.CurrPtr,
> >>> mVariableModuleGlobal->VariableGlobal.AuthFormat), VarDataSize);
> >>> - if (Attributes != NULL) {
> >>> - *Attributes = Variable.CurrPtr->Attributes;
> >>> - }
> >>>
> >>> *DataSize = VarDataSize;
> >>> UpdateVariableInfo (VariableName, VendorGuid,
> >>> Variable.Volatile, TRUE, FALSE, FALSE, FALSE, &gVariableInfo); @@
> >>> -2395,6 +2394,11 @@ VariableServiceGetVariable (
> >>> }
> >>>
> >>> Done:
> >>> + if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
> >>> + if (Attributes != NULL && Variable.CurrPtr != NULL) {
> >>> + *Attributes = Variable.CurrPtr->Attributes;
> >>> + }
> >>> + }
> >>> ReleaseLockOnlyAtBootTime (&mVariableModuleGlobal-
> >>>> VariableGlobal.VariableServicesLock);
> >>> return Status;
> >>> }
> >>> diff --git
> >>>
> >>
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> >> e.c
> >>>
> >>
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> >> e.c
> >>> index 2cf0ed32ae55..ca833fb0244d 100644
> >>> ---
> >>>
> >>
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> >> e.c
> >>> +++
> >>>
> >>
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> >> e.c
> >>> @@ -14,6 +14,7 @@
> >>> InitCommunicateBuffer() is really function to check the variable data
> size.
> >>>
> >>> Copyright (c) 2010 - 2019, Intel Corporation. All rights
> >>> reserved.<BR>
> >>> +Copyright (c) Microsoft Corporation.<BR>
> >>> SPDX-License-Identifier: BSD-2-Clause-Patent
> >>>
> >>> **/
> >>> @@ -642,10 +643,6 @@ FindVariableInRuntimeCache (
> >>> }
> >>>
> >>> CopyMem (Data, GetVariableDataPtr (RtPtrTrack.CurrPtr,
> >>> mVariableAuthFormat), TempDataSize);
> >>> - if (Attributes != NULL) {
> >>> - *Attributes = RtPtrTrack.CurrPtr->Attributes;
> >>> - }
> >>> -
> >>> *DataSize = TempDataSize;
> >>>
> >>> UpdateVariableInfo (VariableName, VendorGuid,
> >>> RtPtrTrack.Volatile, TRUE, FALSE, FALSE, TRUE, &mVariableInfo); @@
> >>> -661,6 +658,11 @@ FindVariableInRuntimeCache (
> >>> }
> >>>
> >>> Done:
> >>> + if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
> >>> + if (Attributes != NULL && RtPtrTrack.CurrPtr != NULL) {
> >>> + *Attributes = RtPtrTrack.CurrPtr->Attributes;
> >>> + }
> >>> + }
> >>> mVariableRuntimeCacheReadLock = FALSE;
> >>>
> >>> return Status;
> >>> --
> >>> 2.16.3.windows.1
> >>
> >>
> >>
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-04-01 2:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20200327215536.9556-1-michael.kubacki@outlook.com>
2020-03-27 21:55 ` [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL Michael Kubacki
2020-03-30 4:15 ` Wang, Jian J
2020-03-31 8:18 ` [edk2-devel] " Guomin Jiang
2020-03-31 17:11 ` Michael Kubacki
2020-04-01 2:37 ` Guomin Jiang
2020-03-27 21:55 ` [PATCH v3 2/3] MdeModulePkg VariablePei: " Michael Kubacki
2020-03-30 4:19 ` Wang, Jian J
2020-03-31 8:04 ` [edk2-devel] " Guomin Jiang
2020-03-27 21:55 ` [PATCH v3 3/3] Revert "NetworkPkg/TlsAuthConfigDxe: fix TlsCaCertificate attributes retrieval" Michael Kubacki
2020-04-01 0:24 ` Wu, Jiaxin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox