So I think the key point is why AutoUpdateLangVariable() return success rather than fail, if is it reasonable for this case or we need other error handing?
I am glad to help you but I can't reproduce it until now, can you provide a step to reproduce it in simulation platform. If it is urgent, I suggest that discuss with your internal team first and explain that we need consider the risk check it into edk2. Best Regards Guomin > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ming > Huang via groups.io > Sent: Tuesday, June 30, 2020 8:26 PM > To: Jiang, Guomin <guomin.ji...@intel.com>; devel@edk2.groups.io; Wang, > Jian J <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com>; Gao, > Liming <liming....@intel.com> > Cc: lidongz...@huawei.com; songdongku...@huawei.com; > wanghuiqi...@huawei.com; qiulian...@huawei.com; > shenli...@huawei.com; xiewen...@huawei.com > Subject: Re: [edk2-devel] [PATCH edk2 v1 1/1] MdeModulePkg/Variable: > Move FindVariable after AutoUpdateLangVariable > > > > 在 2020/6/30 8:58, Jiang, Guomin 写道: > > Hi Huang, > > > >>From issue statement, I guess that > > 1. AutoUpdateLangVariable() invoked, and it will invoke FindVariable() > > first, at the same time, reclaim occur and Variable.CurrPtr is invalid, it > > return > with success 2. UpdateVariable() is invoked when The old Lang's State is valid > and the new Lang's State is also valid. > > 3. In the situation, FindVariable() checked Lang's State and only enable one > Lang's State. But it didn't in fact. > > 4. BmForEachVariable() deadloop in the situation. > > > > Am I right? > > Yes, right. > > Thanks, > Ming > > > > >> -----Original Message----- > >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ming > >> Huang via groups.io > >> Sent: Monday, June 29, 2020 2:06 PM > >> To: devel@edk2.groups.io; Wang, Jian J <jian.j.w...@intel.com>; Wu, > >> Hao A <hao.a...@intel.com>; Gao, Liming <liming....@intel.com> > >> Cc: lidongz...@huawei.com; huangmin...@huawei.com; > >> songdongku...@huawei.com; wanghuiqi...@huawei.com; > >> qiulian...@huawei.com; shenli...@huawei.com; > xiewen...@huawei.com > >> Subject: [edk2-devel] [PATCH edk2 v1 1/1] MdeModulePkg/Variable: > Move > >> FindVariable after AutoUpdateLangVariable > >> > >> When occur reclaim in AutoUpdateLangVariable(), the CurrPtr of > >> Variable is invalid. The State will be update with wrong position > >> after UpdateVariable in this situation and two valid PlatformLang or Lang > variables will exist. > >> BmForEachVariable() will enter endless loop while exist two valid > >> PlatformLang variables. So FindVariable() should be invoked atfer > >> AutoUpdateLangVariable(). > >> > >> https://bugzilla.tianocore.org/show_bug.cgi?id=2667 > >> > >> Signed-off-by: Ming Huang <huangmin...@huawei.com> > >> --- > >> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 26 > >> ++++++++++---------- > >> 1 file changed, 13 insertions(+), 13 deletions(-) > >> > >> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > >> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > >> index 1e71fc6..0cec981 100644 > >> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > >> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > >> @@ -2741,6 +2741,19 @@ VariableServiceSetVariable ( > >> mVariableModuleGlobal->NonVolatileLastVariableOffset = (UINTN) > >> NextVariable - (UINTN) Point; > >> } > >> > >> + if (!FeaturePcdGet (PcdUefiVariableDefaultLangDeprecate)) { > >> + // > >> + // Hook the operation of setting PlatformLangCodes/PlatformLang > >> + and > >> LangCodes/Lang. > >> + // > >> + Status = AutoUpdateLangVariable (VariableName, Data, DataSize); > >> + if (EFI_ERROR (Status)) { > >> + // > >> + // The auto update operation failed, directly return to avoid > >> inconsistency between PlatformLang and Lang. > >> + // > >> + goto Done; > >> + } > >> + } > >> + > >> // > >> // Check whether the input variable is already existed. > >> // > >> @@ -2763,19 +2776,6 @@ VariableServiceSetVariable ( > >> } > >> } > >> > >> - if (!FeaturePcdGet (PcdUefiVariableDefaultLangDeprecate)) { > >> - // > >> - // Hook the operation of setting PlatformLangCodes/PlatformLang and > >> LangCodes/Lang. > >> - // > >> - Status = AutoUpdateLangVariable (VariableName, Data, DataSize); > >> - if (EFI_ERROR (Status)) { > >> - // > >> - // The auto update operation failed, directly return to avoid > inconsistency > >> between PlatformLang and Lang. > >> - // > >> - goto Done; > >> - } > >> - } > >> - > >> if (mVariableModuleGlobal->VariableGlobal.AuthSupport) { > >> Status = AuthVariableLibProcessVariable (VariableName, > >> VendorGuid, Data, DataSize, Attributes); > >> } else { > >> -- > >> 2.8.1 > >> > >> > >> > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61870): https://edk2.groups.io/g/devel/message/61870 Mute This Topic: https://groups.io/mt/75186349/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
