Date   

回复: [edk2-devel] [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Fix buffer double free in CryptPkcs7VerifyEku

gaoliming
 

Jiewen:

 I am OK to merge this bug fix into this stable tag. If no objection, you can merge it tomorrow.

 

Thanks

Liming

发件人: bounce+27952+67779+4905953+8761045@groups.io <bounce+27952+67779+4905953+8761045@groups.io> 代表 Yao, Jiewen
发送时间: 20201122 20:26
收件人: Kun Qin <kun.q@...>; gaoliming <gaoliming@...>; devel@edk2.groups.io
抄送: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Jiang, Guomin <guomin.jiang@...>
主题: Re: [edk2-devel] [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Fix buffer double free in CryptPkcs7VerifyEku

 

I can help to merge if it is approved.

 

I will add reviewed-by tag when I merge it.

 

Thank you

Yao Jiewen

 

From: Kun Qin <kun.q@...>
Sent: Sunday, November 22, 2020 3:10 PM
To: gaoliming <gaoliming@...>; devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@...>
Cc: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Jiang, Guomin <guomin.jiang@...>
Subject: RE: [edk2-devel] [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Fix buffer double free in CryptPkcs7VerifyEku

 

Hi Liming,

 

It will be great if we can get this in. But I have been having trouble sending a v2 patch that incorporates Jiewen’s “Reviewed-by” tag through git command line for the past week (no other changes). It kept giving me an error of "No host provider available to service this request". Please let me know if you have any suggestions.

 

Thanks,

Kun

 

 

From: gaoliming
Sent: Thursday, November 19, 2020 9:39 PM
To: devel@edk2.groups.io; jiewen.yao@...; 'Kun Qin'
Cc: 'Wang, Jian J'; 'Lu, XiaoyuX'; 'Jiang, Guomin'
Subject:
回复: [edk2-devel] [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Fix buffer double free in CryptPkcs7VerifyEku

 

Kun:
 This is a bug fix. It passed code review. Do you request to merge it for
this stable tag 202011?

Thanks
Liming
> -----
邮件原件-----
>
发件人: bounce+27952+67567+4905953+8761045@groups.io
> <bounce+27952+67567+4905953+8761045@groups.io>
代表 Yao, Jiewen
>
发送时间: 20201114 8:32
>
收件人: Kun Qin <kun.q@...>; devel@edk2.groups.io
>
抄送: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX
> <xiaoyux.lu@...>; Jiang, Guomin <guomin.jiang@...>; Yao,
> Jiewen <jiewen.yao@...>
>
主题: Re: [edk2-devel] [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Fix buffer
> double free in CryptPkcs7VerifyEku
>
> Sorry, I missed this email.
>
> Reviewed-by: Jiewen Yao <Jiewen.yao@...>
>
>
> > -----Original Message-----
> > From: Kun Qin <kun.q@...>
> > Sent: Wednesday, October 21, 2020 10:32 AM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX
> > <xiaoyux.lu@...>; Yao, Jiewen <jiewen.yao@...>; Jiang,
> > Guomin <guomin.jiang@...>
> > Subject: [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Fix buffer double free
in
> > CryptPkcs7VerifyEku
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2459
> >
> > SignerCert is part of Pkcs7 instance when both have valid content.
OpenSLL
> > PKCS7_free function will release the memory of SignerCert when
applicable.
> > Freeing SignerCert with X509_free again might cause page fault if use-
> > after-free guard is enabled.
> >
> > Cc: Jian J Wang <jian.j.wang@...>
> > Cc: Xiaoyu Lu <xiaoyux.lu@...>
> > Cc: Jiewen Yao <jiewen.yao@...>
> > Cc: Guomin Jiang <guomin.jiang@...>
> >
> > Signed-off-by: Kun Qin <kun.q@...>
> > ---
> >  CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyEku.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyEku.c
> > b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyEku.c
> > index c9fdb65b99d1..40cc39afe7dd 100644
> > --- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyEku.c
> > +++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyEku.c
> > @@ -508,10 +508,6 @@ Exit:
> >      free (SignedData);
> >
> >    }
> >
> >
> >
> > -  if (SignerCert != NULL) {
> >
> > -    X509_free (SignerCert);
> >
> > -  }
> >
> > -
> >
> >    if (Pkcs7 != NULL) {
> >
> >      PKCS7_free (Pkcs7);
> >
> >    }
> >
> > --
> > 2.28.0.windows.1
>
>
>
>
>

 


Re: [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: add CM4 and 400 as BCM2711 designs

Samer El-Haj-Mahmoud
 

Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Andrei Warkentin via groups.io
Sent: Wednesday, November 18, 2020 6:21 PM
To: Andrei Warkentin <andrey.warkentin@...>; devel@edk2.groups.io; leif@...
Cc: Ard Biesheuvel <Ard.Biesheuvel@...>; pete@...; philmd@...
Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: add CM4 and 400 as BCM2711 designs

 

Hi Leif,

 

 

A


From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Leif Lindholm via groups.io <leif@...>
Sent: Tuesday, November 17, 2020 10:02 AM
To: Andrei Warkentin <andrey.warkentin@...>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; ard.biesheuvel@... <ard.biesheuvel@...>; pete@... <pete@...>; philmd@... <philmd@...>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/1] Platforms/RaspberryPi: add CM4 and 400 as BCM2711 designs

 

Hi Andrei,

On Sun, Nov 15, 2020 at 04:25:27 -0600, Andrei Warkentin wrote:
> Like the Pi 4B, the 3GB/4GB choices apply to it as well.
>
> Signed-off-by: Andrei Warkentin <andrey.warkentin@...>

Patch looks straghtforward enough, but ideally I'd like someone to
chime in with tested-by. That hasn't happened so far - are these
documented anywhere I could easily sanity check myself?

/
    Leif

> ---
>  Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> index 5302ccd8..ade91c9f 100644
> --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
> @@ -596,6 +596,10 @@ RpiFirmwareGetModelName (
>      return "Raspberry Pi Compute Module 3+";
>    case 0x11:
>      return "Raspberry Pi 4 Model B";
> +  case 0x13:
> +    return "Raspberry Pi 400";
> +  case 0x14:
> +    return "Raspberry Pi Compute Module 4";
>    default:
>      return "Unknown Raspberry Pi Model";
>    }
> @@ -670,6 +674,8 @@ RPiFirmwareGetModelFamily (
>        *ModelFamily = 3;
>        break;
>    case 0x11:          // Raspberry Pi 4 Model B
> +  case 0x13:          // Raspberry Pi 400
> +  case 0x14:          // Raspberry Pi Computer Module 4
>        *ModelFamily = 4;
>        break;
>    default:
> --
> 2.20.1
>




IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Fix buffer double free in CryptPkcs7VerifyEku

Yao, Jiewen
 

I can help to merge if it is approved.

 

I will add reviewed-by tag when I merge it.

 

Thank you

Yao Jiewen

 

From: Kun Qin <kun.q@...>
Sent: Sunday, November 22, 2020 3:10 PM
To: gaoliming <gaoliming@...>; devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@...>
Cc: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Jiang, Guomin <guomin.jiang@...>
Subject: RE: [edk2-devel] [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Fix buffer double free in CryptPkcs7VerifyEku

 

Hi Liming,

 

It will be great if we can get this in. But I have been having trouble sending a v2 patch that incorporates Jiewen’s “Reviewed-by” tag through git command line for the past week (no other changes). It kept giving me an error of "No host provider available to service this request". Please let me know if you have any suggestions.

 

Thanks,

Kun

 

 

From: gaoliming
Sent: Thursday, November 19, 2020 9:39 PM
To: devel@edk2.groups.io; jiewen.yao@...; 'Kun Qin'
Cc: 'Wang, Jian J'; 'Lu, XiaoyuX'; 'Jiang, Guomin'
Subject: 回复: [edk2-devel] [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Fix buffer double free in CryptPkcs7VerifyEku

 

Kun:
 This is a bug fix. It passed code review. Do you request to merge it for
this stable tag 202011?

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+67567+4905953+8761045@groups.io
> <bounce+27952+67567+4905953+8761045@groups.io> 代表 Yao, Jiewen
> 发送时间: 20201114 8:32
> 收件人: Kun Qin <kun.q@...>; devel@edk2.groups.io
> 抄送: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX
> <xiaoyux.lu@...>; Jiang, Guomin <guomin.jiang@...>; Yao,
> Jiewen <jiewen.yao@...>
> 主题: Re: [edk2-devel] [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Fix buffer
> double free in CryptPkcs7VerifyEku
>
> Sorry, I missed this email.
>
> Reviewed-by: Jiewen Yao <Jiewen.yao@...>
>
>
> > -----Original Message-----
> > From: Kun Qin <kun.q@...>
> > Sent: Wednesday, October 21, 2020 10:32 AM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX
> > <xiaoyux.lu@...>; Yao, Jiewen <jiewen.yao@...>; Jiang,
> > Guomin <guomin.jiang@...>
> > Subject: [PATCH v1 1/1] CryptoPkg: BaseCryptLib: Fix buffer double free
in
> > CryptPkcs7VerifyEku
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2459
> >
> > SignerCert is part of Pkcs7 instance when both have valid content.
OpenSLL
> > PKCS7_free function will release the memory of SignerCert when
applicable.
> > Freeing SignerCert with X509_free again might cause page fault if use-
> > after-free guard is enabled.
> >
> > Cc: Jian J Wang <jian.j.wang@...>
> > Cc: Xiaoyu Lu <xiaoyux.lu@...>
> > Cc: Jiewen Yao <jiewen.yao@...>
> > Cc: Guomin Jiang <guomin.jiang@...>
> >
> > Signed-off-by: Kun Qin <kun.q@...>
> > ---
> >  CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyEku.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyEku.c
> > b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyEku.c
> > index c9fdb65b99d1..40cc39afe7dd 100644
> > --- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyEku.c
> > +++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyEku.c
> > @@ -508,10 +508,6 @@ Exit:
> >      free (SignedData);
> >
> >    }
> >
> >
> >
> > -  if (SignerCert != NULL) {
> >
> > -    X509_free (SignerCert);
> >
> > -  }
> >
> > -
> >
> >    if (Pkcs7 != NULL) {
> >
> >      PKCS7_free (Pkcs7);
> >
> >    }
> >
> > --
> > 2.28.0.windows.1
>
>
>
>
>

 


TianoCore Design Meeting - APAC/NAMO - Fri, 11/27/2020 9:30am-10:30am #cal-reminder

devel@edk2.groups.io Calendar <devel@...>
 

Reminder: TianoCore Design Meeting - APAC/NAMO

When: Friday, 27 November 2020, 9:30am to 10:30am, (GMT+08:00) Asia/Chongqing

Where:https://intel.webex.com/intel/j.php?MTID=m6bf2875d89c1ee88ca37eda8fd41a47b

View Event

Organizer: Ray Ni ray.ni@...

Description:

TOPIC

  • TBD

For more info, see here: https://www.tianocore.org/design-meeting/

Join Webex Meeting

Meeting number: 130 073 1007

Password: MguMnPN@422

https://intel.webex.com/intel/j.php?MTID=m6bf2875d89c1ee88ca37eda8fd41a47b

Join by video system

Dial 1300731007@...

You can also dial 173.243.2.68 and enter your meeting number.

Join by phone

+1-210-795-1110 US Toll

+1-866-662-9987 US Toll Free

Access code: 130 073 1007

(Webex can be downloaded from https://www.webex.com/downloads.html/.)


[PATCH edk2-test 1/1] uefi-sct/SctPkg: OpenEx incorrect assertion

Heinrich Schuchardt
 

The functional tests for OpenEx() use RecordAssertion() statements that
lack a print code for the Tpl argument. This leads to a segmentation
violation.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
=2D--
.../SimpleFileSystemExBBTestFunction_OpenEx.c | 8 ++++----
.../SimpleFileSystemExBBTestFunction_OpenEx.c | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleFileSystem/B=
lackBoxTest/SimpleFileSystemExBBTestFunction_OpenEx.c b/uefi-sct/SctPkg/Te=
stCase/UEFI/EFI/Protocol/SimpleFileSystem/BlackBoxTest/SimpleFileSystemExB=
BTestFunction_OpenEx.c
index c2bf9b4fdc92..193383993cbe 100644
=2D-- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleFileSystem/BlackB=
oxTest/SimpleFileSystemExBBTestFunction_OpenEx.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleFileSystem/BlackBox=
Test/SimpleFileSystemExBBTestFunction_OpenEx.c
@@ -1155,7 +1155,7 @@ BBTestOpenExBasicTestCheckpoint1_Test1_Async (
EFI_TEST_ASSERTION_FAILED,
gSimpleFileSystemExBBTestFunctionAssertionGuid027,
L"OpenEx() Basic Test - checkpoint1 ----Test1----Asy=
nc",
- L"%a:%d: FileIoEntity->Tpl, Status - %r, File Name -=
%",
+ L"%a:%d: Tpl - %d, Status - %r, FileName - %s",
__FILE__,
(UINTN)__LINE__,
FileIoEntity->Tpl,
@@ -2152,7 +2152,7 @@ BBTestOpenExBasicTestCheckpoint1_Test3_Async (
EFI_TEST_ASSERTION_FAILED,
gSimpleFileSystemExBBTestFunctionAssertionGuid039,
L"OpenEx() Basic Test - checkpoint1 ---Async",
- L"%a:%d: FileIoEntity->Tpl,Status - %r,FileName - %s=
",
+ L"%a:%d: Tpl - %d, Status - %r, FileName - %s",
__FILE__,
(UINTN)__LINE__,
FileIoEntity->Tpl,
@@ -2656,7 +2656,7 @@ BBTestOpenExBasicTestCheckpoint1_Test4_Async (
EFI_TEST_ASSERTION_FAILED,
gSimpleFileSystemExBBTestFunctionAssertionGuid043,
L"OpenEx() Basic Test - checkpoint1 ---Async -- Test=
4----Open File",
- L"%a:%d: FileIoEntity->Tpl,Status - %r,FileName - %s=
",
+ L"%a:%d: Tpl - %d, Status - %r, FileName - %s",
__FILE__,
(UINTN)__LINE__,
FileIoEntity->Tpl,
@@ -3302,7 +3302,7 @@ BBTestOpenExBasicTestCheckpoint1_Test5_Async (
EFI_TEST_ASSERTION_FAILED,
gSimpleFileSystemExBBTestFunctionAssertionGuid047,
L"OpenEx() Basic Test - checkpoint1 ---Async -- Test=
5---Open File",
- L"%a:%d: FileIoEntity->Tpl,Status - %r,FileName - %s=
",
+ L"%a:%d: Tpl - %d, Status - %r, FileName - %s",
__FILE__,
(UINTN)__LINE__,
FileIoEntity->Tpl,
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleFileSystem/B=
lackBoxTest/SimpleFileSystemExBBTestFunction_OpenEx.c b/uefi-sct/SctPkg/Te=
stCase/UEFI/IHV/Protocol/SimpleFileSystem/BlackBoxTest/SimpleFileSystemExB=
BTestFunction_OpenEx.c
index 70ec88f1c065..894d42fc370d 100644
=2D-- a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleFileSystem/BlackB=
oxTest/SimpleFileSystemExBBTestFunction_OpenEx.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleFileSystem/BlackBox=
Test/SimpleFileSystemExBBTestFunction_OpenEx.c
@@ -1155,7 +1155,7 @@ BBTestOpenExBasicTestCheckpoint1_Test1_Async (
EFI_TEST_ASSERTION_FAILED,
gSimpleFileSystemExBBTestFunctionAssertionGuid027,
L"OpenEx() Basic Test - checkpoint1 ----Test1----Asy=
nc",
- L"%a:%d: FileIoEntity->Tpl, Status - %r, File Name -=
%",
+ L"%a:%d: Tpl - %d, Status - %r, FileName - %s",
__FILE__,
(UINTN)__LINE__,
FileIoEntity->Tpl,
@@ -2152,7 +2152,7 @@ BBTestOpenExBasicTestCheckpoint1_Test3_Async (
EFI_TEST_ASSERTION_FAILED,
gSimpleFileSystemExBBTestFunctionAssertionGuid039,
L"OpenEx() Basic Test - checkpoint1 ---Async",
- L"%a:%d: FileIoEntity->Tpl,Status - %r,FileName - %s=
",
+ L"%a:%d: Tpl - %d, Status - %r, FileName - %s",
__FILE__,
(UINTN)__LINE__,
FileIoEntity->Tpl,
@@ -2656,7 +2656,7 @@ BBTestOpenExBasicTestCheckpoint1_Test4_Async (
EFI_TEST_ASSERTION_FAILED,
gSimpleFileSystemExBBTestFunctionAssertionGuid043,
L"OpenEx() Basic Test - checkpoint1 ---Async -- Test=
4----Open File",
- L"%a:%d: FileIoEntity->Tpl,Status - %r,FileName - %s=
",
+ L"%a:%d: Tpl - %d, Status - %r, FileName - %s",
__FILE__,
(UINTN)__LINE__,
FileIoEntity->Tpl,
@@ -3302,7 +3302,7 @@ BBTestOpenExBasicTestCheckpoint1_Test5_Async (
EFI_TEST_ASSERTION_FAILED,
gSimpleFileSystemExBBTestFunctionAssertionGuid047,
L"OpenEx() Basic Test - checkpoint1 ---Async -- Test=
5---Open File",
- L"%a:%d: FileIoEntity->Tpl,Status - %r,FileName - %s=
",
+ L"%a:%d: Tpl - %d, Status - %r, FileName - %s",
__FILE__,
(UINTN)__LINE__,
FileIoEntity->Tpl,
=2D-
2.29.2


Re: 回复: [edk2-devel] A proposal to reduce incompatible case

Zhiguang Liu
 

Hi all,
Thanks all for the comments.

Hi Jiewen:
I think we are thinking from the different aspects.
I want the XXPkgLib.dsc.inc to specify the default library instance that the package will consumes.
You may want it to specify the default library instance that the package will produce.
I now think your way makes more sense, and also align with the implement in NetworkPkg.
I will follow your way when working on the demo code.

Hi Bret:
I see you point about that platform should be like platform and should only change in the stable tag.
I partly agree with you, but in fact there are some projects need to keep the Edk2 updated frequently.
And my proposal should also be an optional way to add the library instance.
Platform dsc can also choose the original way. I think it is at least harmless if some platforms choose not to use this way.

Hi Lazlo:
I agree with you that this proposal should only extract "Single-instance".
After all, this proposal is meant to reduce incompatible case like this VaribalePolicyLib.
I want to the platform to be "Seamless update" in such case.
However, if this lib has two instances, it is a platform's choice and platform owner should be aware the change.
Therefore, "Single-instance" and "Multiple-instance" should be totally different case, and we should only enable this proposal to "Single-instance".

Hi Liming and Ard,
Thanks for liking this idea.
I plan to work on the demo code next week and send it here for more comments.

BTW, about the file name, I want to it to follow the existing rule of NetworkPkg and ArmVirtPkg.
I think for MdeModulePkg, "MdeModuleLibs.dsc.inc" will be better.

Thanks
Zhiguang

-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Friday, November 20, 2020 7:18 PM
To: Ard Biesheuvel <ard.biesheuvel@arm.com>; gaoliming
<gaoliming@byosoft.com.cn>; devel@edk2.groups.io; Yao, Jiewen
<jiewen.yao@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>;
michael.kubacki@outlook.com; awarkentin@vmware.com;
debtech@gmail.com; Feng, Bob C <bob.c.feng@intel.com>; Tian, Hot
<hot.tian@intel.com>
Cc: 'Bret Barkelew' <bret@corthon.com>; Bi, Dandan <dandan.bi@intel.com>;
'Chao Zhang' <chao.b.zhang@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
Wu, Hao A <hao.a.wu@intel.com>; 'Liming Gao' <liming.gao@intel.com>;
Justen, Jordan L <jordan.l.justen@intel.com>; 'Andrew Fish' <afish@apple.com>;
Ni, Ray <ray.ni@intel.com>; 'Bret Barkelew' <brbarkel@microsoft.com>;
Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: 回复: [edk2-devel] A proposal to reduce incompatible case

On 11/20/20 10:02, Ard Biesheuvel wrote:
On 11/20/20 8:27 AM, gaoliming wrote:
Zhiguang:
   This proposal can reduce the potential library class dependency.
Each package has its xxxPkgLib.dsc.inc file that includes the library
instances from this package.
   Platform DSC can include the required package lib.dsc.inc file for
the library instances. Can you work out the code changes to
demonstrate this idea?
+1 for this idea. This would allow us to remove a *lot* of boilerplate
in the .DSC files, and focus on the libraries that actually matter for
the platform at hand.
I feel like I'm in *cautious* agreement with this idea.

In particular, I'd *only* like those lib class -> lib instance resolutions to be
extracted, to DSC include files, that *cannot* involve platform choice. To put it
differently, single-instance lib classes.

("Single-instance" can be interpreted per module type / per architecture of
course -- so if a lib class has exactly 1 instance for PEIMs and exactly 1
(different) instance for DXE_DRIVERs, then those resolutions qualify for
extraction.)

If a library class genuinely has multiple instances in core edk2, then extracting
a "default" resolution would be *wrong*, in my opinion. Every platform needs
to make the choice explicitly, in such cases. This applies even if a lib class only
has two instances, a functional one, and a Null one. The functional one should
not be assumed default.

Example: extracting the UefiLib resolution is valid. Extracting a SerialPortLib
class resolution is invalid.

Basically, I'd like to avoid *overrides*. Overrides are terribly hard to reason
about.

... If someone wants to work on this, they'll have to implement this with *super
small* patches, where every patch extracts resolutions for just one lib class. I
would reject any patch that required me to review the extraction of two or
more lib classes, from an OVMF or ArmVirt DSC file, at the same time.

There is big potential in this proposal for making platform DSC files
*permanently* more difficult to understand & analyze. I don't immediately see
this proposal as a good solution for avoiding the current kind of platform DSC
breakage. Platform CI would be much better, in my opinion. The proposal does
seem workable, but the implementation needs to be surgical, IMO.

OK, I'll get off my soap box now.

Thanks
Laszlo


Re: [PATCH RESEND 0/1] security fix: possible heap corruption with LzmaUefiDecompressGetInfo

Laszlo Ersek
 

On 11/19/20 12:50, Laszlo Ersek wrote:
Repo: https://pagure.io/lersek/edk2.git
Branch: tianocore_1816_resend
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1816

"RESEND" because I'm publicly posting the patch from
<https://bugzilla.tianocore.org/show_bug.cgi?id=1816#c9>.

The Reviewed-by tags on the patch originate from
<https://bugzilla.tianocore.org/show_bug.cgi?id=1816#c12> and
<https://bugzilla.tianocore.org/show_bug.cgi?id=1816#c17>.

Repeated the simple regression test at
<https://bugzilla.tianocore.org/show_bug.cgi?id=1816#c10>.

This series targets edk2-stable202011. I plan to merge it later this
week, based on Liming's R-b.

Liming, highlighting TianoCore#1816 in the "proposed features" list
could be useful.

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!
Laszlo

Laszlo Ersek (1):
MdeModulePkg/LzmaCustomDecompressLib: catch 4GB+ uncompressed buffer
sizes

MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaDecompressLibInternal.h | 5 +++++
MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaDecompress.c | 7 +++++++
2 files changed, 12 insertions(+)
Merged as commit e7bd0dd26db7, via
<https://github.com/tianocore/edk2/pull/1138>.

Thanks,
Laszlo


Re: 回复: [edk2-devel] [PATCH v2 RESEND 0/2] security fix: unlimited FV recursion, round 2 (DXE Core)

Laszlo Ersek
 

On 11/20/20 06:30, gaoliming wrote:
Laszlo:
I am OK to merge this patch and the fix in LzmaUefiDecompressGetInfo for this stable tag. After you are done, I will update the proposed feature list to include them.
Merged as commit range 6c8dd15c4ae4..47343af30435, via
<https://github.com/tianocore/edk2/pull/1137>.

Thanks,
Laszlo


In BZ, there is no CVE number. So, I want to confirm whether CVE number is required.

Thanks
Liming
-----邮件原件-----
发件人: bounce+27952+67707+4905953+8761045@groups.io
<bounce+27952+67707+4905953+8761045@groups.io> 代表 Laszlo Ersek
发送时间: 2020年11月19日 18:54
收件人: edk2-devel-groups-io <devel@edk2.groups.io>
抄送: Dandan Bi <dandan.bi@intel.com>; Hao A Wu <hao.a.wu@intel.com>;
Jian J Wang <jian.j.wang@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>; Philippe Mathieu-Daudé <philmd@redhat.com>
主题: [edk2-devel] [PATCH v2 RESEND 0/2] security fix: unlimited FV
recursion, round 2 (DXE Core)

Repo: https://pagure.io/lersek/edk2.git
Branch: tianocore_1743_v2_resend
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1743

"RESEND" because I'm publicly posting the patches from
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c19>.

The Reviewed-by tags on the patches originate from
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c20> and
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c22>.

Retested with Liming's reproducer; see
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c16> and
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c18>.

This series targets edk2-stable202011. I plan to merge it later this
week, based on Liming's R-b.

Liming, highlighting TianoCore#1743 in the "proposed features" list
could be useful.

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!
Laszlo

Laszlo Ersek (2):
MdeModulePkg/Core/Dxe: assert SectionInstance invariant in
FindChildNode()
MdeModulePkg/Core/Dxe: limit FwVol encapsulation section recursion

MdeModulePkg/MdeModulePkg.dec
| 6 +++
MdeModulePkg/MdeModulePkg.uni
| 6 +++
MdeModulePkg/Core/Dxe/DxeMain.inf
| 1 +
MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c | 52
+++++++++++++++++---
4 files changed, 59 insertions(+), 6 deletions(-)

--
2.19.1.3.g30247aa5d201











Re: [PATCH v2 1/1] [tianocore-docs edk2-TemplateSpecification] Fix the initial version to be "0.10" instead of "0.l0"

Michael D Kinney
 

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

-----Original Message-----
From: Rebecca Cran <rebecca@nuviainc.com>
Sent: Friday, November 20, 2020 12:10 PM
To: devel@edk2.groups.io
Cc: Rebecca Cran <rebecca@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [PATCH v2 1/1] [tianocore-docs edk2-TemplateSpecification] Fix the initial version to be "0.10" instead of "0.l0"

The initial revision used the letter 'l' instead of the number '1'
in "0.10".

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
Contributed-under: TianoCore Contribution Agreement 1.1
---
README.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 30edb2a17030..c09b680433bd 100644
--- a/README.md
+++ b/README.md
@@ -78,5 +78,5 @@ Copyright (c) 2017, Intel Corporation. All rights reserved.

| Revision | Revision History | Date |
| ---------- | ------------------ | ----------- |
-| 0.l0 | Initial release. | March 2017 |
+| 0.10 | Initial release. | March 2017 |
| 0.20 | Second release. | March 2017 |
--
2.26.2


[PATCH v2 1/1] [tianocore-docs edk2-TemplateSpecification] Fix the initial version to be "0.10" instead of "0.l0"

Rebecca Cran
 

The initial revision used the letter 'l' instead of the number '1'
in "0.10".

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
Contributed-under: TianoCore Contribution Agreement 1.1
---
README.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 30edb2a17030..c09b680433bd 100644
--- a/README.md
+++ b/README.md
@@ -78,5 +78,5 @@ Copyright (c) 2017, Intel Corporation. All rights reserved.

| Revision | Revision History | Date |
| ---------- | ------------------ | ----------- |
-| 0.l0 | Initial release. | March 2017 |
+| 0.10 | Initial release. | March 2017 |
| 0.20 | Second release. | March 2017 |
--
2.26.2


Re: [PATCH 0/1] [tianocore-docs edk2-TemplateSpecification] Fix the "initial version" to use the number 1 instead of the letter 'l'

Michael D Kinney
 

Hi Rebecca,

Document source/binary files use a different license than source code files.

The following line is required for patches against documents in repositories
the tianocore-docs organization.

Contributed-under: TianoCore Contribution Agreement 1.1

Thanks,

Mike

-----Original Message-----
From: Rebecca Cran <rebecca@nuviainc.com>
Sent: Friday, November 20, 2020 6:56 AM
To: devel@edk2.groups.io
Cc: Rebecca Cran <rebecca@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [PATCH 0/1] [tianocore-docs edk2-TemplateSpecification] Fix the "initial version" to use the number 1 instead of
the letter 'l'

I presume the CONTRIBUTIONS.txt document in the various tianocore-docs repos
is outdated and we no longer need the "Contributed-under: TianoCore Contribution Agreement 1.0"?

This patch simply changes "0.l0" to "0.10" for the first version in the template.

Rebecca Cran (1):
Fix the initial version to be "0.10" instead of "0.l0"

README.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--
2.26.2


Re: [edk2-platform][PATCH 1/3] Silicon/Qemu: Fix PCD numbering in SbsaQemu

Tanmay Jagdale
 

Hi Tomas,

On Thu, 19 Nov 2020 at 20:28, Tomas Pilar <tomas@...> wrote:
Fix the PCD numbering for PcdPciExpressBarLimit to be sequential.

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Tanmay Jagdale <tanmay.jagdale@...>
Signed-off-by: Tomas Pilar <tomas@...>
Tested this patch series and didn't face any issues.

Tested-by: Tanmay Jagdale <tanmay.jagdale@...>
---
 Silicon/Qemu/SbsaQemu/SbsaQemu.dec | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
index 1bc3e35b9b..2831781c4e 100644
--- a/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
+++ b/Silicon/Qemu/SbsaQemu/SbsaQemu.dec
@@ -45,7 +45,7 @@
   # PCDs complementing gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
   # BarLimit = BaseAddress + Size - 1
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciExpressBarSize|0x10000000|UINT64|0x00000009
-  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciExpressBarLimit|0xFFFFFFFF|UINT64|0x00000010
+  gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdPciExpressBarLimit|0xFFFFFFFF|UINT64|0x000000a

 [PcdsDynamic.common]
   gArmVirtSbsaQemuPlatformTokenSpaceGuid.PcdCoreCount|0x1|UINT32|0x00000100
--
2.25.1


Re: [edk2-platforms][PATCH 1/1] RaspberryPi: get RPi4 and RPi3 building again.

Ard Biesheuvel
 

On 11/19/20 1:01 AM, Andrei Warkentin wrote:
Add VariablePolicyLib and its dependency.
Testing: Pi 4 boot.
Signed-off-by: Andrei Warkentin <andrey.warkentin@gmail.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Pushed as 879f483ce455..663c3108f730

Thanks all,

---
Platform/RaspberryPi/RPi3/RPi3.dsc | 3 +++
Platform/RaspberryPi/RPi4/RPi4.dsc | 3 +++
2 files changed, 6 insertions(+)
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index 325d7bdb..9408138d 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -169,6 +169,8 @@
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
!endif
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
+ VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
GpioLib|Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.inf
[LibraryClasses.common.SEC]
@@ -218,6 +220,7 @@
CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
EfiResetSystemLib|Platform/RaspberryPi/Library/ResetLib/ResetLib.inf
ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
!if $(SECURE_BOOT_ENABLE) == TRUE
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index c994f56d..4e5a36ed 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -169,6 +169,8 @@
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
!endif
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
+ VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
GpioLib|Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.inf
#
@@ -226,6 +228,7 @@
CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
EfiResetSystemLib|Platform/RaspberryPi/Library/ResetLib/ResetLib.inf
ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
!if $(SECURE_BOOT_ENABLE) == TRUE
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf


Re: [PATCH edk2-platforms v1 1/1] Platform/ARM: Add VariablePolicy and SafeInt

Ard Biesheuvel
 

On 11/20/20 3:53 PM, Joey Gouly wrote:
Fixes the build breakage introduced by b6490426e320:
MdeModulePkg: Connect VariablePolicy business logic to VariableServices
Signed-off-by: Joey Gouly <joey.gouly@arm.com>
---
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Pushed as adcb0c92ca57..879f483ce455

Thanks! And welcome :-)

The changes can be seen at:
https://github.com/jgouly/edk2-platforms/tree/1561_fix_variable_policy_v1
Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
index 6f4621393a9713705e360a1c9ad019a6ad93a0a4..b0a48d6c041c35f2c7de6b02e54ddf104774c7c9 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
@@ -36,6 +36,7 @@ [LibraryClasses.common]
DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
+ SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
@@ -140,6 +141,8 @@ [LibraryClasses.common]
OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
+ VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
@@ -227,6 +230,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
!if $(SECURE_BOOT_ENABLE) == TRUE
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
!endif


[PATCH 1/1] [tianocore-docs edk2-TemplateSpecification] Fix the initial version to be "0.10" instead of "0.l0"

Rebecca Cran
 

The initial revision used the letter 'l' instead of the number '1'
in "0.10".

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
README.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 30edb2a17030..c09b680433bd 100644
--- a/README.md
+++ b/README.md
@@ -78,5 +78,5 @@ Copyright (c) 2017, Intel Corporation. All rights reserved.

| Revision | Revision History | Date |
| ---------- | ------------------ | ----------- |
-| 0.l0 | Initial release. | March 2017 |
+| 0.10 | Initial release. | March 2017 |
| 0.20 | Second release. | March 2017 |
--
2.26.2


[PATCH 0/1] [tianocore-docs edk2-TemplateSpecification] Fix the "initial version" to use the number 1 instead of the letter 'l'

Rebecca Cran
 

I presume the CONTRIBUTIONS.txt document in the various tianocore-docs repos
is outdated and we no longer need the "Contributed-under: TianoCore Contribution Agreement 1.0"?

This patch simply changes "0.l0" to "0.10" for the first version in the template.

Rebecca Cran (1):
Fix the initial version to be "0.10" instead of "0.l0"

README.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--
2.26.2


Re: 回复: [edk2-devel] A proposal to reduce incompatible case

Laszlo Ersek
 

On 11/20/20 10:02, Ard Biesheuvel wrote:
On 11/20/20 8:27 AM, gaoliming wrote:
Zhiguang:
   This proposal can reduce the potential library class dependency.
Each package has its xxxPkgLib.dsc.inc file that includes the library
instances from this package.
   Platform DSC can include the required package lib.dsc.inc file for
the library instances. Can you work out the code changes to
demonstrate this idea?
+1 for this idea. This would allow us to remove a *lot* of boilerplate
in the .DSC files, and focus on the libraries that actually matter for
the platform at hand.
I feel like I'm in *cautious* agreement with this idea.

In particular, I'd *only* like those lib class -> lib instance
resolutions to be extracted, to DSC include files, that *cannot* involve
platform choice. To put it differently, single-instance lib classes.

("Single-instance" can be interpreted per module type / per architecture
of course -- so if a lib class has exactly 1 instance for PEIMs and
exactly 1 (different) instance for DXE_DRIVERs, then those resolutions
qualify for extraction.)

If a library class genuinely has multiple instances in core edk2, then
extracting a "default" resolution would be *wrong*, in my opinion. Every
platform needs to make the choice explicitly, in such cases. This
applies even if a lib class only has two instances, a functional one,
and a Null one. The functional one should not be assumed default.

Example: extracting the UefiLib resolution is valid. Extracting a
SerialPortLib class resolution is invalid.

Basically, I'd like to avoid *overrides*. Overrides are terribly hard to
reason about.

... If someone wants to work on this, they'll have to implement this
with *super small* patches, where every patch extracts resolutions for
just one lib class. I would reject any patch that required me to review
the extraction of two or more lib classes, from an OVMF or ArmVirt DSC
file, at the same time.

There is big potential in this proposal for making platform DSC files
*permanently* more difficult to understand & analyze. I don't
immediately see this proposal as a good solution for avoiding the
current kind of platform DSC breakage. Platform CI would be much better,
in my opinion. The proposal does seem workable, but the implementation
needs to be surgical, IMO.

OK, I'll get off my soap box now.

Thanks
Laszlo


Re: [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib

Laszlo Ersek
 

On 11/20/20 08:11, Yao, Jiewen wrote:
I agree with Liming.

I recommend we follow the code-freeze process.
"By the date of the soft feature freeze, developers must have sent their patches to the mailing list and received positive maintainer reviews (Reviewed-by or Acked-by tags)."

The re-design could be compatible in some way. For example, we can keep old API and define RequestMonotonicCounterEx(), IncrementMonotonicCounterEx().

I am also thinking that we should check in together with a lib consumer to show the design to see what is really needed for the counter index.

So I vote to revert the change.
I agree. Without knowing many of the details:

The patch references
<https://bugzilla.tianocore.org/show_bug.cgi?id=2594>, and that ticket
is a TianoCore Feature Request. Additionally, comment#0 on the BZ says:

"Two more features are needed to be added to non-volatile variable
services [...] This BZ is for RPMC based solution only".

I think the patch should not have been committed.

Thanks
Laszlo


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
gaoliming
Sent: Friday, November 20, 2020 2:55 PM
To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io; Mistry,
Nishant C <nishant.c.mistry@intel.com>
Cc: afish@apple.com; lersek@redhat.com; 'Leif Lindholm'
<leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: 回复: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the
RpmcLib

Jian:
The commit message mentions that the re-design requires multiple RPMC
counter usages.
The library API is also updated to support multiple RPMC. So, I think this
is new feature.

But, this is just my idea. I would like to collect more feedback from the
mail list.

Thanks
Liming
-----邮件原件-----
发件人: Wang, Jian J <jian.j.wang@intel.com>
发送时间: 2020年11月20日 14:33
收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Mistry,
Nishant C
<nishant.c.mistry@intel.com>
主题: RE: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the
RpmcLib

Liming,

Sorry, I didn't notice it. But the patch was just updating the existing
code. It'd
be
more like bug fix than feature, I think.

Regards,
Jian

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
gaoliming
Sent: Friday, November 20, 2020 2:27 PM
To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Mistry,
Nishant C <nishant.c.mistry@intel.com>
Cc: gaoliming@byosoft.com.cn
Subject: 回复: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the
RpmcLib

Jian:
This change is like a feature instead of bug fix. Now, we are in soft
feature freeze phase.
According to SFF definition
https://github.com/tianocore/tianocore.github.io/wiki/SoftFeatureFreeze,
this feature should be deferred to next stable tag.

So, I suggest to revert this change, and merge it after the stable tag
202011.

Thanks
Liming
-----邮件原件-----
发件人: bounce+27952+67669+4905953+8761045@groups.io
<bounce+27952+67669+4905953+8761045@groups.io> 代表 Wang,
Jian J
发送时间: 2020年11月18日 11:35
收件人: devel@edk2.groups.io; Mistry, Nishant C
<nishant.c.mistry@intel.com>
主题: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the
RpmcLib


Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

Regards,
Jian

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
Nishant
Mistry
Sent: Thursday, November 12, 2020 2:49 AM
To: devel@edk2.groups.io
Subject: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the
RpmcLib

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2594

The re-design requires multiple RPMC counter usages.
The consumer will be capable of selecting amongst multiple counters.

Signed-off-by: Nishant C Mistry <nishant.c.mistry@intel.com>
---
SecurityPkg/Include/Library/RpmcLib.h | 6 +++++-
SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c | 6 +++++-
2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/SecurityPkg/Include/Library/RpmcLib.h
b/SecurityPkg/Include/Library/RpmcLib.h
index 5882bfae2f..3c15bce1ce 100644
--- a/SecurityPkg/Include/Library/RpmcLib.h
+++ b/SecurityPkg/Include/Library/RpmcLib.h
@@ -14,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
/**
Requests the monotonic counter from the designated RPMC
counter.

+ @param[in] CounterIndex The RPMC index
@param[out] CounterValue A pointer to a buffer
to
store the RPMC
value.

@retval EFI_SUCCESS The operation
completed
successfully.
@@ -23,12 +24,15 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
EFI_STATUS
EFIAPI
RequestMonotonicCounter (
+ IN UINT8 CounterIndex,
OUT UINT32 *CounterValue
);

/**
Increments the monotonic counter in the SPI flash device by 1.

+ @param[in] CounterIndex The RPMC index
+
@retval EFI_SUCCESS The operation
completed
successfully.
@retval EFI_DEVICE_ERROR A device error
occurred
while attempting
to update the counter.
@retval EFI_UNSUPPORTED The operation is
un-supported.
@@ -36,7 +40,7 @@ RequestMonotonicCounter (
EFI_STATUS
EFIAPI
IncrementMonotonicCounter (
- VOID
+ IN UINT8 CounterIndex
);

#endif
diff --git a/SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c
b/SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c
index e1dd09eb10..697e493a7c 100644
--- a/SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c
+++ b/SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c
@@ -12,6 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
/**
Requests the monotonic counter from the designated RPMC
counter.

+ @param[in] CounterIndex The RPMC index
@param[out] CounterValue A pointer to a buffer
to
store the RPMC
value.

@retval EFI_SUCCESS The operation
completed
successfully.
@@ -21,6 +22,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
EFI_STATUS
EFIAPI
RequestMonotonicCounter (
+ IN UINT8 CounterIndex,
OUT UINT32 *CounterValue
)
{
@@ -31,6 +33,8 @@ RequestMonotonicCounter (
/**
Increments the monotonic counter in the SPI flash device by 1.

+ @param[in] CounterIndex The RPMC index
+
@retval EFI_SUCCESS The operation
completed
successfully.
@retval EFI_DEVICE_ERROR A device error
occurred
while attempting
to update the counter.
@retval EFI_UNSUPPORTED The operation is
un-supported.
@@ -38,7 +42,7 @@ RequestMonotonicCounter (
EFI_STATUS
EFIAPI
IncrementMonotonicCounter (
- VOID
+ IN UINT8 CounterIndex
)
{
ASSERT (FALSE);
--
2.16.2.windows.1


















Re: [PATCH 3/4] OvmfPkg: create a SEV secret area in the AmdSev memfd

Laszlo Ersek
 

On 11/20/20 07:29, James Bottomley wrote:
On Thu, 2020-11-19 at 13:41 -0600, Brijesh Singh wrote:
On 11/19/20 1:50 AM, Laszlo Ersek wrote:
On 11/18/20 21:23, James Bottomley wrote:
On Mon, 2020-11-16 at 23:46 +0100, Laszlo Ersek wrote:
On 11/12/20 01:13, James Bottomley wrote:
[... I made all the changes above this]
diff --git a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
index 980e0138e7..7d3214e55d 100644
--- a/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
+++ b/OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
@@ -35,6 +35,8 @@ ALIGN 16
; the build time RIP value. The GUID must always be 48
bytes
from the
; end of the firmware.
;
+; 0xffffffc2 (-0x3e) - Base Location of the SEV Launch
Secret
+; 0xffffffc6 (-0x3a) - Size of SEV Launch Secret
; 0xffffffca (-0x36) - IP value
; 0xffffffcc (-0x34) - CS segment base [31:16]
; 0xffffffce (-0x32) - Size of the SEV-ES reset block
@@ -51,6 +53,8 @@ ALIGN 16
TIMES (32 - (sevEsResetBlockEnd - sevEsResetBlockStart)) DB
0

sevEsResetBlockStart:
+ DD SEV_LAUNCH_SECRET_BASE
+ DD SEV_LAUNCH_SECRET_SIZE
DD SEV_ES_AP_RESET_IP
DW sevEsResetBlockEnd - sevEsResetBlockStart
DB 0xDE, 0x71, 0xF7, 0x00, 0x7E, 0x1A, 0xCB, 0x4F
(5) I'd prefer if we could introduce a new GUID-ed structure
for these new fields. The logic in QEMU should be extended to
start scanning at 4GB-48 for GUIDS. If the GUID is not
recognized, then terminate scanning. Otherwise, act upon the
GUID-ed structure found there as necessary, and then determine
the next GUID *candidate* location by subtracting the last
recognized GUID-ed structure's "size" field.
So for this one, we can do it either way. However, the current
design of the sevEsRestBlock is (according to AMD) to allow the
addition of SEV specific information. Each piece of information
is a specific offset from the GUID and the length of the
structure can only grow, so the ordering is fixed once the info
is added and you can tell if the section contains what you're
looking for is present if the length covers it.

We can certainly move this to a fully GUID based system, which
would allow us to have an unordered list rather than the strict
definition the never decreasing length scheme allows, but if we
do that, the length word above becomes redundant.
Well, GUIDed structs in UEFI/PI are sometimes permitted to grow
compatibily, and for that, either a revision field or a size field
is necessary / used. I kind of desire both here -- it makes sense
to extend (for example) the SEV-ES reset block with relevant
information, and to add other blocks of information (identified
with different GUIDs).

Basically I wouldn't want to finalize the SEV-ES AP reset block
just yet, *but* I also think this new information does not beloing
in the SEV-ES *AP reset block*. The new info is related to SEV-ES
alright, but not to the AP reset block, in my opinion. If you read
the larger context (the docs) in the assembly source around
"sevEsResetBlockStart", the launch secret just doesn't seem to fit
that.

I don't have a huge preference for either mechanism ... they seem
to work equally well, but everyone should agree before I replace
the length based scheme. I agree we should all agree about it
first.
And, to reiterate, I'd like to keep both the length fields and the
GUID-ed identification. In other words, a GUID should not imply an
exact struct size, just a minimum struct size.
I agree with the GUID based approach, it aligns well with the future
needs. Looking forwardm we will need to reserve couple of pages
(secret and cpuid) for the SNP. In my WIP patches I extended reset
block to define a new GUID for those new fields.

https://github.com/AMDESE/ovmf/commit/87d47319411763d91219b377da709efdb057e662#diff-0ca7ec2856c316694c87b519c95db3270e0cac798eb09745cce167aad7f2d46dR28

And I am using this qemu patch to iterate through all the GUIDs and
call the corresponding callbacks.

https://github.com/AMDESE/qemu/commit/16a1266353d372cbb7c1998f27081fb8aa4d31e9
OK, if that's not yet upstream, I think we should do this properly:
that means having a guid and length that identifies the entire table
and then all the incorporated guids and lengths. That way we don't
have a double meaning for the reset block guid as both identifying the
start of the table and the reset vector data. Also it means we don't
need a zero guid to signal the end of the table. And also means the
reset block GUID doesn't have to always be present (if it got
deprecated for some reason).

However, the downside is that you'll have to pull out the table by this
new guid at 0xffffffd0 and its length and then iterate over the table
to find the reset block guid ... but that will make it very easy to add
the additional guids.
I agree with doing things properly.

Thanks
Laszlo


Re: 回复: [edk2-devel] [PATCH v2 RESEND 0/2] security fix: unlimited FV recursion, round 2 (DXE Core)

Laszlo Ersek
 

On 11/20/20 06:30, gaoliming wrote:
Laszlo:
I am OK to merge this patch and the fix in LzmaUefiDecompressGetInfo for this stable tag. After you are done, I will update the proposed feature list to include them.
Thanks!

In BZ, there is no CVE number. So, I want to confirm whether CVE number is required.
We seem to have failed getting a CVE number. I'm unaware of any CVE
being assigned to this issue.

Thanks
Laszlo


Thanks
Liming
-----邮件原件-----
发件人: bounce+27952+67707+4905953+8761045@groups.io
<bounce+27952+67707+4905953+8761045@groups.io> 代表 Laszlo Ersek
发送时间: 2020年11月19日 18:54
收件人: edk2-devel-groups-io <devel@edk2.groups.io>
抄送: Dandan Bi <dandan.bi@intel.com>; Hao A Wu <hao.a.wu@intel.com>;
Jian J Wang <jian.j.wang@intel.com>; Liming Gao
<gaoliming@byosoft.com.cn>; Philippe Mathieu-Daudé <philmd@redhat.com>
主题: [edk2-devel] [PATCH v2 RESEND 0/2] security fix: unlimited FV
recursion, round 2 (DXE Core)

Repo: https://pagure.io/lersek/edk2.git
Branch: tianocore_1743_v2_resend
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1743

"RESEND" because I'm publicly posting the patches from
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c19>.

The Reviewed-by tags on the patches originate from
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c20> and
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c22>.

Retested with Liming's reproducer; see
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c16> and
<https://bugzilla.tianocore.org/show_bug.cgi?id=1743#c18>.

This series targets edk2-stable202011. I plan to merge it later this
week, based on Liming's R-b.

Liming, highlighting TianoCore#1743 in the "proposed features" list
could be useful.

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!
Laszlo

Laszlo Ersek (2):
MdeModulePkg/Core/Dxe: assert SectionInstance invariant in
FindChildNode()
MdeModulePkg/Core/Dxe: limit FwVol encapsulation section recursion

MdeModulePkg/MdeModulePkg.dec
| 6 +++
MdeModulePkg/MdeModulePkg.uni
| 6 +++
MdeModulePkg/Core/Dxe/DxeMain.inf
| 1 +
MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c | 52
+++++++++++++++++---
4 files changed, 59 insertions(+), 6 deletions(-)

--
2.19.1.3.g30247aa5d201





16321 - 16340 of 84031