[edk2-devel] [PATCH edk2 v1 1/1] MdeModulePkg/Variable: Move FindVariable after AutoUpdateLangVariable

[Guomin Jiang]

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?

> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Ming
> Huang via groups.io
> Sent: Monday, June 29, 2020 2:06 PM
> To: devel at edk2.groups.io; Wang, Jian J <jian.j.wang at intel.com>; Wu, Hao A
> <hao.a.wu at intel.com>; Gao, Liming <liming.gao at intel.com>
> Cc: lidongzhan at huawei.com; huangming23 at huawei.com;
> songdongkuang at huawei.com; wanghuiqiang at huawei.com;
> qiuliangen at huawei.com; shenlimei at huawei.com; xiewenyi2 at 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 <huangming23 at 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 (#61817): https://edk2.groups.io/g/devel/message/61817
Mute This Topic: https://groups.io/mt/75186349/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-