From: "Guomin Jiang" <guomin.jiang@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"huangming23@huawei.com" <huangming23@huawei.com>,
"Wang, Jian J" <jian.j.wang@intel.com>,
"Wu, Hao A" <hao.a.wu@intel.com>,
"Gao, Liming" <liming.gao@intel.com>
Cc: "lidongzhan@huawei.com" <lidongzhan@huawei.com>,
"songdongkuang@huawei.com" <songdongkuang@huawei.com>,
"wanghuiqiang@huawei.com" <wanghuiqiang@huawei.com>,
"qiuliangen@huawei.com" <qiuliangen@huawei.com>,
"shenlimei@huawei.com" <shenlimei@huawei.com>,
"xiewenyi2@huawei.com" <xiewenyi2@huawei.com>
Subject: Re: [edk2-devel] [PATCH edk2 v1 1/1] MdeModulePkg/Variable: Move FindVariable after AutoUpdateLangVariable
Date: Wed, 1 Jul 2020 00:22:12 +0000 [thread overview]
Message-ID: <DM6PR11MB2955E50994976175B1F5846A9D6C0@DM6PR11MB2955.namprd11.prod.outlook.com> (raw)
In-Reply-To: <d6aa3efd-0786-38cf-e215-8707766d4160@huawei.com>
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.jiang@intel.com>; devel@edk2.groups.io; Wang,
> Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao,
> Liming <liming.gao@intel.com>
> Cc: lidongzhan@huawei.com; songdongkuang@huawei.com;
> wanghuiqiang@huawei.com; qiuliangen@huawei.com;
> shenlimei@huawei.com; xiewenyi2@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.wang@intel.com>; Wu,
> >> Hao A <hao.a.wu@intel.com>; Gao, Liming <liming.gao@intel.com>
> >> Cc: lidongzhan@huawei.com; huangming23@huawei.com;
> >> songdongkuang@huawei.com; wanghuiqiang@huawei.com;
> >> qiuliangen@huawei.com; shenlimei@huawei.com;
> xiewenyi2@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@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
> >>
> >>
> >>
> >
> >
> >
>
>
>
next prev parent reply other threads:[~2020-07-01 0:22 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-29 6:06 [PATCH edk2 v1 0/1] Fix a infrequent issue in variable Ming Huang
2020-06-29 6:06 ` [PATCH edk2 v1 1/1] MdeModulePkg/Variable: Move FindVariable after AutoUpdateLangVariable Ming Huang
2020-06-30 0:58 ` [edk2-devel] " Guomin Jiang
2020-06-30 12:25 ` Ming Huang
2020-07-01 0:22 ` Guomin Jiang [this message]
2020-07-03 12:48 ` Ming Huang
2020-07-22 0:47 ` Guomin Jiang
2020-07-22 6:44 ` 答复: " Ming Huang
2020-07-22 6:48 ` Guomin Jiang
2020-07-22 11:30 ` 答复: " Ming Huang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR11MB2955E50994976175B1F5846A9D6C0@DM6PR11MB2955.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox