* [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow SetVariable from SMM @ 2021-01-06 3:50 Michael D Kinney 2021-01-06 4:13 ` [EXTERNAL] [edk2-devel] " Bret Barkelew 2021-01-06 5:46 ` Wu, Hao A 0 siblings, 2 replies; 5+ messages in thread From: Michael D Kinney @ 2021-01-06 3:50 UTC (permalink / raw) To: devel; +Cc: Bret Barkelew, Hao A Wu, Liming Gao REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3154 Update VarCheckLibSetVariableCheck() to allow locked variables to be updated if the RequestSource is VarCheckFromTrusted even if one or more variable check handlers return EFI_WRITE_PROTECTED. RequestSource is only set to VarCheckFromTrusted if the request is through the EFI_SMM_VARAIBLE_PROTOCOL. Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> Cc: Hao A Wu <hao.a.wu@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> --- MdeModulePkg/Library/VarCheckLib/VarCheckLib.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c index 470d782444bf..9596d760e945 100644 --- a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c +++ b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c @@ -1,7 +1,7 @@ /** @file Implementation functions and structures for var check services. -Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR> +Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -655,6 +655,13 @@ VarCheckLibSetVariableCheck ( DataSize, Data ); + if (Status == EFI_WRITE_PROTECTED && RequestSource == VarCheckFromTrusted) { + // + // If RequestSource is trusted, then allow variable to be set even if it + // is write protected. + // + continue; + } if (EFI_ERROR (Status)) { DEBUG ((EFI_D_INFO, "Variable Check handler fail %r - %g:%s\n", Status, VendorGuid, VariableName)); return Status; -- 2.29.2.windows.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [EXTERNAL] [edk2-devel] [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow SetVariable from SMM 2021-01-06 3:50 [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow SetVariable from SMM Michael D Kinney @ 2021-01-06 4:13 ` Bret Barkelew 2021-01-06 5:46 ` Wu, Hao A 1 sibling, 0 replies; 5+ messages in thread From: Bret Barkelew @ 2021-01-06 4:13 UTC (permalink / raw) To: devel@edk2.groups.io, Kinney, Michael D; +Cc: Hao A Wu, Liming Gao [-- Attachment #1: Type: text/plain, Size: 2672 bytes --] Reviewed-by: Bret Barkelew <bret.barkelew@microsoft.com> - Bret From: Michael D Kinney via groups.io<mailto:michael.d.kinney=intel.com@groups.io> Sent: Tuesday, January 5, 2021 7:51 PM To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> Cc: Bret Barkelew<mailto:Bret.Barkelew@microsoft.com>; Hao A Wu<mailto:hao.a.wu@intel.com>; Liming Gao<mailto:gaoliming@byosoft.com.cn> Subject: [EXTERNAL] [edk2-devel] [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow SetVariable from SMM REF: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3154&data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cdd8b9b71d99a4ae7056e08d8b1f64465%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637455018562833685%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=0UiR50Bobd3%2BQdK9lRD3ffTSoaHCNbPlY40sVeTIx3s%3D&reserved=0 Update VarCheckLibSetVariableCheck() to allow locked variables to be updated if the RequestSource is VarCheckFromTrusted even if one or more variable check handlers return EFI_WRITE_PROTECTED. RequestSource is only set to VarCheckFromTrusted if the request is through the EFI_SMM_VARAIBLE_PROTOCOL. Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> Cc: Hao A Wu <hao.a.wu@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> --- MdeModulePkg/Library/VarCheckLib/VarCheckLib.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c index 470d782444bf..9596d760e945 100644 --- a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c +++ b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c @@ -1,7 +1,7 @@ /** @file Implementation functions and structures for var check services. -Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR> +Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -655,6 +655,13 @@ VarCheckLibSetVariableCheck ( DataSize, Data ); + if (Status == EFI_WRITE_PROTECTED && RequestSource == VarCheckFromTrusted) { + // + // If RequestSource is trusted, then allow variable to be set even if it + // is write protected. + // + continue; + } if (EFI_ERROR (Status)) { DEBUG ((EFI_D_INFO, "Variable Check handler fail %r - %g:%s\n", Status, VendorGuid, VariableName)); return Status; -- 2.29.2.windows.2 [-- Attachment #2: Type: text/html, Size: 5747 bytes --] ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow SetVariable from SMM 2021-01-06 3:50 [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow SetVariable from SMM Michael D Kinney 2021-01-06 4:13 ` [EXTERNAL] [edk2-devel] " Bret Barkelew @ 2021-01-06 5:46 ` Wu, Hao A 2021-01-06 16:53 ` Michael D Kinney 1 sibling, 1 reply; 5+ messages in thread From: Wu, Hao A @ 2021-01-06 5:46 UTC (permalink / raw) To: Kinney, Michael D, devel@edk2.groups.io; +Cc: Bret Barkelew, Liming Gao > -----Original Message----- > From: Michael D Kinney <michael.d.kinney@intel.com> > Sent: Wednesday, January 6, 2021 11:51 AM > To: devel@edk2.groups.io > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wu, Hao A > <hao.a.wu@intel.com>; Liming Gao <gaoliming@byosoft.com.cn> > Subject: [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow SetVariable > from SMM > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3154 > > Update VarCheckLibSetVariableCheck() to allow locked variables to be > updated if the RequestSource is VarCheckFromTrusted even if one or more > variable check handlers return EFI_WRITE_PROTECTED. > RequestSource is only set to VarCheckFromTrusted if the request is through > the EFI_SMM_VARAIBLE_PROTOCOL. Hello Mike, Sorry for a question. If a SetVar request is blocked by a registered VarCheck handler, I think it would better to change that handler to allow requests from SMM. I am not sure if there is a VarCheck handler that has its own specific rule to return 'EFI_WRITE_PROTECTED' to block some SetVar requests. Is there any special consideration (e.g. VarCheck handler not being able to get the source of the SetVar request) for not doing this way? Thanks in advance. Best Regards, Hao Wu > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> > Cc: Hao A Wu <hao.a.wu@intel.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> > --- > MdeModulePkg/Library/VarCheckLib/VarCheckLib.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c > b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c > index 470d782444bf..9596d760e945 100644 > --- a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c > +++ b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c > @@ -1,7 +1,7 @@ > /** @file > Implementation functions and structures for var check services. > > -Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -655,6 +655,13 @@ VarCheckLibSetVariableCheck ( > DataSize, > Data > ); > + if (Status == EFI_WRITE_PROTECTED && RequestSource == > VarCheckFromTrusted) { > + // > + // If RequestSource is trusted, then allow variable to be set even if it > + // is write protected. > + // > + continue; > + } > if (EFI_ERROR (Status)) { > DEBUG ((EFI_D_INFO, "Variable Check handler fail %r - %g:%s\n", Status, > VendorGuid, VariableName)); > return Status; > -- > 2.29.2.windows.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow SetVariable from SMM 2021-01-06 5:46 ` Wu, Hao A @ 2021-01-06 16:53 ` Michael D Kinney 2021-01-07 1:02 ` Wu, Hao A 0 siblings, 1 reply; 5+ messages in thread From: Michael D Kinney @ 2021-01-06 16:53 UTC (permalink / raw) To: Wu, Hao A, devel@edk2.groups.io, Kinney, Michael D Cc: Bret Barkelew, Liming Gao > -----Original Message----- > From: Wu, Hao A <hao.a.wu@intel.com> > Sent: Tuesday, January 5, 2021 9:47 PM > To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Liming Gao <gaoliming@byosoft.com.cn> > Subject: RE: [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow SetVariable from SMM > > > -----Original Message----- > > From: Michael D Kinney <michael.d.kinney@intel.com> > > Sent: Wednesday, January 6, 2021 11:51 AM > > To: devel@edk2.groups.io > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wu, Hao A > > <hao.a.wu@intel.com>; Liming Gao <gaoliming@byosoft.com.cn> > > Subject: [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow SetVariable > > from SMM > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3154 > > > > Update VarCheckLibSetVariableCheck() to allow locked variables to be > > updated if the RequestSource is VarCheckFromTrusted even if one or more > > variable check handlers return EFI_WRITE_PROTECTED. > > RequestSource is only set to VarCheckFromTrusted if the request is through > > the EFI_SMM_VARAIBLE_PROTOCOL. > > > Hello Mike, > > Sorry for a question. > > If a SetVar request is blocked by a registered VarCheck handler, I think it would better to change that handler to allow > requests from SMM. > I am not sure if there is a VarCheck handler that has its own specific rule to return 'EFI_WRITE_PROTECTED' to block some > SetVar requests. > Is there any special consideration (e.g. VarCheck handler not being able to get the source of the SetVar request) for not > doing this way? The VarCheck handlers do not have the context information. The VarCheckHandlers function type is the same as EFI_SET_VARIABLE. typedef EFI_SET_VARIABLE VAR_CHECK_SET_VARIABLE_CHECK_HANDLER; EFI_SET_VARIABLE defines EFI_WRITE_PROTECTED return status for read-only condition. So using EFI_WRITE_PROTECTED to block a write for other reasons would not be conformant. > > Thanks in advance. > > Best Regards, > Hao Wu > > > > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> > > Cc: Hao A Wu <hao.a.wu@intel.com> > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> > > --- > > MdeModulePkg/Library/VarCheckLib/VarCheckLib.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c > > b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c > > index 470d782444bf..9596d760e945 100644 > > --- a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c > > +++ b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c > > @@ -1,7 +1,7 @@ > > /** @file > > Implementation functions and structures for var check services. > > > > -Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR> > > +Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR> > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > **/ > > @@ -655,6 +655,13 @@ VarCheckLibSetVariableCheck ( > > DataSize, > > Data > > ); > > + if (Status == EFI_WRITE_PROTECTED && RequestSource == > > VarCheckFromTrusted) { > > + // > > + // If RequestSource is trusted, then allow variable to be set even if it > > + // is write protected. > > + // > > + continue; > > + } > > if (EFI_ERROR (Status)) { > > DEBUG ((EFI_D_INFO, "Variable Check handler fail %r - %g:%s\n", Status, > > VendorGuid, VariableName)); > > return Status; > > -- > > 2.29.2.windows.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow SetVariable from SMM 2021-01-06 16:53 ` Michael D Kinney @ 2021-01-07 1:02 ` Wu, Hao A 0 siblings, 0 replies; 5+ messages in thread From: Wu, Hao A @ 2021-01-07 1:02 UTC (permalink / raw) To: Kinney, Michael D, devel@edk2.groups.io; +Cc: Bret Barkelew, Liming Gao > -----Original Message----- > From: Kinney, Michael D <michael.d.kinney@intel.com> > Sent: Thursday, January 7, 2021 12:53 AM > To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io; Kinney, > Michael D <michael.d.kinney@intel.com> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Liming Gao > <gaoliming@byosoft.com.cn> > Subject: RE: [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow > SetVariable from SMM > > > -----Original Message----- > > From: Wu, Hao A <hao.a.wu@intel.com> > > Sent: Tuesday, January 5, 2021 9:47 PM > > To: Kinney, Michael D <michael.d.kinney@intel.com>; > > devel@edk2.groups.io > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Liming Gao > > <gaoliming@byosoft.com.cn> > > Subject: RE: [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow > > SetVariable from SMM > > > > > -----Original Message----- > > > From: Michael D Kinney <michael.d.kinney@intel.com> > > > Sent: Wednesday, January 6, 2021 11:51 AM > > > To: devel@edk2.groups.io > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wu, Hao A > > > <hao.a.wu@intel.com>; Liming Gao <gaoliming@byosoft.com.cn> > > > Subject: [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow > > > SetVariable from SMM > > > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3154 > > > > > > Update VarCheckLibSetVariableCheck() to allow locked variables to be > > > updated if the RequestSource is VarCheckFromTrusted even if one or > > > more variable check handlers return EFI_WRITE_PROTECTED. > > > RequestSource is only set to VarCheckFromTrusted if the request is > > > through the EFI_SMM_VARAIBLE_PROTOCOL. > > > > > > Hello Mike, > > > > Sorry for a question. > > > > If a SetVar request is blocked by a registered VarCheck handler, I > > think it would better to change that handler to allow requests from SMM. > > I am not sure if there is a VarCheck handler that has its own specific > > rule to return 'EFI_WRITE_PROTECTED' to block some SetVar requests. > > Is there any special consideration (e.g. VarCheck handler not being > > able to get the source of the SetVar request) for not doing this way? > > The VarCheck handlers do not have the context information. > > The VarCheckHandlers function type is the same as EFI_SET_VARIABLE. > > typedef EFI_SET_VARIABLE VAR_CHECK_SET_VARIABLE_CHECK_HANDLER; > > EFI_SET_VARIABLE defines EFI_WRITE_PROTECTED return status for read- > only condition. So using EFI_WRITE_PROTECTED to block a write for other > reasons would not be conformant. Thanks. Reviewed-by: Hao A Wu <hao.a.wu@intel.com> Best Regards, Hao Wu > > > > > Thanks in advance. > > > > Best Regards, > > Hao Wu > > > > > > > > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> > > > Cc: Hao A Wu <hao.a.wu@intel.com> > > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> > > > --- > > > MdeModulePkg/Library/VarCheckLib/VarCheckLib.c | 9 ++++++++- > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > diff --git a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c > > > b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c > > > index 470d782444bf..9596d760e945 100644 > > > --- a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c > > > +++ b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.c > > > @@ -1,7 +1,7 @@ > > > /** @file > > > Implementation functions and structures for var check services. > > > > > > -Copyright (c) 2015 - 2016, Intel Corporation. All rights > > > reserved.<BR> > > > +Copyright (c) 2015 - 2021, Intel Corporation. All rights > > > +reserved.<BR> > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > **/ > > > @@ -655,6 +655,13 @@ VarCheckLibSetVariableCheck ( > > > DataSize, > > > Data > > > ); > > > + if (Status == EFI_WRITE_PROTECTED && RequestSource == > > > VarCheckFromTrusted) { > > > + // > > > + // If RequestSource is trusted, then allow variable to be set even if it > > > + // is write protected. > > > + // > > > + continue; > > > + } > > > if (EFI_ERROR (Status)) { > > > DEBUG ((EFI_D_INFO, "Variable Check handler fail %r - > > > %g:%s\n", Status, VendorGuid, VariableName)); > > > return Status; > > > -- > > > 2.29.2.windows.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-07 1:03 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-01-06 3:50 [Patch 1/1] MdeModulePkg/Library/VarCheckLib: Allow SetVariable from SMM Michael D Kinney 2021-01-06 4:13 ` [EXTERNAL] [edk2-devel] " Bret Barkelew 2021-01-06 5:46 ` Wu, Hao A 2021-01-06 16:53 ` Michael D Kinney 2021-01-07 1:02 ` Wu, Hao A
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox