Date   

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






Re: [PATCH v1 2/2] UefiCpuPkg/CpuCacheInfoLib: Add new CpuCacheInfoLib.

Laszlo Ersek
 

On 11/20/20 06:06, Ni, Ray wrote:
Laszlo,
The library can be used by code that produces SMBIOS_TABLE_TYPE7 SMBIOS table.
Great information, thank you! Exactly the kind that I wanted to hear.

Please add this sentence to the commit message (either via a v2 posting,
or at merge time, as you see fit).

Thanks!
Laszlo

It's true that right now such code is not in open source.

Thanks,
Ray

-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Thursday, November 19, 2020 5:52 PM
To: Lou, Yun <yun.lou@intel.com>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
Subject: Re: [PATCH v1 2/2] UefiCpuPkg/CpuCacheInfoLib: Add new CpuCacheInfoLib.

On 11/19/20 03:36, jasonlouyun wrote:
This library uses a platform agnostic algorithm to get CPU cache
information. It provides user with an API(GetCpuCacheInfo) to get
detailed CPU cache information by each package, each core type
included in this package, and each cache level & type.

Signed-off-by: Jason Lou <yun.lou@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c | 602 ++++++++++++++++++++
UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.c | 131 +++++
UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.c | 130 +++++
UefiCpuPkg/Include/Library/CpuCacheInfoLib.h | 68 +++
UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.uni | 15 +
UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf | 43 ++
UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h | 64 +++
UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf | 43 ++
UefiCpuPkg/UefiCpuPkg.dec | 3 +
UefiCpuPkg/UefiCpuPkg.dsc | 4 +
10 files changed, 1103 insertions(+)
I defer this review to the other UefiCpuPkg reviewers / maintainers.

Two superficial suggestions:

- please file a TianoCore feature request BZ, and reference it in the
commit messages in this series

- *at least one* of the commit message and the bugzilla ticket, but
preferably both, should describe the use case. In other words, what
platforms intend to use the new library (especially if they are open
source in edk2-platforms or otherwise), and what use cases will benefit
from having cache information.

Otherwise, this library is just dead code in edk2. I don't insist that
the consumers of this library be open source, but I do insist that
*something* be publicly explained about the use case.

Thanks
Laszlo


Re: [PATCH v9 00/13] Add the VariablePolicy feature

Laszlo Ersek
 

On 11/19/20 21:41, Bret Barkelew wrote:
Is there a way for me to still say “not my problem” but sound like less of a jerk while doing it? Maybe, maybe not. I guess I’ve always tried to be a lovable ass and emphasize the “lovable” while minimizing the “ass”.

I did some digging and discovered that it’s true that the RPi projects did not receive the same info as some of the others because apparently that project cannot be targeted by Bugzilla bugs. Take, for example, this one for CoffeeLake:
2738 – CoffeelakeSiliconPkg: Add the VariablePolicy engine to your EDK2 platform (tianocore.org)<https://bugzilla.tianocore.org/show_bug.cgi?id=2738>

I apologize for that. Don’t know who to talk to about getting your project added.
I confirm Bret filed a huge bunch of BZs for advertizing the fallout.
Again, that was in May.

If I look at the bugmail traffic back then, it seems like -- aka "I
remember it like" -- Bret ran a script that iterated over all the
platforms that had a matching component in Bugzilla, and for each such
component, the script filed a bug titled "Add the VariablePolicy engine
to your EDK2 platform". The "Package" field on each ticket was set by
the script correctly, as far as I remember.

Because I found the bug titles a little lacking in expressivity, I then
dug in and manually prefixed each title with the affected package's name.

I also seem to have run "GetMaintainer.py" manually, for (almost) every
DSC file underlying these BZs, and CC'ing the designated maintainers
manually on the corresponding BZs.

Again, checking my bugmail folder, the above reports cover the following
contiguous BZ ranges:

- #2731 through #2751 (21 tickets)
- #2754 through #2761 (8 tickets)

I think due diligence was observed. The problem was most likely that
some platforms didn't have corresponding "Component" values under the
"EDK2 Platforms" project in Bugzilla.

(In fact Bret's script was so diligent that it created tickets for some
unaffected components too, which we then ruled out manually, with the
help of Liming and maybe others. Examples: 2736, 2737, 2738, 2740, ....
Additionally, there was at least one mis-execution of the script, whcih
created duplicates. Examples: 2731, 2732, 2733, 2734, 2735, ....)

For creating a new Component in Bugzilla, please contact Mike Kinney.
And, please watch out for bugmail.

Thanks
Laszlo

I think that – philosophically – what I was trying to do is make sure that I document the required platform changes well enough that any platform could easily make them on their own time. After all, if the docs aren’t clear enough for edk2-platforms, what hope do any of our industry consumers have?

- Bret

From: Andrei Warkentin<mailto:awarkentin@vmware.com>
Sent: Thursday, November 19, 2020 12:02 PM
To: Ard Biesheuvel<mailto:ard.biesheuvel@arm.com>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; debtech@gmail.com<mailto:debtech@gmail.com>
Cc: Bret Barkelew<mailto:bret@corthon.com>; Yao, Jiewen<mailto:jiewen.yao@intel.com>; Dandan Bi<mailto:dandan.bi@intel.com>; Chao Zhang<mailto:chao.b.zhang@intel.com>; Jian J Wang<mailto:jian.j.wang@intel.com>; Hao A Wu<mailto:hao.a.wu@intel.com>; liming.gao<mailto:liming.gao@intel.com>; Jordan Justen<mailto:jordan.l.justen@intel.com>; Laszlo Ersek<mailto:lersek@redhat.com>; Andrew Fish<mailto:afish@apple.com>; Ni, Ray<mailto:ray.ni@intel.com>; Bret Barkelew<mailto:Bret.Barkelew@microsoft.com>
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v9 00/13] Add the VariablePolicy feature

Hi Bret,

To be honest, I don't recall seeing anything. Again, maybe I should have been more proactive, but that's probably the net reality for most people. It would be unreasonable to expect you to test every platform, but it is very reasonable to assume that if you know you're adding build breakage to every platform (that is trivial to fix), that you would be taking care of it... Principle of least surprise. And yes, in some weird corner case perhaps that would be insufficient (again, I don't think anyone would expect you to compile test every platform), but it would take care of 99% of obvious fall-out.

For reference, there are occasional clean-ups that happen to the edk2 tree, and I've never seen anyone claim "not my problem" to deal with the obvious fall-out resulting from renames and such.

A

From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Bret Barkelew via groups.io <debtech=gmail.com@groups.io>
Sent: Thursday, November 19, 2020 10:15 AM
To: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Bret Barkelew <bret@corthon.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Jiewen Yao <jiewen.yao@intel.com>; Dandan Bi <dandan.bi@intel.com>; Chao Zhang <chao.b.zhang@intel.com>; Jian J Wang <jian.j.wang@intel.com>; Hao A Wu <hao.a.wu@intel.com>; Liming Gao <liming.gao@intel.com>; Jordan Justen <jordan.l.justen@intel.com>; Laszlo Ersek <lersek@redhat.com>; Andrew Fish <afish@apple.com>; Ray Ni <ray.ni@intel.com>; Bret Barkelew <brbarkel@microsoft.com>
Subject: Re: [edk2-devel] [PATCH v9 00/13] Add the VariablePolicy feature

Those bugs and recommendations were sent out months ago. Several platforms have staged the changes already.

You need to add the library class to your DSC.

--
[ Insert obscure pop-culture reference here. ]

On Nov 19, 2020, at 4:46 AM, Ard Biesheuvel <ard.biesheuvel@arm.com> wrote:

On 11/9/20 7:45 AM, Bret Barkelew wrote:
The 14 patches in this series add the VariablePolicy feature to the core,
deprecate Edk2VarLock (while adding a compatibility layer to reduce code
churn), and integrate the VariablePolicy libraries and protocols into
Variable Services.
Since the integration requires multiple changes, including adding libraries,
a protocol, an SMI communication handler, and VariableServices integration,
the patches are broken up by individual library additions and then a final
integration. Security-sensitive changes like bypassing Authenticated
Variable enforcement are also broken out into individual patches so that
attention can be called directly to them.
Platform porting instructions are described in this wiki entry:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FVariablePolicy-Protocol---Enhanced-Method-for-Managing-Variables%23platform-porting&;data=04%7C01%7Cawarkentin%40vmware.com%7C594f15b45aaf476bff7e08d88cb57390%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637414058247128819%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=LLKZ7qeffR0WCvLbYuHtQIuwJGhXY0mVqB2w9B0q180%3D&amp;reserved=0<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FVariablePolicy-Protocol---Enhanced-Method-for-Managing-Variables%23platform-porting&data=04%7C01%7CBret.Barkelew%40microsoft.com%7Ca7adba0050614e2b511608d88cc609f0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637414129505929483%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=a5rbCHus%2BDVQSYmWYaOsV5aIF8WfAjH4jaEt0WZXBwM%3D&reserved=0>
Discussion of the feature can be found in multiple places throughout
the last year on the RFC channel, staging branches, and in devel.
Most recently, this subject was discussed in this thread:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&;data=04%7C01%7Cawarkentin%40vmware.com%7C594f15b45aaf476bff7e08d88cb57390%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637414058247133820%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=GYY52rlsPxw07vfdu%2BVbWhzRjtHWXlIGveCTT17mlfc%3D&amp;reserved=0<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&data=04%7C01%7CBret.Barkelew%40microsoft.com%7Ca7adba0050614e2b511608d88cc609f0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637414129505939477%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=J3dLJeVPCKyqNXG3F2%2F8mQceMJTCD%2BsJnzZKXZmTn8Q%3D&reserved=0>
(the code branches shared in that discussion are now out of date, but the
whitepapers and discussion are relevant).
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Bret Barkelew <brbarkel@microsoft.com>
Signed-off-by: Bret Barkelew <brbarkel@microsoft.com>
This series has now made it into edk2, and has subsequently broken every single platform in edk2-platforms. Is anyone intending to propose any fixes for this?


v9 changes:
* Rebase
* Address the event ordering issues around MorLock at EndOfDxe
* Drop problematic tests
* Address ECC issues
v8 changes:
* Rebase
* Small tweaks from final PRs
* Drank a lot
* Enrolled several members and a steward in CatFacts
v7 changes:
* Address comments from Dandan about security of the MM handler
* Add readme
* Fix bug around hex characters in BOOT####, etc
* Add additional testing for hex characters
* Add additional testing for authenticated variables
v6 changes:
* Fix an issue with uninitialized Status in InitVariablePolicyLib() and DeinitVariablePolicyLib()
* Fix GCC building in shell-based functional test
* Rebase on latest origin/master
v5 changes:
* Fix the CONST mismatch in VariablePolicy.h and VariablePolicySmmDxe.c
* Fix EFIAPI mismatches in the functional unittest
* Rebase on latest origin/master
v4 changes:
* Remove Optional PcdAllowVariablePolicyEnforcementDisable PCD from platforms
* Rebase on master
* Migrate to new MmCommunicate2 protocol
* Fix an oversight in the default return value for InitMmCommonCommBuffer
* Fix in VariablePolicyLib to allow ExtraInitRuntimeDxe to consume variables
V3 changes:
* Address all non-unittest issues with ECC
* Make additional style changes
* Include section name in hunk headers in "ini-style" files
* Remove requirement for the EdkiiPiSmmCommunicationsRegionTable driver
(now allocates its own buffer)
* Change names from VARIABLE_POLICY_PROTOCOL and gVariablePolicyProtocolGuid
to EDKII_VARIABLE_POLICY_PROTOCOL and gEdkiiVariablePolicyProtocolGuid
* Fix GCC warning about initializing externs
* Add UNI strings for new PCD
* Add patches for ArmVirtPkg, OvmfXen, and UefiPayloadPkg
* Reorder patches according to Liming's feedback about adding to platforms
before changing variable driver
V2 changes:
* Fixed implementation for RuntimeDxe
* Add PCD to block DisableVariablePolicy
* Fix the DumpVariablePolicy pagination in SMM
Bret Barkelew (13):
MdeModulePkg: Define the VariablePolicy protocol interface
MdeModulePkg: Define the VariablePolicyLib
MdeModulePkg: Define the VariablePolicyHelperLib
MdeModulePkg: Define the VarCheckPolicyLib and SMM interface
OvmfPkg: Add VariablePolicy engine to OvmfPkg platform
EmulatorPkg: Add VariablePolicy engine to EmulatorPkg platform
ArmVirtPkg: Add VariablePolicy engine to ArmVirtPkg platform
UefiPayloadPkg: Add VariablePolicy engine to UefiPayloadPkg platform
MdeModulePkg: Connect VariablePolicy business logic to
VariableServices
MdeModulePkg: Allow VariablePolicy state to delete protected variables
SecurityPkg: Allow VariablePolicy state to delete authenticated
variables
MdeModulePkg: Change TCG MOR variables to use VariablePolicy
MdeModulePkg: Drop VarLock from RuntimeDxe variable driver
MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c | 346 ++++++++
MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c | 396 ++++++++++
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c | 46 ++
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRuntimeDxe.c | 85 ++
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c | 830 ++++++++++++++++++++
MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c | 52 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c | 60 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c | 49 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c | 60 ++
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c | 71 ++
MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c | 573 ++++++++++++++
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 7 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 14 +
SecurityPkg/Library/AuthVariableLib/AuthService.c | 30 +-
ArmVirtPkg/ArmVirt.dsc.inc | 4 +
EmulatorPkg/EmulatorPkg.dsc | 3 +
MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h | 54 ++
MdeModulePkg/Include/Library/VariablePolicyHelperLib.h | 164 ++++
MdeModulePkg/Include/Library/VariablePolicyLib.h | 207 +++++
MdeModulePkg/Include/Protocol/VariablePolicy.h | 157 ++++
MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf | 42 +
MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni | 12 +
MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf | 35 +
MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.uni | 12 +
MdeModulePkg/Library/VariablePolicyLib/ReadMe.md | 406 ++++++++++
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf | 48 ++
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni | 12 +
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf | 51 ++
MdeModulePkg/MdeModulePkg.ci.yaml | 4 +-
MdeModulePkg/MdeModulePkg.dec | 26 +-
MdeModulePkg/MdeModulePkg.dsc | 9 +
MdeModulePkg/MdeModulePkg.uni | 7 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 5 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 4 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf | 11 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf | 4 +
OvmfPkg/OvmfPkgIa32.dsc | 5 +
OvmfPkg/OvmfPkgIa32X64.dsc | 5 +
OvmfPkg/OvmfPkgX64.dsc | 5 +
OvmfPkg/OvmfXen.dsc | 4 +
SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf | 2 +
UefiPayloadPkg/UefiPayloadPkgIa32.dsc | 4 +
UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc | 4 +
43 files changed, 3845 insertions(+), 80 deletions(-)
create mode 100644 MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
create mode 100644 MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c
create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c
create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRuntimeDxe.c
create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c
create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
create mode 100644 MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h
create mode 100644 MdeModulePkg/Include/Library/VariablePolicyHelperLib.h
create mode 100644 MdeModulePkg/Include/Library/VariablePolicyLib.h
create mode 100644 MdeModulePkg/Include/Protocol/VariablePolicy.h
create mode 100644 MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf
create mode 100644 MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni
create mode 100644 MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
create mode 100644 MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.uni
create mode 100644 MdeModulePkg/Library/VariablePolicyLib/ReadMe.md
create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf




Re: [EXTERNAL] Re: [PATCH v9 00/13] Add the VariablePolicy feature

Laszlo Ersek
 

On 11/19/20 17:35, Ard Biesheuvel wrote:
On 11/19/20 5:26 PM, Bret Barkelew wrote:
I should clarify that it wasn’t an official process. It was just a
suggestion that reached some consensus.
Fair enough. I haven't been as active on the mailing list recently, so I
may have missed some of this.
Right, I remember at some point there was a big influx of new BZs for
adopting VariablePolicy in edk2-platforms platforms... In fact if I run
a search in the TianoCore bugzilla for "variablepolicy", 15 open bugs
are returned -- from which 14 exist for various components (DSC/FDF
files) in the "Edk2 Platforms" project. Most seem to originate from
mid-to-end of May 2020.

Anyway, *if* I wanted to assign blame here, I'd absolutely assign it to
the fact that edk2-platforms exists separately from edk2... I don't have
a *clue* how we could catch breakage like this *in the general case*,
even with CI.

I don't know if I'll ever find my peace with edk2-platforms *not* being
covered by any git-grep that I run in edk2...

I was just slightly shocked that every single platform got broken by
this change.
Hopefully fixing up the platforms won't be very difficult. IIRC, it was
one of the design goals to restrict platform level changes to lib
classes / lib instance hooking into drivers / maybe some PCD changes.

Thanks,
Laszlo



------------------------------------------------------------------------
*From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Bret
Barkelew via groups.io <bret.barkelew=microsoft.com@groups.io>
*Sent:* Thursday, November 19, 2020 8:23:45 AM
*To:* Ard Biesheuvel <ard.biesheuvel@arm.com>; Bret Barkelew
<debtech@gmail.com>
*Cc:* Bret Barkelew <bret@corthon.com>; devel@edk2.groups.io
<devel@edk2.groups.io>; Yao, Jiewen <jiewen.yao@intel.com>; Dandan Bi
<dandan.bi@intel.com>; Jian J Wang <jian.j.wang@intel.com>; Hao A Wu
<hao.a.wu@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Jordan
Justen <jordan.l.justen@intel.com>; Laszlo Ersek <lersek@redhat.com>;
Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>
*Subject:* Re: [edk2-devel] [EXTERNAL] Re: [PATCH v9 00/13] Add the
VariablePolicy feature
I followed the process that was agreed upon in this list: I opened
bugs to every platform maintainer with explicit details of what needed
to be changed and waited a full month before making any further
progress with the main patches (adding a month to the completion time).

I’m wrapping things up to take a vacation next week, but if there are
still problems when I return, I can probably contribute to one or two
platforms.

- Bret
------------------------------------------------------------------------
*From:* Ard Biesheuvel <ard.biesheuvel@arm.com>
*Sent:* Thursday, November 19, 2020 8:19:37 AM
*To:* Bret Barkelew <debtech@gmail.com>
*Cc:* Bret Barkelew <bret@corthon.com>; devel@edk2.groups.io
<devel@edk2.groups.io>; Yao, Jiewen <jiewen.yao@intel.com>; Dandan Bi
<dandan.bi@intel.com>; Jian J Wang <jian.j.wang@intel.com>; Hao A Wu
<hao.a.wu@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Jordan
Justen <jordan.l.justen@intel.com>; Laszlo Ersek <lersek@redhat.com>;
Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Bret
Barkelew <Bret.Barkelew@microsoft.com>
*Subject:* [EXTERNAL] Re: [PATCH v9 00/13] Add the VariablePolicy feature
On 11/19/20 5:15 PM, Bret Barkelew wrote:
Those bugs and recommendations were sent out months ago. Several
platforms have staged the changes already.

You need to add the library class to your DSC.
I know it has been painful to get these changes in, and I am glad that
you stuck with it. But that does not make it OK to simply break every
platform in edk2-platforms/ and not take any responsibility whatsoever
for fixing it. If it is such a trivial fix, why didn't you fix it
yourself?


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

Ard Biesheuvel
 

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.


-----邮件原件-----
发件人: bounce+27952+67752+4905953+8761045@groups.io
<bounce+27952+67752+4905953+8761045@groups.io> 代表 Yao, Jiewen
发送时间: 2020年11月20日 15:20
收件人: Liu, Zhiguang <zhiguang.liu@intel.com>; devel@edk2.groups.io;
michael.kubacki@outlook.com; awarkentin@vmware.com; Ard Biesheuvel
<ard.biesheuvel@arm.com>; debtech@gmail.com; Feng, Bob C
<bob.c.feng@intel.com>; Tian, Hot <hot.tian@intel.com>
抄送: 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>; Laszlo
Ersek <lersek@redhat.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>; Liming Gao
<gaoliming@byosoft.com.cn>
主题: Re: [edk2-devel] A proposal to reduce incompatible case

I like this idea. MinPlatform also adopt the same strategy - define common
stuff in a dsc include file @
https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/Mi
nPlatformPkg/Include/Dsc


A minor clarification:
For VariablePolicyLib, I think we just need add to MdeModulePkgLib.dsc.inc.
We don’t need update UefiPayloadPkgLib.dsc.inc or SecurityPkgLib.dsc.inc,
right? They are just consumer, not producer.

Thank you
Yao Jiewen


-----Original Message-----
From: Liu, Zhiguang <zhiguang.liu@intel.com>
Sent: Friday, November 20, 2020 2:52 PM
To: devel@edk2.groups.io; michael.kubacki@outlook.com;
awarkentin@vmware.com; Ard Biesheuvel <ard.biesheuvel@arm.com>;
debtech@gmail.com; Feng, Bob C <bob.c.feng@intel.com>; Tian, Hot
<hot.tian@intel.com>
Cc: Bret Barkelew <bret@corthon.com>; Yao, Jiewen
<jiewen.yao@intel.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>; Laszlo Ersek <lersek@redhat.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>; Liming Gao <gaoliming@byosoft.com.cn>
Subject: A proposal to reduce incompatible case

Hi all,

As Michael mentioned, there are some platforms do not build and some is
because incompatible code change like this one.
I think it is a burden for both contributor and maintainer to fix platform code
when meeting such incompatible change.
I want to proposal one solution to minimum the effort of such code change.

We could add a package library instance dsc include file under each
package,
like XXXPkgLib.dsc.inc
It will specify the default library instance that will be used by modules in this
package.
For example, we add MdeModulePkgLib.dsc.inc file in MdeModulePkg.
Some package already has similar dsc include file, such as
ArmVirtPkg/ArmVirt.dsc.inc and NetworkPkg\Network.dsc.inc.
In platform dsc file, we include the XXXPkgLib.dsc.inc file at the beginning,
if
the platform uses component from the package.
We place the inc file in the beginning because we can override the library
instance in other part of the platform dsc file.

Whenever the contributor adds a new library dependency in one module, he
should also add a default library instance in the package library instance dsc
include file.

For example, in this case,
Contributor will add the below information in UefiPayloadPkgLib.dsc.inc,
SecurityPkgLib.dsc.inc and MdeModulePkgLib.dsc.inc

[LibraryClasses]

VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLi
b.inf

VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Va
riablePolicyHelperLib.inf
[LibraryClasses.common.DXE_RUNTIME_DRIVER]

VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLi
bRuntimeDxe.inf

If the platform already includes these inc files, the code change won't break
any build.
If the platform wants to choose another library instance, it can specify in the
dsc file, and will override the configuration in inc files.
This feature can even reduce the code in platform dsc file if platform choose
to use default library instance.
The problem is that it may compiles redundant modules if the

Please give comments about this proposal.

Thanks
Zhiguang


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
Michael
Kubacki
Sent: Friday, November 20, 2020 4:16 AM
To: devel@edk2.groups.io; awarkentin@vmware.com; Ard Biesheuvel
<ard.biesheuvel@arm.com>; debtech@gmail.com
Cc: Bret Barkelew <bret@corthon.com>; Yao, Jiewen
<jiewen.yao@intel.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>; Laszlo Ersek <lersek@redhat.com>; Andrew
Fish
<afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Bret Barkelew
<brbarkel@microsoft.com>
Subject: Re: [edk2-devel] [PATCH v9 00/13] Add the VariablePolicy feature

While I'm not currently a maintainer in either repo, I believe the current
process is not ideal. I highlighted some of my observations
here: https://edk2.groups.io/g/devel/message/65902.

Again, I don't have a strong vested interest in this but I do think some level
of a
more well defined process needs to be reached between repo maintiners
to
ease feature development in the future.

Thanks,
Michael

On 11/19/2020 12:02 PM, Andrei Warkentin wrote:
Hi Bret,

To be honest, I don't recall seeing anything. Again, maybe I should
have been more proactive, but that's probably the net reality for most
people. It would be unreasonable to expect you to test every platform,
but it is very reasonable to assume that if you know you're adding
build breakage to every platform (that is trivial to fix), that you
would be taking care of it... Principle of least surprise. And yes, in
some weird corner case perhaps that would be insufficient (again, I
don't think anyone would expect you to compile test every platform),
but it would take care of 99% of obvious fall-out.

For reference, there are occasional clean-ups that happen to the edk2
tree, and I've never seen anyone claim "not my problem" to deal with
the obvious fall-out resulting from renames and such.

A
----------------------------------------------------------------------
--
*From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of
Bret
Barkelew via groups.io <debtech=gmail.com@groups.io>
*Sent:* Thursday, November 19, 2020 10:15 AM
*To:* Ard Biesheuvel <ard.biesheuvel@arm.com>
*Cc:* Bret Barkelew <bret@corthon.com>; devel@edk2.groups.io
<devel@edk2.groups.io>; Jiewen Yao <jiewen.yao@intel.com>; Dandan
Bi
<dandan.bi@intel.com>; Chao Zhang <chao.b.zhang@intel.com>; Jian J
Wang <jian.j.wang@intel.com>; Hao A Wu <hao.a.wu@intel.com>;
Liming
Gao <liming.gao@intel.com>; Jordan Justen
<jordan.l.justen@intel.com>;
Laszlo Ersek <lersek@redhat.com>; Andrew Fish <afish@apple.com>;
Ray
Ni <ray.ni@intel.com>; Bret Barkelew <brbarkel@microsoft.com>
*Subject:* Re: [edk2-devel] [PATCH v9 00/13] Add the VariablePolicy
feature Those bugs and recommendations were sent out months ago.
Several platforms have staged the changes already.

You need to add the library class to your DSC.

--
[ Insert obscure pop-culture reference here. ]

On Nov 19, 2020, at 4:46 AM, Ard Biesheuvel
<ard.biesheuvel@arm.com>
wrote:

On 11/9/20 7:45 AM, Bret Barkelew wrote:
The 14 patches in this series add the VariablePolicy feature to the
core, deprecate Edk2VarLock (while adding a compatibility layer to
reduce code churn), and integrate the VariablePolicy libraries and
protocols into Variable Services.
Since the integration requires multiple changes, including adding
libraries, a protocol, an SMI communication handler, and
VariableServices integration, the patches are broken up by
individual library additions and then a final integration.
Security-sensitive changes like bypassing Authenticated Variable
enforcement are also broken out into individual patches so that
attention
can be called directly to them.
Platform porting instructions are described in this wiki entry:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
thub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FVariablePolicy-P
rotocol---Enhanced-Method-for-Managing-Variables%23platform-
porting&
amp;data=04%7C01%7Cawarkentin%40vmware.com%7C594f15b45aaf476bf
f
7e
08d
88cb57390%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637414
0
5
82471
28819%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
u
M
zIiLC
JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=LLKZ7qeffR0WCvLbY
uH
tQI
uwJGhXY0mVqB2w9B0q180%3D&amp;reserved=0
<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
hub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FVariablePolicy-
Prot
ocol---Enhanced-Method-for-Managing-Variables%23platform-
porting&amp;d
ata=04%7C01%7Cawarkentin%40vmware.com%7C594f15b45aaf476bff7e08d
88
cb573
90%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C6374140582471
2
8
819%7CU
nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
6Ik
1ha
WwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=LLKZ7qeffR0WCvLbYuHtQIuwJG
hX
Y0mVqB2
w9B0q180%3D&amp;reserved=0>
Discussion of the feature can be found in multiple places throughout
the last year on the RFC channel, staging branches, and in devel.
Most recently, this subject was discussed in this thread:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fed
k2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&amp;data=04%7C01%7C
a
wa
rke
ntin%40vmware.com%7C594f15b45aaf476bff7e08d88cb57390%7Cb39138ca
3c
ee4
b4aa4d6cd83d9dd62f0%7C0%7C0%7C637414058247133820%7CUnknown%
7
CT
WFpbGZ
sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6M
n0
%3D%7C1000&amp;sdata=GYY52rlsPxw07vfdu%2BVbWhzRjtHWXlIG
ve
CTT
17mlfc%3
D&amp;reserved=0
<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk
2.groups.io%2Fg%2Fdevel%2Fmessage%2F53712&amp;data=04%7C01%7Ca
war
kenti
n%40vmware.com%7C594f15b45aaf476bff7e08d88cb57390%7Cb39138ca3c
e
e
4b4aa4
d6cd83d9dd62f0%7C0%7C0%7C637414058247133820%7CUnknown%7CTW
F
pb
GZsb3d8ey
JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
%
7
C100
0&amp;sdata=GYY52rlsPxw07vfdu%2BVbWhzRjtHWXlIGveCTT17mlfc%3D&a
mp
;reser
ved=0>
(the code branches shared in that discussion are now out of date,
but the whitepapers and discussion are relevant).
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Bret Barkelew <brbarkel@microsoft.com>
Signed-off-by: Bret Barkelew <brbarkel@microsoft.com>
This series has now made it into edk2, and has subsequently broken
every
single platform in edk2-platforms. Is anyone intending to propose any fixes
for
this?


v9 changes:
* Rebase
* Address the event ordering issues around MorLock at EndOfDxe
* Drop problematic tests
* Address ECC issues
v8 changes:
* Rebase
* Small tweaks from final PRs
* Drank a lot
* Enrolled several members and a steward in CatFacts
v7 changes:
* Address comments from Dandan about security of the MM handler
* Add readme
* Fix bug around hex characters in BOOT####, etc
* Add additional testing for hex characters
* Add additional testing for authenticated variables
v6 changes:
* Fix an issue with uninitialized Status in InitVariablePolicyLib()
and DeinitVariablePolicyLib()
* Fix GCC building in shell-based functional test
* Rebase on latest origin/master
v5 changes:
* Fix the CONST mismatch in VariablePolicy.h and
VariablePolicySmmDxe.c
* Fix EFIAPI mismatches in the functional unittest
* Rebase on latest origin/master
v4 changes:
* Remove Optional PcdAllowVariablePolicyEnforcementDisable PCD
from
platforms
* Rebase on master
* Migrate to new MmCommunicate2 protocol
* Fix an oversight in the default return value for
InitMmCommonCommBuffer
* Fix in VariablePolicyLib to allow ExtraInitRuntimeDxe to consume
variables
V3 changes:
* Address all non-unittest issues with ECC
* Make additional style changes
* Include section name in hunk headers in "ini-style" files
* Remove requirement for the
EdkiiPiSmmCommunicationsRegionTable
driver
(now allocates its own buffer)
* Change names from VARIABLE_POLICY_PROTOCOL and
gVariablePolicyProtocolGuid
to EDKII_VARIABLE_POLICY_PROTOCOL and
gEdkiiVariablePolicyProtocolGuid
* Fix GCC warning about initializing externs
* Add UNI strings for new PCD
* Add patches for ArmVirtPkg, OvmfXen, and UefiPayloadPkg
* Reorder patches according to Liming's feedback about adding to
platforms
before changing variable driver
V2 changes:
* Fixed implementation for RuntimeDxe
* Add PCD to block DisableVariablePolicy
* Fix the DumpVariablePolicy pagination in SMM Bret Barkelew
(13):
MdeModulePkg: Define the VariablePolicy protocol interface
MdeModulePkg: Define the VariablePolicyLib
MdeModulePkg: Define the VariablePolicyHelperLib
MdeModulePkg: Define the VarCheckPolicyLib and SMM interface
OvmfPkg: Add VariablePolicy engine to OvmfPkg platform
EmulatorPkg: Add VariablePolicy engine to EmulatorPkg platform
ArmVirtPkg: Add VariablePolicy engine to ArmVirtPkg platform
UefiPayloadPkg: Add VariablePolicy engine to UefiPayloadPkg
platform
MdeModulePkg: Connect VariablePolicy business logic to
VariableServices
MdeModulePkg: Allow VariablePolicy state to delete protected
variables
SecurityPkg: Allow VariablePolicy state to delete authenticated
variables
MdeModulePkg: Change TCG MOR variables to use VariablePolicy
MdeModulePkg: Drop VarLock from RuntimeDxe variable driver
MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
| 346 ++++++++

MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelper
Li
b.
c | 396 ++++++++++

MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.
c
| 46 ++

MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRunti
me
Dxe.c | 85 ++
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
| 830 ++++++++++++++++++++
MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
| 52 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.
c

| 60 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
| 49 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
| 60 ++

MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequest
T
oLo
ck.
c | 71 ++
MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySm
mD
xe.c

| 573 ++++++++++++++
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
| 7 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRunt
im
eDx
e.c
| 14 +
SecurityPkg/Library/AuthVariableLib/AuthService.c
| 30 +-
ArmVirtPkg/ArmVirt.dsc.inc
| 4 +
EmulatorPkg/EmulatorPkg.dsc
| 3 +
MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h
| 54 ++
MdeModulePkg/Include/Library/VariablePolicyHelperLib.h
| 164 ++++
MdeModulePkg/Include/Library/VariablePolicyLib.h
| 207 +++++
MdeModulePkg/Include/Protocol/VariablePolicy.h
| 157 ++++
MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf
| 42 +
MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni
| 12 +

MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelper
Li
b.
inf | 35 +

MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelper
Li
b.
uni | 12 +
MdeModulePkg/Library/VariablePolicyLib/ReadMe.md
| 406 ++++++++++
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
| 48 ++
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
| 12 +

MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDx
e
.in
f | 51 ++
MdeModulePkg/MdeModulePkg.ci.yaml
| 4 +-
MdeModulePkg/MdeModulePkg.dec
| 26 +-
MdeModulePkg/MdeModulePkg.dsc
| 9 +
MdeModulePkg/MdeModulePkg.uni
| 7 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeD
xe
.inf

| 5 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
| 4 +

MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntim
e
Dxe.
inf
| 11 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandalon
e
Mm.i
nf
| 4 +
OvmfPkg/OvmfPkgIa32.dsc
| 5 +
OvmfPkg/OvmfPkgIa32X64.dsc
| 5 +
OvmfPkg/OvmfPkgX64.dsc
| 5 +
OvmfPkg/OvmfXen.dsc
| 4 +
SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
| 2 +
UefiPayloadPkg/UefiPayloadPkgIa32.dsc
| 4 +
UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc
| 4 +
43 files changed, 3845 insertions(+), 80 deletions(-)
create mode 100644
MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
create mode 100644
MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelper
Li
b.
c
create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.
c
create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRunti
me
Dxe.c
create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
create mode 100644
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequest
T
oLo
ck.
c
create mode 100644
MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmD
x
e.c
create mode 100644
MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h
create mode 100644
MdeModulePkg/Include/Library/VariablePolicyHelperLib.h
create mode 100644
MdeModulePkg/Include/Library/VariablePolicyLib.h
create mode 100644
MdeModulePkg/Include/Protocol/VariablePolicy.h
create mode 100644
MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf
create mode 100644
MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni
create mode 100644
MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelper
Li
b.
inf
create mode 100644
MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelper
Li
b.
uni
create mode 100644
MdeModulePkg/Library/VariablePolicyLib/ReadMe.md
create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni
create mode 100644
MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDx
e
.in
f









Hard Feature Freeze starts now for edk2-stable202011

gaoliming
 

Hi, all

  Today, we enter into Hard Feature Freeze phase until edk2-stable202011 tag is created at 2020-11-27. In this phase, there is no feature to be pushed. The critical bug fix is still allowed. So far, I know two security patches need to catch this stable tag. They both passed code review before HFF. If no objection, Laszlo will merge them for this stable tag.

 

 https://edk2.groups.io/g/devel/message/67708 Security fix: possible heap corruption with LzmaUefiDecompressGetInfo

 https://edk2.groups.io/g/devel/message/67706 MdeModulePkg/Core/Dxe: limit FwVol encapsulation section recursion

 

  If the patch is sent after Hard Feature Freeze, and plans to catch this stable tag, please add edk2-stable202011 key words in the patch title and BZ, and also cc to Tianocore Stewards, then Stewards can give the comments.

 

Below is edk2-stable202011 tag planning.

Date (00:00:00 UTC-8) Description

2020-09-04  Beginning of development

2020-11-06  Feature Planning Freeze

2020-11-13  Soft Feature Freeze

2020-11-20  Hard Feature Freeze

2020-11-27  Release

 

Thanks

Liming


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

Yao, Jiewen
 

Hey
The more I review the code, the more I think we should revisit the usage mode for multiple counters.

Some thought:

1) In previous design, it is very clear that we only have one counter. We read it and we increase it.
But here, if we add Index, that means there could be multiple counters. Then the question is how many? There is no API to tell us how many counters we can use.
For multiple counter case, I think we need at least one API - GetCounterNumber().

2) The description for Index is not clear.
Does that start from 0 or 1, to any ID to match hardware?
If it is the last one, then we need a bit-mask, or a valid ID array to show which counter number is valid to use.

3) Compatibility is another problem, if the old solution *just* need one counter, then it has to use the new API to select which counter?

Now, I am not sure if using *Index* is the best way to resolve the problem.

I recommend we check in a real solution to use the RpmcLib, then we can see what interface is really needed.

Thank you
Yao Jiewen

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wang,
Jian J
Sent: Wednesday, November 18, 2020 11:35 AM
To: devel@edk2.groups.io; Mistry, Nishant C <nishant.c.mistry@intel.com>
Subject: 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







16561 - 16580 of 84265