Re: [edk2-devel] [PATCH 0/1] Fix XhciDxe Timeouts

[Wu, Hao A]

> -Original Message-
> From: patrick.h...@hpe.com 
> Sent: Wednesday, September 2, 2020 2:55 AM
> To: devel@edk2.groups.io
> Cc: henz ; Wang, Jian J ; Wu,
> Hao A ; Ni, Ray 
> Subject: [PATCH 0/1] Fix XhciDxe Timeouts
> 
> From: henz 
> 
> Timeouts in the XhciDxe driver are taking longer than expected due to the
> timeout loops not accounting for code execution time. As en example, 5 second
> timeouts have been observed to take around 36 seconds to complete.
> Use SetTimer and Create/CheckEvent from Boot Services to determine when
> timeout occurred. This patch was tested using forced timeouts and print
> statements with QEmu as well as phycial hardware. The forced timeouts were
> implemented in code via static variables that guaranteed a timeout the first 
> time
> the function with the broken timeout was called.
> 
> Example:
> 
> XhcExecTransfer (
>   .
>   .
>  )
> {
>   .
>   .
>   static int do_once = 1;  // test line
>   .
>   .
>   do {
> Finished = XhcCheckUrbResult (Xhc, Urb);
> if (do_once) Finished = 0; // test line
> if (Finished) {
>   break;
> }
> gBS->Stall (XHC_1_MICROSECOND);
>   } while (!EFI_ERROR(TimerStatus) && EFI_ERROR(gBS->CheckEvent
> (TimeoutEvent)));
> 
>   do_once = 0; // test line
> 
> Using this forced timeout approach the correct timeouts were observed on both
> hardware and in QEmu.
> 
> Similar broken timeout loops have been found in the Uhci and Ehci drivers. 
> This
> patch does not fix those issues.


Hello Patrick,

Besides the comments made by Ray in patch 1, could you help to provide 2 more 
patches for UHCI and EHCI drivers as well for complete enhancement?
Thanks in advance.

Best Regards,
Hao Wu


> 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Ray Ni 
> Signed-off-by: Patrick Henz 
> 
> Patrick Henz (1):
>   MdeModulePkg/XhciDxe: Fix Broken Timeouts
> 
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c   | 28 ---
>  MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 34 +---
>  2 files changed, 49 insertions(+), 13 deletions(-)
> 
> --
> 2.27.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64939): https://edk2.groups.io/g/devel/message/64939
Mute This Topic: https://groups.io/mt/76576825/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v10 0/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

[Vladimir Olovyannikov via groups.io]

Signed-off-by: Vladimir Olovyannikov 
Cc: Zhichao Gao 
Cc: Maciej Rabeda 
Cc: Jiaxin Wu 
Cc: Siyuan Fu 
Cc: Ray Ni 
Cc: Liming Gao 
Cc: Nd 
Cc: Laszlo Ersek 
Cc: Samer El-Haj-Mahmoud 

This patchset introduces an http client utilizing EDK2 HTTP protocol, to
allow fast image downloading from http/https servers.
HTTP download speed is usually faster than tftp.
The client is based on the same approach as tftp dynamic command, and
uses the same UEFI Shell command line parameters. This makes it easy
integrating http into existing UEFI Shell scripts.
Note that to enable HTTP download, feature Pcd
gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections must be set to TRUE.

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2860

PATCH v10 changes:
Address comments from Zhichao:
  - fix indentation issues;
  - fix type inconsistency for EfiGetEpochDays function.


Vladimir Olovyannikov (1):
  ShellPkg/DynamicCommand: add HttpDynamicCommand

 ShellPkg/ShellPkg.dec |1 +
 ShellPkg/ShellPkg.dsc |5 +
 .../HttpDynamicCommand/HttpApp.inf|   58 +
 .../HttpDynamicCommand/HttpDynamicCommand.inf |   63 +
 .../DynamicCommand/HttpDynamicCommand/Http.h  |   90 +
 ShellPkg/Include/Guid/ShellLibHiiGuid.h   |5 +
 .../DynamicCommand/HttpDynamicCommand/Http.c  | 1831 +
 .../HttpDynamicCommand/HttpApp.c  |   61 +
 .../HttpDynamicCommand/HttpDynamicCommand.c   |  137 ++
 .../HttpDynamicCommand/Http.uni   |  117 ++
 10 files changed, 2368 insertions(+)
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
 create mode 100644 
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.h
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.c
 create mode 100644 
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.c
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.uni

-- 
2.26.2.266.ge870325ee8


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64937): https://edk2.groups.io/g/devel/message/64937
Mute This Topic: https://groups.io/mt/76576292/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v10 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

[Vladimir Olovyannikov via groups.io]

Introduce an http client utilizing EDK2 HTTP protocol, to
allow fast image downloading from http/https servers.
HTTP download speed is usually faster than tftp.
The client is based on the same approach as tftp dynamic command, and
uses the same UEFI Shell command line parameters. This makes it easy
integrating http into existing UEFI Shell scripts.
Note that to enable HTTP download, feature Pcd
gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections must
be set to TRUE.
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2860

Signed-off-by: Vladimir Olovyannikov 
Cc: Samer El-Haj-Mahmoud 
Cc: Laszlo Ersek 
Cc: Zhichao Gao 
Cc: Maciej Rabeda 
Cc: Jiaxin Wu 
Cc: Siyuan Fu 
Cc: Ray Ni 
Cc: Liming Gao 
Cc: Nd 
---
 ShellPkg/ShellPkg.dec |1 +
 ShellPkg/ShellPkg.dsc |5 +
 .../HttpDynamicCommand/HttpApp.inf|   58 +
 .../HttpDynamicCommand/HttpDynamicCommand.inf |   63 +
 .../DynamicCommand/HttpDynamicCommand/Http.h  |   90 +
 ShellPkg/Include/Guid/ShellLibHiiGuid.h   |5 +
 .../DynamicCommand/HttpDynamicCommand/Http.c  | 1831 +
 .../HttpDynamicCommand/HttpApp.c  |   61 +
 .../HttpDynamicCommand/HttpDynamicCommand.c   |  137 ++
 .../HttpDynamicCommand/Http.uni   |  117 ++
 10 files changed, 2368 insertions(+)
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
 create mode 100644 
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.h
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.c
 create mode 100644 
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.c
 create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.uni

diff --git a/ShellPkg/ShellPkg.dec b/ShellPkg/ShellPkg.dec
index d0843d338126..7b2d1230bd2c 100644
--- a/ShellPkg/ShellPkg.dec
+++ b/ShellPkg/ShellPkg.dec
@@ -53,6 +53,7 @@ [Guids]
   gShellNetwork1HiiGuid   = {0xf3d301bb, 0xf4a5, 0x45a8, {0xb0, 0xb7, 
0xfa, 0x99, 0x9c, 0x62, 0x37, 0xae}}
   gShellNetwork2HiiGuid   = {0x174b2b5, 0xf505, 0x4b12, {0xaa, 0x60, 
0x59, 0xdf, 0xf8, 0xd6, 0xea, 0x37}}
   gShellTftpHiiGuid   = {0x738a9314, 0x82c1, 0x4592, {0x8f, 0xf7, 
0xc1, 0xbd, 0xf1, 0xb2, 0x0e, 0xd4}}
+  gShellHttpHiiGuid   = {0x390f84b3, 0x221c, 0x4d9e, {0xb5, 0x06, 
0x6d, 0xb9, 0x42, 0x3e, 0x0a, 0x7e}}
   gShellBcfgHiiGuid   = {0x5f5f605d, 0x1583, 0x4a2d, {0xa6, 0xb2, 
0xeb, 0x12, 0xda, 0xb4, 0xa2, 0xb6}}
   gShellAcpiViewHiiGuid   = {0xda8ccdf4, 0xed8f, 0x4ffc, {0xb5, 0xef, 
0x2e, 0xf5, 0x5e, 0x24, 0x93, 0x2a}}
   # FILE_GUID as defined in ShellPkg/Application/Shell/Shell.inf
diff --git a/ShellPkg/ShellPkg.dsc b/ShellPkg/ShellPkg.dsc
index 86e9f1e0040d..c42bc9464a0f 100644
--- a/ShellPkg/ShellPkg.dsc
+++ b/ShellPkg/ShellPkg.dsc
@@ -139,6 +139,11 @@ [Components]
   gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
   }
   ShellPkg/DynamicCommand/TftpDynamicCommand/TftpApp.inf
+  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf {
+
+  gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
+  }
+  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
   ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf {
 
   gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf 
b/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
new file mode 100644
index ..d08d47fb37d5
--- /dev/null
+++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
@@ -0,0 +1,58 @@
+##  @file
+# Provides Shell 'http' standalone application.
+#
+# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved. 
+# Copyright (c) 2015, ARM Ltd. All rights reserved.
+# Copyright (c) 2020, Broadcom. All rights reserved.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010006
+  BASE_NAME  = http
+  FILE_GUID  = 56B00FB7-91D2-869B-CE5C-26CD1A89C73C
+  MODULE_TYPE= UEFI_APPLICATION
+  VERSION_STRING = 1.0
+  ENTRY_POINT= HttpAppInitialize
+#
+#  This flag specifies whether HII resource section is generated into PE image.
+#
+  UEFI_HII_RESOURCE_SECTION  = TRUE
+
+[Sources.common]
+  Http.c
+  HttpApp.c
+  Http.h
+  Http.uni
+
+[Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  NetworkPkg/NetworkPkg.dec
+  ShellPkg/ShellPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  FileHandleLib
+  HiiLib
+  HttpLib
+  MemoryAllocationLib
+  NetLib
+  ShellLib
+  UefiApplicationEntryPoint
+  UefiBootServicesTableLib
+  UefiHiiServicesLib
+  UefiLib
+  UefiRuntimeServicesTableLib
+
+[Protocols]

Re: [edk2-devel] [PATCH 0/3] SecurityPkg/DxeImageVerificationLib: catch alignment overflow (CVE-2019-14562)

[Yao, Jiewen]

The series (1~3) is reviewed-by: Jiewen Yao 

Thank you
Yao Jiewen

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Laszlo Ersek
> Sent: Tuesday, September 1, 2020 5:12 PM
> To: edk2-devel-groups-io 
> Cc: Wang, Jian J ; Yao, Jiewen ;
> Xu, Min M ; Wenyi Xie 
> Subject: [edk2-devel] [PATCH 0/3] SecurityPkg/DxeImageVerificationLib: catch
> alignment overflow (CVE-2019-14562)
> 
> Ref:https://bugzilla.tianocore.org/show_bug.cgi?id=2215
> Repo:   https://pagure.io/lersek/edk2.git
> Branch: tianocore_2215
> 
> I'm neutral on whether this becomes part of edk2-stable202008.
> 
> Cc: Jian J Wang 
> Cc: Jiewen Yao 
> Cc: Min Xu 
> Cc: Wenyi Xie 
> 
> Thanks,
> Laszlo
> 
> Laszlo Ersek (3):
>   SecurityPkg/DxeImageVerificationLib: extract SecDataDirEnd,
> SecDataDirLeft
>   SecurityPkg/DxeImageVerificationLib: assign WinCertificate after size
> check
>   SecurityPkg/DxeImageVerificationLib: catch alignment overflow
> (CVE-2019-14562)
> 
>  SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 16
> 
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> --
> 2.19.1.3.g30247aa5d201
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64936): https://edk2.groups.io/g/devel/message/64936
Mute This Topic: https://groups.io/mt/76552538/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v9 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

[Vladimir Olovyannikov via groups.io]

Hi Zhichao,
Thank you for the feedback.

> -Original Message-
> From: Gao, Zhichao 
> Sent: Tuesday, September 1, 2020 7:32 PM
> To: devel@edk2.groups.io; vladimir.olovyanni...@broadcom.com
> Cc: Samer El-Haj-Mahmoud ; Laszlo
> Ersek ; Maciej Rabeda
> ; Wu, Jiaxin ; Fu,
> Siyuan ; Ni, Ray ; Gao, Liming
> ; Nd 
> Subject: RE: [edk2-devel] [PATCH v9 1/1] ShellPkg/DynamicCommand: add
> HttpDynamicCommand
>
> There is no much change for the shell part VS V6. But there are still
some
> coding issue. I already fixed it in my branch
> https://github.com/ZhichaoGao/edk2/tree/push, only indent issue.
Thank you for spotting these.
Which tool do you use to find issues like this?
I looked through the code and just overlooked this issue.
>
> Another issue is the EfiTimeToEpoch:
> ---
>   UINT32 EpochDays;   // Number of days elapsed since EPOCH_JULIAN_DAY
>   UINT32 EpochSeconds;
>
>   EpochDays = EfiGetEpochDays (Time);
> ---
> EfiGetEpochDays return UINTN and it would cause a warning (warning would
> be treated as error) under X64 build. Please fix it.
OK. I actually took it exactly as it is in
EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c
The very same issue exists there. Even though it builds fine with
build -a X64 -b NOOPT -t GCC48 -p ShellPkg/ShellPkg.dsc -m
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
I agree, it should have UINTN everywhere.
Similar fix has to be done for
EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c in a later, separate patch.
>
> Others are OK for me.
>
> Thanks,
> Zhichao
Thank you,
Vladimir
>
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of
> Vladimir
> > Olovyannikov via groups.io
> > Sent: Sunday, August 30, 2020 8:29 AM
> > To: devel@edk2.groups.io
> > Cc: Vladimir Olovyannikov ;
> Samer El-
> > Haj-Mahmoud ; Laszlo Ersek
> > ; Gao, Zhichao ; Maciej
> Rabeda
> > ; Wu, Jiaxin ; Fu,
> Siyuan
> > ; Ni, Ray ; Gao, Liming
> > ; Nd 
> > Subject: [edk2-devel] [PATCH v9 1/1] ShellPkg/DynamicCommand: add
> > HttpDynamicCommand
> >
> > Introduce an http client utilizing EDK2 HTTP protocol, to
> > allow fast image downloading from http/https servers.
> > HTTP download speed is usually faster than tftp.
> > The client is based on the same approach as tftp dynamic command, and
> > uses the same UEFI Shell command line parameters. This makes it easy
> > integrating http into existing UEFI Shell scripts.
> > Note that to enable HTTP download, feature Pcd
> > gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections must
> > be set to TRUE.
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2860
> >
> > Signed-off-by: Vladimir Olovyannikov
> 
> > Cc: Samer El-Haj-Mahmoud 
> > Cc: Laszlo Ersek 
> > Cc: Zhichao Gao 
> > Cc: Maciej Rabeda 
> > Cc: Jiaxin Wu 
> > Cc: Siyuan Fu 
> > Cc: Ray Ni 
> > Cc: Liming Gao 
> > Cc: Nd 
> > ---
> >  ShellPkg/ShellPkg.dec |1 +
> >  ShellPkg/ShellPkg.dsc |5 +
> >  .../HttpDynamicCommand/HttpApp.inf|   58 +
> >  .../HttpDynamicCommand/HttpDynamicCommand.inf |   63 +
> >  .../DynamicCommand/HttpDynamicCommand/Http.h  |   90 +
> >  ShellPkg/Include/Guid/ShellLibHiiGuid.h   |5 +
> >  .../DynamicCommand/HttpDynamicCommand/Http.c  | 1830
> > +
> >  .../HttpDynamicCommand/HttpApp.c  |   61 +
> >  .../HttpDynamicCommand/HttpDynamicCommand.c   |  137 ++
> >  .../HttpDynamicCommand/Http.uni   |  117 ++
> >  10 files changed, 2367 insertions(+)
> >  create mode 100644
> > ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
> >  create mode 100644
> >
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand
> .inf
> >  create mode 100644
> > ShellPkg/DynamicCommand/HttpDynamicCommand/Http.h
> >  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
> >  create mode 100644
> > ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.c
> >  create mode 100644
> >
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand
> .c
> >  create mode 100644
> > ShellPkg/DynamicCommand/HttpDynamicCommand/Http.uni
> >
> > diff --git a/ShellPkg/ShellPkg.dec b/ShellPkg/ShellPkg.dec
> > index d0843d338126..7b2d1230bd2c 100644
> > --- a/ShellPkg/ShellPkg.dec
> > +++ b/ShellPkg/ShellPkg.dec
> > @@ -53,6 +53,7 @@ [Guids]
> >gShellNetwork1HiiGuid   = {0xf3d301bb, 0xf4a5, 0x45a8,
{0xb0, 0xb7,
> 0xfa,
> > 0x99, 0x9c, 0x62, 0x37, 0xae}}
> >gShellNetwork2HiiGuid   = {0x174b2b5, 0xf505, 0x4b12,
{0xaa, 0x60,
> 0x59,
> > 0xdf, 0xf8, 0xd6, 0xea, 0x37}}
> >gShellTftpHiiGuid   = {0x738a9314, 0x82c1, 0x4592,
{0x8f, 0xf7, 0xc1,
> > 0xbd, 0xf1, 0xb2, 0x0e, 0xd4}}
> > +  gShellHttpHiiGuid   = {0x390f84b3, 0x221c, 0x4d9e,
{0xb5, 0x06,
> 0x6d,
> > 0xb9, 0x42, 0x3e, 0x0a, 0x7e}}
> >gShellBcfgHiiGuid   = {0x5f5f605d, 0x1583, 0x4a2d,
{0xa6, 0xb2, 0xeb,
> > 0x12, 0xda, 0xb4, 0xa2, 0xb6}}
> >gShellAcpiViewHiiGuid   = {0xda8ccdf4, 0xed8f, 0x4ffc,

Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/XhciDxe: Fix Broken Timeouts

[Ni, Ray]

In general, thanks for enhancing the logic to take the cmd execution duration 
into the timeout.
2 minor comments:

> 
> + (EFI_TIMER_PERIOD_MICROSECONDS(0x)):
1. Can you enhance the patch to avoid creating the timer event the Timeout is 0.
> 
> +  } while (!EFI_ERROR(TimerStatus) && EFI_ERROR(gBS->CheckEvent 
> (TimeoutEvent)));
> 
2. Can you please move the !EFI_ERROR (TimerStatus) check from the while() to 
outside of while()?
Because this status value never changes in while loop.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64934): https://edk2.groups.io/g/devel/message/64934
Mute This Topic: https://groups.io/mt/76575773/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v9 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

[Gao, Zhichao]

There is no much change for the shell part VS V6. But there are still some 
coding issue. I already fixed it in my branch 
https://github.com/ZhichaoGao/edk2/tree/push, only indent issue.

Another issue is the EfiTimeToEpoch:
---
  UINT32 EpochDays;   // Number of days elapsed since EPOCH_JULIAN_DAY
  UINT32 EpochSeconds;

  EpochDays = EfiGetEpochDays (Time);
---
EfiGetEpochDays return UINTN and it would cause a warning (warning would be 
treated as error) under X64 build. Please fix it.

Others are OK for me.

Thanks,
Zhichao

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Vladimir
> Olovyannikov via groups.io
> Sent: Sunday, August 30, 2020 8:29 AM
> To: devel@edk2.groups.io
> Cc: Vladimir Olovyannikov ; Samer El-
> Haj-Mahmoud ; Laszlo Ersek
> ; Gao, Zhichao ; Maciej Rabeda
> ; Wu, Jiaxin ; Fu, Siyuan
> ; Ni, Ray ; Gao, Liming
> ; Nd 
> Subject: [edk2-devel] [PATCH v9 1/1] ShellPkg/DynamicCommand: add
> HttpDynamicCommand
> 
> Introduce an http client utilizing EDK2 HTTP protocol, to
> allow fast image downloading from http/https servers.
> HTTP download speed is usually faster than tftp.
> The client is based on the same approach as tftp dynamic command, and
> uses the same UEFI Shell command line parameters. This makes it easy
> integrating http into existing UEFI Shell scripts.
> Note that to enable HTTP download, feature Pcd
> gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections must
> be set to TRUE.
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2860
> 
> Signed-off-by: Vladimir Olovyannikov 
> Cc: Samer El-Haj-Mahmoud 
> Cc: Laszlo Ersek 
> Cc: Zhichao Gao 
> Cc: Maciej Rabeda 
> Cc: Jiaxin Wu 
> Cc: Siyuan Fu 
> Cc: Ray Ni 
> Cc: Liming Gao 
> Cc: Nd 
> ---
>  ShellPkg/ShellPkg.dec |1 +
>  ShellPkg/ShellPkg.dsc |5 +
>  .../HttpDynamicCommand/HttpApp.inf|   58 +
>  .../HttpDynamicCommand/HttpDynamicCommand.inf |   63 +
>  .../DynamicCommand/HttpDynamicCommand/Http.h  |   90 +
>  ShellPkg/Include/Guid/ShellLibHiiGuid.h   |5 +
>  .../DynamicCommand/HttpDynamicCommand/Http.c  | 1830
> +
>  .../HttpDynamicCommand/HttpApp.c  |   61 +
>  .../HttpDynamicCommand/HttpDynamicCommand.c   |  137 ++
>  .../HttpDynamicCommand/Http.uni   |  117 ++
>  10 files changed, 2367 insertions(+)
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/Http.h
>  create mode 100644 ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.c
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.c
>  create mode 100644
> ShellPkg/DynamicCommand/HttpDynamicCommand/Http.uni
> 
> diff --git a/ShellPkg/ShellPkg.dec b/ShellPkg/ShellPkg.dec
> index d0843d338126..7b2d1230bd2c 100644
> --- a/ShellPkg/ShellPkg.dec
> +++ b/ShellPkg/ShellPkg.dec
> @@ -53,6 +53,7 @@ [Guids]
>gShellNetwork1HiiGuid   = {0xf3d301bb, 0xf4a5, 0x45a8, {0xb0, 
> 0xb7, 0xfa,
> 0x99, 0x9c, 0x62, 0x37, 0xae}}
>gShellNetwork2HiiGuid   = {0x174b2b5, 0xf505, 0x4b12, {0xaa, 0x60, 
> 0x59,
> 0xdf, 0xf8, 0xd6, 0xea, 0x37}}
>gShellTftpHiiGuid   = {0x738a9314, 0x82c1, 0x4592, {0x8f, 
> 0xf7, 0xc1,
> 0xbd, 0xf1, 0xb2, 0x0e, 0xd4}}
> +  gShellHttpHiiGuid   = {0x390f84b3, 0x221c, 0x4d9e, {0xb5, 
> 0x06, 0x6d,
> 0xb9, 0x42, 0x3e, 0x0a, 0x7e}}
>gShellBcfgHiiGuid   = {0x5f5f605d, 0x1583, 0x4a2d, {0xa6, 
> 0xb2, 0xeb,
> 0x12, 0xda, 0xb4, 0xa2, 0xb6}}
>gShellAcpiViewHiiGuid   = {0xda8ccdf4, 0xed8f, 0x4ffc, {0xb5, 
> 0xef, 0x2e,
> 0xf5, 0x5e, 0x24, 0x93, 0x2a}}
># FILE_GUID as defined in ShellPkg/Application/Shell/Shell.inf
> diff --git a/ShellPkg/ShellPkg.dsc b/ShellPkg/ShellPkg.dsc
> index 86e9f1e0040d..c42bc9464a0f 100644
> --- a/ShellPkg/ShellPkg.dsc
> +++ b/ShellPkg/ShellPkg.dsc
> @@ -139,6 +139,11 @@ [Components]
>gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
>}
>ShellPkg/DynamicCommand/TftpDynamicCommand/TftpApp.inf
> +
> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand.inf {
> +
> +  gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> +  }
> +  ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
>ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf {
>  
>gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
> diff --git a/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
> b/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
> new file mode 100644
> index ..d08d47fb37d5
> --- /dev/null
> +++ b/ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
> @@ -0,0 +1,58 @@
> +##  @file
> +# Provides Shell 'http' standalone application.
> +#
> +# Copyright (c) 2010 

Re: [edk2-devel] [PATCH v3 3/3] IntelSiliconPkg/PlatformVTdInfoSamplePei: Install Null Root Entry Table

[Ni, Ray]

Reviewed-by: Ray Ni 

> -Original Message-
> From: Sheng, W 
> Sent: Monday, August 31, 2020 2:38 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Chaganty, Rangasai V 
> 
> Subject: [PATCH v3 3/3] IntelSiliconPkg/PlatformVTdInfoSamplePei: Install 
> Null Root Entry Table
> 
> BIOS uses TE with a null root entry table to block VT-d engine access
> to block any DMA traffic in pre-memory phase.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2867
> 
> Change-Id: I6c086c1f26e60f781de79cc37677cc5717c5edec
> Cc: Ray Ni 
> Cc: Rangasai V Chaganty 
> Signed-off-by: Sheng Wei 
> ---
>  .../PlatformVTdInfoSamplePei/PlatformVTdInfoSamplePei.c  | 16 
> 
>  .../PlatformVTdInfoSamplePei.inf |  3 ++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git 
> a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/PlatformVTdInfoSamplePei.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/PlatformVTdInfoSamplePei.c
> index 6f6c14f7..616a96ce 100644
> --- 
> a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/PlatformVTdInfoSamplePei.c
> +++ 
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/PlatformVTdInfoSamplePei.c
> @@ -9,6 +9,7 @@
>  #include 
> 
>  #include 
> +#include 
> 
>  #include 
>  #include 
> @@ -164,6 +165,15 @@ EFI_PEI_PPI_DESCRIPTOR mPlatformVTdNoIgdInfoSampleDesc = 
> {
>
>  };
> 
> +// BIOS uses TE with a null root entry table to block VT-d engine access to 
> block any DMA traffic in pre-memory phase.
> +EDKII_VTD_NULL_ROOT_ENTRY_TABLE_PPI mNullRootEntryTable = 0xFED2;
> +
> +EFI_PEI_PPI_DESCRIPTOR mPlatformNullRootEntryTableDesc = {
> +  (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> +  ,
> +  
> +};
> +
>  /**
>Initialize VTd register.
>Initialize the VTd hardware unit which has INCLUDE_PCI_ALL set
> @@ -344,6 +354,12 @@ PlatformVTdInfoSampleInitialize (
>if (!EFI_ERROR(Status)) {
>  SiliconInitialized = TRUE;
>}
> +
> +  Status = PeiServicesInstallPpi ();
> +  if (EFI_ERROR (Status)) {
> +ASSERT_EFI_ERROR (Status);
> +  }
> +
>DEBUG ((DEBUG_INFO, "SiliconInitialized - %x\n", SiliconInitialized));
>if (!SiliconInitialized) {
>  Status = PeiServicesNotifyPpi ();
> diff --git 
> a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/PlatformVTdInfoSamplePei.inf
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/PlatformVTdInfoSamplePei.inf
> index dacfdf5e..b35853b6 100644
> --- 
> a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/PlatformVTdInfoSamplePei.inf
> +++ 
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/PlatformVTdInfoSamplePei.inf
> @@ -38,7 +38,8 @@
>IoLib
> 
>  [Ppis]
> -  gEdkiiVTdInfoPpiGuid ## PRODUCES
> +  gEdkiiVTdInfoPpiGuid  ## PRODUCES
> +  gEdkiiVTdNullRootEntryTableGuid   ## PRODUCES
> 
>  [Depex]
>gEfiPeiMasterBootModePpiGuid
> --
> 2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64932): https://edk2.groups.io/g/devel/message/64932
Mute This Topic: https://groups.io/mt/76529335/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/3] SecurityPkg/DxeImageVerificationLib: assign WinCertificate after size check

[wenyi,xie via groups.io]

On 2020/9/1 17:12, Laszlo Ersek wrote:
> Currently the (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) check only
> guards the de-referencing of the "WinCertificate" pointer. It does not
> guard the calculation of the pointer itself:
> 
>   WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
> 
> This is wrong; if we don't know for sure that we have enough room for a
> WIN_CERTIFICATE, then even creating such a pointer, not just
> de-referencing it, may invoke undefined behavior.
> 
> Move the pointer calculation after the size check.
> 
> Cc: Jian J Wang 
> Cc: Jiewen Yao 
> Cc: Min Xu 
> Cc: Wenyi Xie 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215
> Signed-off-by: Laszlo Ersek 

Tested-by: Wenyi Xie 

> ---
>  SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 8 
> +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git 
> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index 377feebb205a..100739eb3eb6 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> @@ -1855,10 +1855,12 @@ DxeImageVerificationHandler (
>for (OffSet = SecDataDir->VirtualAddress;
> OffSet < SecDataDirEnd;
> OffSet += (WinCertificate->dwLength + ALIGN_SIZE 
> (WinCertificate->dwLength))) {
> -WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
>  SecDataDirLeft = SecDataDirEnd - OffSet;
> -if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
> -SecDataDirLeft < WinCertificate->dwLength) {
> +if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) {
> +  break;
> +}
> +WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
> +if (SecDataDirLeft < WinCertificate->dwLength) {
>break;
>  }
>  
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64930): https://edk2.groups.io/g/devel/message/64930
Mute This Topic: https://groups.io/mt/76552539/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 3/3] SecurityPkg/DxeImageVerificationLib: catch alignment overflow (CVE-2019-14562)

[wenyi,xie via groups.io]

On 2020/9/1 17:12, Laszlo Ersek wrote:
> The DxeImageVerificationHandler() function currently checks whether
> "SecDataDir" has enough room for "WinCertificate->dwLength". However, for
> advancing "OffSet", "WinCertificate->dwLength" is aligned to the next
> multiple of 8. If "WinCertificate->dwLength" is large enough, the
> alignment will return 0, and "OffSet" will be stuck at the same value.
> 
> Check whether "SecDataDir" has room left for both
> "WinCertificate->dwLength" and the alignment.
> 
> Cc: Jian J Wang 
> Cc: Jiewen Yao 
> Cc: Min Xu 
> Cc: Wenyi Xie 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215
> Signed-off-by: Laszlo Ersek 

Tested-by: Wenyi Xie 

> ---
>  SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 4 
> +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git 
> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index 100739eb3eb6..11154b6cc58a 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> @@ -1860,7 +1860,9 @@ DxeImageVerificationHandler (
>break;
>  }
>  WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
> -if (SecDataDirLeft < WinCertificate->dwLength) {
> +if (SecDataDirLeft < WinCertificate->dwLength ||
> +(SecDataDirLeft - WinCertificate->dwLength <
> + ALIGN_SIZE (WinCertificate->dwLength))) {
>break;
>  }
>  
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64931): https://edk2.groups.io/g/devel/message/64931
Mute This Topic: https://groups.io/mt/76552541/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/3] SecurityPkg/DxeImageVerificationLib: extract SecDataDirEnd, SecDataDirLeft

[wenyi,xie via groups.io]

On 2020/9/1 17:12, Laszlo Ersek wrote:
> The following two quantities:
> 
>   SecDataDir->VirtualAddress + SecDataDir->Size
>   SecDataDir->VirtualAddress + SecDataDir->Size - OffSet
> 
> are used multiple times in DxeImageVerificationHandler(). Introduce helper
> variables for them: "SecDataDirEnd" and "SecDataDirLeft", respectively.
> This saves us multiple calculations and significantly simplifies the code.
> 
> Note that all three summands above have type UINT32, therefore the new
> variables are also of type UINT32.
> 
> This patch does not change behavior.
> 
> (Note that the code already handles the case when the
> 
>   SecDataDir->VirtualAddress + SecDataDir->Size
> 
> UINT32 addition overflows -- namely, in that case, the certificate loop is
> never entered, and the corruption check right after the loop fires.)
> 
> Cc: Jian J Wang 
> Cc: Jiewen Yao 
> Cc: Min Xu 
> Cc: Wenyi Xie 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215
> Signed-off-by: Laszlo Ersek 

Tested-by: Wenyi Xie 

> ---
>  SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 12 
> 
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git 
> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index b08fe24e85aa..377feebb205a 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> @@ -1652,6 +1652,8 @@ DxeImageVerificationHandler (
>UINT8*AuthData;
>UINTNAuthDataSize;
>EFI_IMAGE_DATA_DIRECTORY *SecDataDir;
> +  UINT32   SecDataDirEnd;
> +  UINT32   SecDataDirLeft;
>UINT32   OffSet;
>CHAR16   *NameStr;
>RETURN_STATUSPeCoffStatus;
> @@ -1849,12 +1851,14 @@ DxeImageVerificationHandler (
>// "Attribute Certificate Table".
>// The first certificate starts at offset (SecDataDir->VirtualAddress) 
> from the start of the file.
>//
> +  SecDataDirEnd = SecDataDir->VirtualAddress + SecDataDir->Size;
>for (OffSet = SecDataDir->VirtualAddress;
> -   OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
> +   OffSet < SecDataDirEnd;
> OffSet += (WinCertificate->dwLength + ALIGN_SIZE 
> (WinCertificate->dwLength))) {
>  WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
> -if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof 
> (WIN_CERTIFICATE) ||
> -(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < 
> WinCertificate->dwLength) {
> +SecDataDirLeft = SecDataDirEnd - OffSet;
> +if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
> +SecDataDirLeft < WinCertificate->dwLength) {
>break;
>  }
>  
> @@ -1948,7 +1952,7 @@ DxeImageVerificationHandler (
>  }
>}
>  
> -  if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size)) {
> +  if (OffSet != SecDataDirEnd) {
>  //
>  // The Size in Certificate Table or the attribute certificate table is 
> corrupted.
>  //
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64929): https://edk2.groups.io/g/devel/message/64929
Mute This Topic: https://groups.io/mt/76552540/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] Basetools as a pip module

[Andrew Fish via groups.io]

> On Sep 1, 2020, at 4:35 PM, Matthew Carlson  wrote:
> 
> Hello all,
> 
> A recent topic on the RFC mailing list went out and the work on moving 
> Basetools/Sources/Python to a separate repo has started. See the RFC 
> conversation here: https://edk2.groups.io/g/rfc/topic/74009714#270 
> 
> 
> The repo in question is here: https://github.com/tianocore/edk2-basetools 
> 
> 
> The current plan is shortly after the stable tag is created, a series of 
> patches will come into edk2 that redirects the build system into the new 
> python module as well as adds additional documentation. You can see a sample 
> of this work here: https://github.com/matthewfcarlson/edk2 
>  as this has a branch that has the 
> work required to use the basetools pip module. The patches won't delete the 
> Basetools/Sources/Python folder but will allow users to select between them. 
> After a certain grace period, the python folder will be deleted and the pip 
> module will be the de facto way of using basetools.
> 
> Three questions need to be answered:
> 
> 1. After the patches that enable the pip module land, how long should the 
> grace period be?
> 2. During the grace period, should basetools commits land in both places or 
> just in the edk2-basetools directory?
> 3. How should the user be able to select which basetools to use (the one in 
> EDK2 or the pip module)? Currently the approach being considered is a simple 
> environmental variable? One of the key considerations is transparency since 
> it won't be apparent what is being used for a particular build without some 
> sort of mechanism to notify the developer. With two seperate versions of 
> Basetools, it becomes very easy for the version of basetools you're using to 
> not be the one you expect.
> 

Matthew,

I’ll throw out some current developer centric ideas. 

1) If you `source edksetup.sh` (edksetup.bat) you get the current behavior, and 
you add an argument you get the pip flavor? So maybe `edksetup.bat 
pip-basetools`?
2) We have similar issues to this with env variables and the build command 
dumps them out when it runs. Can we use the current EDK_TOOL_PATH? Or maybe add 
an extra print to show that the pip module is being used?

Thanks,

Andrew Fish


> Thank you.
> -Matthew Carlson
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64928): https://edk2.groups.io/g/devel/message/64928
Mute This Topic: https://groups.io/mt/76572200/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: 回复: [edk2-devel] [Patch V2] BaseTools/Ecc: Fix an issue of path separator compatibility

[Bob Feng]

Thank you Laszlo.

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Laszlo Ersek
Sent: Wednesday, September 2, 2020 2:04 AM
To: devel@edk2.groups.io; gaolim...@byosoft.com.cn; Feng, Bob C 

Cc: Chen, Christine ; Zhang, Shenglei 

Subject: Re: 回复: [edk2-devel] [Patch V2] BaseTools/Ecc: Fix an issue of path 
separator compatibility

On 09/01/20 16:42, gaoliming wrote:
> Reviewed-by: Liming Gao 

Merged in commit 751355992635, via
.

Thanks
Laszlo

>> -邮件原件-
>> 发件人: bounce+27952+64887+4905953+8761...@groups.io
>>  代表 Bob Feng
>> 发送时间: 2020年9月1日 18:23
>> 收件人: devel@edk2.groups.io
>> 抄送: Liming Gao ; Yuwei Chen 
>> ; Shenglei Zhang 
>> 主题: [edk2-devel] [Patch V2] BaseTools/Ecc: Fix an issue of path
> separator
>> compatibility
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2904
>>
>> The path separator is different in Windows and Linux, the original 
>> code does not handle this difference. This patch is to fix this 
>> issue.
>>
>> Signed-off-by: Bob Feng 
>> Cc: Liming Gao 
>> Cc: Yuwei Chen 
>> Cc: Shenglei Zhang 
>> ---
>> V2
>> Change to a better method to get path separator.
>>
>>  BaseTools/Source/Python/Ecc/Check.py | 8 
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/BaseTools/Source/Python/Ecc/Check.py
>> b/BaseTools/Source/Python/Ecc/Check.py
>> index 0fdc7e35c1..6087abfa4d 100644
>> --- a/BaseTools/Source/Python/Ecc/Check.py
>> +++ b/BaseTools/Source/Python/Ecc/Check.py
>> @@ -1101,15 +1101,15 @@ class Check(object):
>>  for Item in InfPathSet:
>>  if Item[0] not in InfPathList:
>>  InfPathList.append(Item[0])
>>  SqlCommand = """
>>   select ID, Path, FullPath from File where
>> upper(FullPath) not in
>> -(select upper(A.Path) || '\\' ||
>> upper(B.Value1) from File as A, INF as B
>> +(select upper(A.Path) || '%s' ||
>> upper(B.Value1) from File as A, INF as B
>>  where A.ID in (select BelongsToFile from 
>> INF where Model = %s group by BelongsToFile) and
>>  B.BelongsToFile = A.ID and B.Model = %s)
>>  and (Model = %s or Model = %s)
>> -""" % (MODEL_EFI_SOURCE_FILE,
>> MODEL_EFI_SOURCE_FILE, MODEL_FILE_C, MODEL_FILE_H)
>> +""" % (os.sep, MODEL_EFI_SOURCE_FILE,
>> MODEL_EFI_SOURCE_FILE, MODEL_FILE_C, MODEL_FILE_H)
>>  RecordSet = EccGlobalData.gDb.TblInf.Exec(SqlCommand)
>>  for Record in RecordSet:
>>  Path = Record[1]
>>  Path = Path.upper().replace('\X64', 
>> '').replace('\IA32', '').replace('\EBC', '').replace('\IPF', 
>> '').replace('\ARM', '')
>>  if Path in InfPathList:
>> @@ -1130,13 +1130,13 @@ class Check(object):
>>  if Pcd[3]:
>>  PcdName = Pcd[3]
>>  BelongsToFile = Pcd[4]
>>  SqlCommand = """
>>   select ID from File where FullPath in
>> -(select B.Path || '\\' || A.Value1 from INF
>> as A, File as B where A.Model = %s and A.BelongsToFile = %s
>> +(select B.Path || '%s' || A.Value1 from
>> INF as A, File as B where A.Model = %s and A.BelongsToFile = %s
>>   and B.ID = %s and (B.Model = %s or 
>> B.Model = %s))
>> - """ % (MODEL_EFI_SOURCE_FILE,
>> BelongsToFile, BelongsToFile, MODEL_FILE_C, MODEL_FILE_H)
>> + """ % (os.sep,
>> MODEL_EFI_SOURCE_FILE, BelongsToFile, BelongsToFile, MODEL_FILE_C,
>> MODEL_FILE_H)
>>  TableSet =
>> EccGlobalData.gDb.TblFile.Exec(SqlCommand)
>>  for Tbl in TableSet:
>>  TblName = 'Identifier' + str(Tbl[0])
>>  SqlCommand = """
>>   select Name, ID from %s where value 
>> like '%s' and Model = %s
>> --
>> 2.20.1.windows.1
>>
>>
>>
> 
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64927): https://edk2.groups.io/g/devel/message/64927
Mute This Topic: https://groups.io/mt/76557763/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Add check for CR3/GDT/IDT.

[Dong, Eric]

AP needs to run from real mode to 32bit mode to LONG mode. Page table
(pointed by CR3) and GDT are necessary to set up to correct value when
CPU execution mode is switched to LONG mode.
AP uses the same location page table (CR3) and GDT as what BSP uses.
But when the page table or GDT is above 4GB, it's impossible for CPU
to use because GDTR.base and CR3 are 32bits before switching to LONG
mode.
This patch adds check for the CR3, GDT.Base and IDT.Base to not above
32 bits restriction.

Signed-off-by: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
---
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c |   8 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.c| 108 +++-
 UefiCpuPkg/Library/MpInitLib/MpLib.h|   6 +-
 3 files changed, 97 insertions(+), 25 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c 
b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 2c00d72dde..27f12a75a8 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -393,13 +393,19 @@ MpInitChangeApLoopCallback (
   )
 {
   CPU_MP_DATA   *CpuMpData;
+  EFI_STATUSStatus;
 
   CpuMpData = GetCpuMpData ();
   CpuMpData->PmCodeSegment = GetProtectedModeCS ();
   CpuMpData->Pm16CodeSegment = GetProtectedMode16CS ();
   CpuMpData->ApLoopMode = PcdGet8 (PcdCpuApLoopMode);
   mNumberToFinish = CpuMpData->CpuCount - 1;
-  WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL, TRUE);
+  Status = WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, NULL, TRUE);
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "ERROR :: %a() Change Ap Loop Mode failed!\n", 
__FUNCTION__));
+return;
+  }
+
   while (mNumberToFinish > 0) {
 CpuPause ();
   }
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 07426274f6..21b17a7b40 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -470,7 +470,7 @@ GetProcessorNumber (
 
   @return  CPU count detected
 **/
-UINTN
+EFI_STATUS
 CollectProcessorCount (
   IN CPU_MP_DATA *CpuMpData
   )
@@ -478,12 +478,17 @@ CollectProcessorCount (
   UINTN  Index;
   CPU_INFO_IN_HOB*CpuInfoInHob;
   BOOLEANX2Apic;
+  EFI_STATUS Status;
 
   //
   // Send 1st broadcast IPI to APs to wakeup APs
   //
   CpuMpData->InitFlag = ApInitConfig;
-  WakeUpAP (CpuMpData, TRUE, 0, NULL, NULL, TRUE);
+  Status = WakeUpAP (CpuMpData, TRUE, 0, NULL, NULL, TRUE);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
   CpuMpData->InitFlag = ApInitDone;
   ASSERT (CpuMpData->CpuCount <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
   //
@@ -520,7 +525,11 @@ CollectProcessorCount (
 //
 // Wakeup all APs to enable x2APIC mode
 //
-WakeUpAP (CpuMpData, TRUE, 0, ApFuncEnableX2Apic, NULL, TRUE);
+Status = WakeUpAP (CpuMpData, TRUE, 0, ApFuncEnableX2Apic, NULL, TRUE);
+if (EFI_ERROR (Status)) {
+  return Status;
+}
+
 //
 // Wait for all known APs finished
 //
@@ -546,7 +555,7 @@ CollectProcessorCount (
 
   DEBUG ((DEBUG_INFO, "MpInitLib: Find %d processors in system.\n", 
CpuMpData->CpuCount));
 
-  return CpuMpData->CpuCount;
+  return EFI_SUCCESS;
 }
 
 /**
@@ -990,7 +999,7 @@ WaitApWakeup (
   @param[in] CpuMpData  Pointer to CPU MP Data
 
 **/
-VOID
+EFI_STATUS
 FillExchangeInfoData (
   IN CPU_MP_DATA   *CpuMpData
   )
@@ -1001,6 +1010,35 @@ FillExchangeInfoData (
   IA32_CR4 Cr4;
 
   ExchangeInfo  = CpuMpData->MpCpuExchangeInfo;
+  ExchangeInfo->Cr3 = AsmReadCr3 ();
+  if (ExchangeInfo->Cr3 > 0x) {
+//
+// AP needs to run from real mode to 32bit mode to LONG mode. Page table
+// (pointed by CR3) and GDT are necessary to set up to correct value when
+// CPU execution mode is switched to LONG mode.
+// AP uses the same location page table (CR3) and GDT as what BSP uses.
+// But when the page table or GDT is above 4GB, it's impossible for CPU
+// to use because GDTR.base and CR3 are 32bits before switching to LONG
+// mode.
+// Here add check for the CR3, GDT.Base and IDT.Base to not above 32 bits
+// limitation.
+//
+return EFI_UNSUPPORTED;
+  }
+
+  //
+  // Get the BSP's data of GDT and IDT
+  //
+  AsmReadGdtr ((IA32_DESCRIPTOR *) >GdtrProfile);
+  if (ExchangeInfo->GdtrProfile.Base > 0x) {
+return EFI_UNSUPPORTED;
+  }
+
+  AsmReadIdtr ((IA32_DESCRIPTOR *) >IdtrProfile);
+  if (ExchangeInfo->IdtrProfile.Base > 0x) {
+return EFI_UNSUPPORTED;
+  }
+
   ExchangeInfo->Lock= 0;
   ExchangeInfo->StackStart  = CpuMpData->Buffer;
   ExchangeInfo->StackSize   = CpuMpData->CpuApStackSize;
@@ -1009,9 +1047,6 @@ FillExchangeInfoData (
 
   ExchangeInfo->CodeSegment = AsmReadCs ();
   ExchangeInfo->DataSegment = AsmReadDs ();
-
-  ExchangeInfo->Cr3 = AsmReadCr3 ();
-
   ExchangeInfo->CFunction

回复: 回复: [edk2-devel] [Patch V2] BaseTools/Ecc: Fix an issue of path separator compatibility

[gaoliming]

Laszlo:
  Thank you!

Liming
> -邮件原件-
> 发件人: bounce+27952+64917+4905953+8761...@groups.io
>  代表 Laszlo Ersek
> 发送时间: 2020年9月2日 2:04
> 收件人: devel@edk2.groups.io; gaolim...@byosoft.com.cn;
> bob.c.f...@intel.com
> 抄送: 'Yuwei Chen' ; 'Shenglei Zhang'
> 
> 主题: Re: 回复: [edk2-devel] [Patch V2] BaseTools/Ecc: Fix an issue of path
> separator compatibility
> 
> On 09/01/20 16:42, gaoliming wrote:
> > Reviewed-by: Liming Gao 
> 
> Merged in commit 751355992635, via
> .
> 
> Thanks
> Laszlo
> 
> >> -邮件原件-
> >> 发件人: bounce+27952+64887+4905953+8761...@groups.io
> >>  代表 Bob Feng
> >> 发送时间: 2020年9月1日 18:23
> >> 收件人: devel@edk2.groups.io
> >> 抄送: Liming Gao ; Yuwei Chen
> >> ; Shenglei Zhang 
> >> 主题: [edk2-devel] [Patch V2] BaseTools/Ecc: Fix an issue of path
> > separator
> >> compatibility
> >>
> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2904
> >>
> >> The path separator is different in Windows and Linux, the
> >> original code does not handle this difference. This patch
> >> is to fix this issue.
> >>
> >> Signed-off-by: Bob Feng 
> >> Cc: Liming Gao 
> >> Cc: Yuwei Chen 
> >> Cc: Shenglei Zhang 
> >> ---
> >> V2
> >> Change to a better method to get path separator.
> >>
> >>  BaseTools/Source/Python/Ecc/Check.py | 8 
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/BaseTools/Source/Python/Ecc/Check.py
> >> b/BaseTools/Source/Python/Ecc/Check.py
> >> index 0fdc7e35c1..6087abfa4d 100644
> >> --- a/BaseTools/Source/Python/Ecc/Check.py
> >> +++ b/BaseTools/Source/Python/Ecc/Check.py
> >> @@ -1101,15 +1101,15 @@ class Check(object):
> >>  for Item in InfPathSet:
> >>  if Item[0] not in InfPathList:
> >>  InfPathList.append(Item[0])
> >>  SqlCommand = """
> >>   select ID, Path, FullPath from File where
> >> upper(FullPath) not in
> >> -(select upper(A.Path) || '\\' ||
> >> upper(B.Value1) from File as A, INF as B
> >> +(select upper(A.Path) || '%s' ||
> >> upper(B.Value1) from File as A, INF as B
> >>  where A.ID in (select BelongsToFile
> from
> >> INF where Model = %s group by BelongsToFile) and
> >>  B.BelongsToFile = A.ID and B.Model
> = %s)
> >>  and (Model = %s or Model = %s)
> >> -""" % (MODEL_EFI_SOURCE_FILE,
> >> MODEL_EFI_SOURCE_FILE, MODEL_FILE_C, MODEL_FILE_H)
> >> +""" % (os.sep, MODEL_EFI_SOURCE_FILE,
> >> MODEL_EFI_SOURCE_FILE, MODEL_FILE_C, MODEL_FILE_H)
> >>  RecordSet = EccGlobalData.gDb.TblInf.Exec(SqlCommand)
> >>  for Record in RecordSet:
> >>  Path = Record[1]
> >>  Path = Path.upper().replace('\X64', '').replace('\IA32',
> >> '').replace('\EBC', '').replace('\IPF', '').replace('\ARM', '')
> >>  if Path in InfPathList:
> >> @@ -1130,13 +1130,13 @@ class Check(object):
> >>  if Pcd[3]:
> >>  PcdName = Pcd[3]
> >>  BelongsToFile = Pcd[4]
> >>  SqlCommand = """
> >>   select ID from File where FullPath in
> >> -(select B.Path || '\\' || A.Value1 from
> INF
> >> as A, File as B where A.Model = %s and A.BelongsToFile = %s
> >> +(select B.Path || '%s' || A.Value1 from
> >> INF as A, File as B where A.Model = %s and A.BelongsToFile = %s
> >>   and B.ID = %s and (B.Model = %s or
> >> B.Model = %s))
> >> - """ % (MODEL_EFI_SOURCE_FILE,
> >> BelongsToFile, BelongsToFile, MODEL_FILE_C, MODEL_FILE_H)
> >> + """ % (os.sep,
> >> MODEL_EFI_SOURCE_FILE, BelongsToFile, BelongsToFile, MODEL_FILE_C,
> >> MODEL_FILE_H)
> >>  TableSet =
> >> EccGlobalData.gDb.TblFile.Exec(SqlCommand)
> >>  for Tbl in TableSet:
> >>  TblName = 'Identifier' + str(Tbl[0])
> >>  SqlCommand = """
> >>   select Name, ID from %s where
> >> value like '%s' and Model = %s
> >> --
> >> 2.20.1.windows.1
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
> 
> 
> 




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64925): https://edk2.groups.io/g/devel/message/64925
Mute This Topic: https://groups.io/mt/76573343/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Basetools as a pip module

[Matthew Carlson]

Hello all,

A recent topic on the RFC mailing list went out and the work on moving
Basetools/Sources/Python to a separate repo has started. See the RFC
conversation here: https://edk2.groups.io/g/rfc/topic/74009714#270

The repo in question is here: https://github.com/tianocore/edk2-basetools

The current plan is shortly after the stable tag is created, a series of
patches will come into edk2 that redirects the build system into the new
python module as well as adds additional documentation. You can see a
sample of this work here: https://github.com/matthewfcarlson/edk2 as this
has a branch that has the work required to use the basetools pip module.
The patches won't delete the Basetools/Sources/Python folder but will allow
users to select between them. After a certain grace period, the python
folder will be deleted and the pip module will be the de facto way of using
basetools.

Three questions need to be answered:

1. After the patches that enable the pip module land, how long should the
grace period be?
2. During the grace period, should basetools commits land in both places or
just in the edk2-basetools directory?
3. How should the user be able to select which basetools to use (the one in
EDK2 or the pip module)? Currently the approach being considered is a
simple environmental variable? One of the key considerations is
transparency since it won't be apparent what is being used for a particular
build without some sort of mechanism to notify the developer. With two
seperate versions of Basetools, it becomes very easy for the version of
basetools you're using to not be the one you expect.

Thank you.
-Matthew Carlson

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64924): https://edk2.groups.io/g/devel/message/64924
Mute This Topic: https://groups.io/mt/76572200/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-staging/EdkRepo] [PATCH V2 2/2] EdkRepo: Add support for subst drives

[Carsey, Jaben]

Instead of looping over the dictionary, could we convert the target drive and 
see if it's in the dictionary? 

I am thinking like remove the : from 
os.path.splitdrive(workspace_dir)[0].upper() string and see if that's in the 
dictionary?


> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Nate
> DeSimone
> Sent: Monday, August 31, 2020 12:28 PM
> To: devel@edk2.groups.io
> Cc: Desimone, Ashley E ; Pandya, Puja
> ; Bret Barkelew ;
> Agyeman, Prince ; Bjorge, Erik C
> 
> Subject: [edk2-devel] [edk2-staging/EdkRepo] [PATCH V2 2/2] EdkRepo: Add
> support for subst drives
> 
> get_workspace_path() now converts a virtual drive path to a real path before
> any git repo operations are done.
> 
> Cc: Ashley E Desimone 
> Cc: Puja Pandya 
> Cc: Bret Barkelew 
> Cc: Prince Agyeman 
> Cc: Erik Bjorge 
> Signed-off-by: Nate DeSimone 
> ---
>  edkrepo/commands/clone_command.py |  6 ++
> edkrepo/config/config_factory.py  | 10 +-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/edkrepo/commands/clone_command.py
> b/edkrepo/commands/clone_command.py
> index f638090..be5ad86 100644
> --- a/edkrepo/commands/clone_command.py
> +++ b/edkrepo/commands/clone_command.py
> @@ -20,6 +20,7 @@ from edkrepo.common.edkrepo_exception import
> EdkrepoInvalidParametersException,
>  from edkrepo.common.edkrepo_exception import
> EdkrepoManifestNotFoundException  from edkrepo.common.humble
> import CLONE_INVALID_WORKSPACE, CLONE_INVALID_PROJECT_ARG,
> CLONE_INVALID_COMBO_ARG  from edkrepo.common.humble import
> SPARSE_CHECKOUT, CLONE_INVALID_LOCAL_ROOTS
> +from edkrepo.common.pathfix import get_subst_drive_dict
>  from edkrepo.common.workspace_maintenance.workspace_maintenance
> import case_insensitive_single_match  from
> edkrepo.common.workspace_maintenance.manifest_repos_maintenance
> import pull_all_manifest_repos, find_project_in_all_indices  from
> edkrepo.common.workspace_maintenance.manifest_repos_maintenance
> import list_available_manifest_repos @@ -77,6 +78,11 @@ class
> CloneCommand(EdkrepoCommand):
>  workspace_dir = os.getcwd()
>  else:
>  workspace_dir = os.path.abspath(workspace_dir)
> +subst = get_subst_drive_dict()
> +for drive in subst.keys():
> +if '{}:'.format(drive) == 
> os.path.splitdrive(workspace_dir)[0].upper():
> +workspace_dir = os.path.join(subst[drive],
> os.path.splitdrive(workspace_dir)[1][1:])
> +workspace_dir = os.path.normpath(workspace_dir)
>  if os.path.isdir(workspace_dir) and os.listdir(workspace_dir):
>  raise
> EdkrepoInvalidParametersException(CLONE_INVALID_WORKSPACE)
>  if not os.path.isdir(workspace_dir):
> diff --git a/edkrepo/config/config_factory.py
> b/edkrepo/config/config_factory.py
> index a82a438..1cb6eb7 100644
> --- a/edkrepo/config/config_factory.py
> +++ b/edkrepo/config/config_factory.py
> @@ -11,13 +11,16 @@ import os
>  import sys
>  import configparser
>  import collections
> -from ctypes import *
> +if sys.platform == "win32":
> +from ctypes import oledll, c_void_p, c_uint32, c_wchar_p
> +from ctypes import create_unicode_buffer
> 
>  import edkrepo.config.humble.config_factory_humble as humble  from
> edkrepo.common.edkrepo_exception import
> EdkrepoGlobalConfigNotFoundException, EdkrepoConfigFileInvalidException
> from edkrepo.common.edkrepo_exception import
> EdkrepoWorkspaceInvalidException,
> EdkrepoGlobalDataDirectoryNotFoundException
>  from edkrepo.common.edkrepo_exception import
> EdkrepoConfigFileReadOnlyException
>  from edkrepo.common.humble import
> MIRROR_PRIMARY_REPOS_MISSING, MIRROR_DECODE_WARNING,
> MAX_PATCH_SET_INVALID
> +from edkrepo.common.pathfix import get_subst_drive_dict
>  from edkrepo_manifest_parser import edk_manifest  from
> edkrepo.common.pathfix import expanduser
> 
> @@ -238,6 +241,11 @@ def get_workspace_path():
>  while True:
>  if os.path.isdir(os.path.join(path, "repo")):
>  if os.path.isfile(os.path.join(os.path.join(path, "repo"),
> "Manifest.xml")):
> +if sys.platform == "win32":
> +subst = get_subst_drive_dict()
> +for drive in subst.keys():
> +if '{}:'.format(drive) == 
> os.path.splitdrive(path)[0].upper():
> +path = os.path.join(subst[drive],
> + os.path.splitdrive(path)[1][1:])
>  return path
>  if os.path.dirname(path) == path:
>  break
> --
> 2.27.0.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64923): https://edk2.groups.io/g/devel/message/64923
Mute This Topic: https://groups.io/mt/76541439/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Cancelling September Community meeting

[Soumya Guptha]

Dear Community members,
I plan on cancelling the September 3rd community meeting due to the holiday 
week (labor day) in US and no updates from the Stewards call since this week's 
Stewards meeting has also been cancelled.

Let's resume during October. Please send me any topics that you like to discuss 
during next community meeting.

Thanks,
Soumya


Soumya Guptha
Open Source Firmware PM, SFP/IAGS




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64922): https://edk2.groups.io/g/devel/message/64922
Mute This Topic: https://groups.io/mt/76569341/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Cancelled Event: TianoCore Community Meeting - EMEA / NAMO - Thursday, 3 September 2020 #cal-cancelled

[devel@edk2.groups.io Calendar]

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Groups.io Inc//Groups.io Calendar//EN
METHOD:CANCELLED
CALSCALE:GREGORIAN
BEGIN:VEVENT
UID:1ao6.1590558161886501237.j...@groups.io
DTSTAMP:20200901T210321Z
ORGANIZER;CN=Soumya Guptha:mailto:soumya.k.gup...@intel.com
DTSTART:20200903T16Z
DTEND:20200903T17Z
SUMMARY:TianoCore Community Meeting - EMEA / NAMO
DESCRIPTION:Meeting URL\n\nhttps://bluejeans.com/889357567?src=join_info\
 n\nMeeting ID\n\n889 357 567\n\nWant to dial in from a phone?\n\nDial one
  of the following numbers:\n\n+1.408.740.7256 (US (San Jose))\n\n+1.408.3
 17.9253 (US (Primary\, San Jose))\n\n(see all numbers - https://www.bluej
 eans.com/numbers ( https://www.bluejeans.com/numbers ) )\n\nEnter the mee
 ting ID and passcode followed by #
LOCATION:https://bluejeans.com/889357567?src=join_info
SEQUENCE:999
STATUS:CANCELLED
END:VEVENT
END:VCALENDAR


invite.ics
Description: application/ics

[edk2-devel] Cancelled Event: TianoCore Community Meeting - APAC / NAMO - Thursday, 3 September 2020 #cal-cancelled

[devel@edk2.groups.io Calendar]

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Groups.io Inc//Groups.io Calendar//EN
METHOD:CANCELLED
CALSCALE:GREGORIAN
BEGIN:VEVENT
UID:1i0d.1590558226951005717.t...@groups.io
DTSTAMP:20200901T210352Z
ORGANIZER;CN=Soumya Guptha:mailto:soumya.k.gup...@intel.com
DTSTART:20200904T023000Z
DTEND:20200904T033000Z
SUMMARY:TianoCore Community Meeting - APAC / NAMO
DESCRIPTION:Meeting URL\n\nhttps://bluejeans.com/889357567?src=join_info\
 n\nMeeting ID\n\n889 357 567\n\nWant to dial in from a phone?\n\nDial one
  of the following numbers:\n\n+1.408.740.7256 (US (San Jose))\n\n+1.408.3
 17.9253 (US (Primary\, San Jose))\n\n(see all numbers - https://www.bluej
 eans.com/numbers ( https://www.bluejeans.com/numbers ) )\n\nEnter the mee
 ting ID and passcode followed by #
LOCATION:https://bluejeans.com/889357567?src=join_info
SEQUENCE:999
STATUS:CANCELLED
END:VEVENT
END:VCALENDAR


invite.ics
Description: application/ics

[edk2-devel] [PATCH edk2-platforms 1/1] SbsaQemu: AcpiDxe: Read MPIDR from device tree

[Tanmay Jagdale]

The Qemu device tree for Sbsa platform now contains MPIDR value
for every CPU in the form of "reg" property under every CPU's
node. Hence, add a function that provides support to read this
value from the device tree.

Signed-off-by: Tanmay Jagdale 
---
 .../Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c | 35 ++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c 
b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
index 47a9bd1d423a..fb7c1835c3d7 100644
--- a/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
+++ b/Silicon/Qemu/SbsaQemu/Drivers/SbsaQemuAcpiDxe/SbsaQemuAcpiDxe.c
@@ -22,6 +22,9 @@
 #include 
 #include 
 
+STATIC INT32 FdtFirstCpuOffset;
+STATIC INT32 FdtCpuNodeSize;
+
 /*
  * A function that walks through the Device Tree created
  * by Qemu and counts the number of CPUs present in it.
@@ -56,12 +59,14 @@ CountCpusFromFdt (
   // The count of these subnodes corresponds to the number of
   // CPUs created by Qemu.
   Prev = fdt_first_subnode (DeviceTreeBase, CpuNode);
+  FdtFirstCpuOffset = Prev;
   while (1) {
 CpuCount++;
 Node = fdt_next_subnode (DeviceTreeBase, Prev);
 if (Node < 0) {
   break;
 }
+FdtCpuNodeSize = Node - Prev;
 Prev = Node;
   }
 
@@ -69,6 +74,34 @@ CountCpusFromFdt (
   ASSERT_RETURN_ERROR (PcdStatus);
 }
 
+/*
+ * Get MPIDR from device tree passed by Qemu
+ */
+STATIC
+UINT64
+GetMpidr (
+  IN UINTN   CpuId
+  )
+{
+  VOID   *DeviceTreeBase;
+  CONST UINT64   *RegVal;
+  INT32  Len;
+
+  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
+  ASSERT (DeviceTreeBase != NULL);
+
+  RegVal = fdt_getprop (DeviceTreeBase,
+ FdtFirstCpuOffset + (CpuId * FdtCpuNodeSize),
+ "reg",
+ );
+  if (!RegVal) {
+DEBUG ((DEBUG_ERROR, "Couldn't find reg property for CPU:%d\n", CpuId));
+return 0;
+  }
+
+  return (fdt64_to_cpu (ReadUnaligned64 (RegVal)));
+}
+
 /*
  * A Function to Compute the ACPI Table Checksum
  */
@@ -173,7 +206,7 @@ AddMadtTable (
 CopyMem (New, , sizeof (EFI_ACPI_6_0_GIC_STRUCTURE));
 GiccPtr = (EFI_ACPI_6_0_GIC_STRUCTURE *) New;
 GiccPtr->AcpiProcessorUid = NumCores;
-GiccPtr->MPIDR = NumCores;
+GiccPtr->MPIDR = GetMpidr (NumCores);
 New += sizeof (EFI_ACPI_6_0_GIC_STRUCTURE);
   }
 
-- 
2.28.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64919): https://edk2.groups.io/g/devel/message/64919
Mute This Topic: https://groups.io/mt/76565197/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 0/6] Extend Last Attempt Status Usage

[Michael Kubacki]

Hi Mike,

I agree that's a good way to meet the requirement. Maintaining the two 
APIs would just be a transient state (i.e. on order of days) to update 
edk2-platforms users and then we could remove the old API, right?


Our main concern was maintaining the two APIs side-by-side long term.

Thanks,
Michael

On 8/28/2020 12:25 PM, Kinney, Michael D wrote:

Hi Michael,

There are implementations of FMpDeviceLib in edk2-platforms.  So the challenge 
is how
to prepare a set of patch series to edk2 and edk2-platforms where there are no
intermediate states that break builds.  Adding new APIs and later removing the
old APIs is one way to meet this requirement.  Do you have other ideas?

Mike


-Original Message-
From: devel@edk2.groups.io  On Behalf Of Michael Kubacki
Sent: Friday, August 28, 2020 12:21 PM
To: Sean Brogan ; devel@edk2.groups.io; Kinney, Michael D 

Cc: Gao, Liming ; Jiang, Guomin ; Xu, 
Wei6 ; Liu, Zhiguang

Subject: Re: [edk2-devel] [PATCH v3 0/6] Extend Last Attempt Status Usage

Hi Mike,

We've stated a few reasons we prefer to change the library API now. Do
you find that acceptable for v4?

Thanks,
Michael

On 8/21/2020 2:25 PM, Michael Kubacki wrote:

Hi Sean,

Thanks for the feedback.

#1 - I will include both suggestions in v4.

Thanks,
Michael

On 8/20/2020 7:37 PM, Sean Brogan wrote:

Michael,

#1
I would suggest calling out the sub-range

0x10A0 | 0x17FF  -- Free for Library Implementations of FmpDevicePkg

I also might suggest splitting FmpDependencyLib and
FmpDependencyCheckLib ranges just to show a consistent pattern of how
each library instance within FmpDevicePkg gets a defined range.


#2
Given that edk2 does not have any real FmpDeviceLibs (only instance is
FmpDevicePkg\Library\FmpDeviceLibNull\FmpDeviceLibNull.inf).  Now is
the best time to make a breaking change. It is very common for
downstream code repositories to integrate edk2 and be forced to make
changes associated with the new integration.  To me this is
preferred.  A build break is easy to resolve. When functionality
changes or new features are added but don't cause a break this is more
difficult to integrate correctly.  Not only that, it leads to nearly
everyone ignoring the change.  There is no need for this change to be
a multi-year integration or cause extra development of shims and
translation functions.  The API change is pretty easy.  The
implementation can choose to to avoid the new ranges and just set the
value to 1 (FMP unknown error).  This would match the behavior of
today.  Obviously i believe it would better to take the extra time to
create unique ranges for each of your libs.  Also note that if anyone
is doing third party binary integration this is not a breaking
change.  This only breaks for those doing a src build.

Thanks
Sean






On 8/20/2020 6:25 PM, Michael Kubacki wrote:

Hi Mike,

1) Yes, we can certainly leave more of the unsuccessful vendor range
available for future expansion. I believe we can also reduce the FMP
reserved range. How about a length of 0x800 for both?

The ranges would then be defined as follows:

START | END   | Usage
--|
0x1000    | 0x17FF    | FmpDevicePkg  |
     0x1000 |    0x107F | FmpDxe driver |
     0x1080 |    0x109F | FMP dependencies (e.g. FmpDependencyLib)  |
0x1800    | 0x1FFF    | FmpDeviceLib instances implementation |
0x2000    | 0x3FFF    | Unused. Available for future expansion.   |

Also, I don't see a problem with removing the length defines and
directly specifying min/max defines for each range.

2.) I understand the compatibility concern but as you noted it is
cleaner to maintain a single interface. I believe making the
transition to the new API will only become more difficult in the
future as it may go unnoticed resulting in implementations that don't
implement support for this capability leading to an increasing amount
of future effort to do work that could be done now. Perhaps we could
get thoughts from others as well?

3.) I will update these to return back an expected value.

Thanks,
Michael

On 8/19/2020 7:57 PM, Kinney, Michael D wrote:

Hi Michael,

A couple a couple general questions:

1) I see the entire range from 0x2000-0x3FFF is reserved for
FmpDeviceLib.  If we
     every add more device/platform specific libs for FMP, there are
no ranges available.
     Should we limit the FmpDeviceLib to a smaller range to support
future expansion?

     Also, the style of LastAttemptStatus.h with extra defines for
the length of
     each range is hard to read, and I do not think there are any
consumers of the
     length defines from this public include file.  Since there are
really only 3
     defined ranges, couldn't this be simplified to min/max defines
for each range
     for a total of 6 #defines.  I do not expect ranges (once
defined) to change in
     length, and the most that might happen in the future is 

Re: 回复: [edk2-devel] [Patch V2] BaseTools/Ecc: Fix an issue of path separator compatibility

[Laszlo Ersek]

On 09/01/20 16:42, gaoliming wrote:
> Reviewed-by: Liming Gao 

Merged in commit 751355992635, via
.

Thanks
Laszlo

>> -邮件原件-
>> 发件人: bounce+27952+64887+4905953+8761...@groups.io
>>  代表 Bob Feng
>> 发送时间: 2020年9月1日 18:23
>> 收件人: devel@edk2.groups.io
>> 抄送: Liming Gao ; Yuwei Chen
>> ; Shenglei Zhang 
>> 主题: [edk2-devel] [Patch V2] BaseTools/Ecc: Fix an issue of path
> separator
>> compatibility
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2904
>>
>> The path separator is different in Windows and Linux, the
>> original code does not handle this difference. This patch
>> is to fix this issue.
>>
>> Signed-off-by: Bob Feng 
>> Cc: Liming Gao 
>> Cc: Yuwei Chen 
>> Cc: Shenglei Zhang 
>> ---
>> V2
>> Change to a better method to get path separator.
>>
>>  BaseTools/Source/Python/Ecc/Check.py | 8 
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/BaseTools/Source/Python/Ecc/Check.py
>> b/BaseTools/Source/Python/Ecc/Check.py
>> index 0fdc7e35c1..6087abfa4d 100644
>> --- a/BaseTools/Source/Python/Ecc/Check.py
>> +++ b/BaseTools/Source/Python/Ecc/Check.py
>> @@ -1101,15 +1101,15 @@ class Check(object):
>>  for Item in InfPathSet:
>>  if Item[0] not in InfPathList:
>>  InfPathList.append(Item[0])
>>  SqlCommand = """
>>   select ID, Path, FullPath from File where
>> upper(FullPath) not in
>> -(select upper(A.Path) || '\\' ||
>> upper(B.Value1) from File as A, INF as B
>> +(select upper(A.Path) || '%s' ||
>> upper(B.Value1) from File as A, INF as B
>>  where A.ID in (select BelongsToFile from
>> INF where Model = %s group by BelongsToFile) and
>>  B.BelongsToFile = A.ID and B.Model = %s)
>>  and (Model = %s or Model = %s)
>> -""" % (MODEL_EFI_SOURCE_FILE,
>> MODEL_EFI_SOURCE_FILE, MODEL_FILE_C, MODEL_FILE_H)
>> +""" % (os.sep, MODEL_EFI_SOURCE_FILE,
>> MODEL_EFI_SOURCE_FILE, MODEL_FILE_C, MODEL_FILE_H)
>>  RecordSet = EccGlobalData.gDb.TblInf.Exec(SqlCommand)
>>  for Record in RecordSet:
>>  Path = Record[1]
>>  Path = Path.upper().replace('\X64', '').replace('\IA32',
>> '').replace('\EBC', '').replace('\IPF', '').replace('\ARM', '')
>>  if Path in InfPathList:
>> @@ -1130,13 +1130,13 @@ class Check(object):
>>  if Pcd[3]:
>>  PcdName = Pcd[3]
>>  BelongsToFile = Pcd[4]
>>  SqlCommand = """
>>   select ID from File where FullPath in
>> -(select B.Path || '\\' || A.Value1 from INF
>> as A, File as B where A.Model = %s and A.BelongsToFile = %s
>> +(select B.Path || '%s' || A.Value1 from
>> INF as A, File as B where A.Model = %s and A.BelongsToFile = %s
>>   and B.ID = %s and (B.Model = %s or
>> B.Model = %s))
>> - """ % (MODEL_EFI_SOURCE_FILE,
>> BelongsToFile, BelongsToFile, MODEL_FILE_C, MODEL_FILE_H)
>> + """ % (os.sep,
>> MODEL_EFI_SOURCE_FILE, BelongsToFile, BelongsToFile, MODEL_FILE_C,
>> MODEL_FILE_H)
>>  TableSet =
>> EccGlobalData.gDb.TblFile.Exec(SqlCommand)
>>  for Tbl in TableSet:
>>  TblName = 'Identifier' + str(Tbl[0])
>>  SqlCommand = """
>>   select Name, ID from %s where
>> value like '%s' and Model = %s
>> --
>> 2.20.1.windows.1
>>
>>
>>
> 
> 
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64917): https://edk2.groups.io/g/devel/message/64917
Mute This Topic: https://groups.io/mt/76557763/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v10 0/5] Use RngLib instead of TimerLib for OpensslLib

[Matthew Carlson]

From: Matthew Carlson 

Hello all,

This patch contains a fix for Bugzilla 1871.
There's been a good bit of community discussion around the topic,
so below follows a general overview of the discussion and what this patch does.

This is the seventh iteration of this patch series, focused on code style and a
few functions being renamed to comply with style.

Back in Devel message#40590 (https://edk2.groups.io/g/devel/message/40590)
around the patch series that updates OpenSSL to 1.1.1b, a comment was made
that suggested that platforms be in charge of the entropy/randomness that
is provided to OpenSSL as currently the entropry source seems to be a
hand-rolled random number generator that uses the PerformanceCounter from
TimerLib. This causes OpenSSL to depend on TimerLib, which is often platform
specific. In addition to being a potentially weaker source of randomness,
this also poses a challenge to compile BaseCryptLibOnProtocol with a platform-
agnostic version of TimerLib that works universally.

The solution here is to allow platform to specify their source of entropy in
addition to providing two new RngLibs: one that uses the TimerLib as well as
one that uses RngProtocol to provide randomness. Then the decision to use
RDRAND or other entropy sources is up to the platform. Mixing various entropy
sources is the onus of the platform. It has been suggested on Devel#40590 and
BZ#1871 that there should be mixing of the PerformanceCounter and RDRAND using
something similar to the yarrow alogirthm that FreeBSD uses for example. This
patch series doesn't offer an RngLib that offers that sort of mixing as the
ultimate source of random is defined by the platform.

This patch series offers three benefits:
1. Dependency reduction: Removes the need for a platform specific timer
library.  We publish a single binary used on numerous platforms for
crypto and the introduced timer lib dependency caused issues because we
could not fulfill our platform needs with one library instance.

2. Code maintenance: Removing this additional code and leveraging an existing
library within Edk2 means less code to maintain.

3. Platform defined quality: A platform can choose which instance to use and
the implications of that instance.

This patch series seeks to address five seperate issues.
  1) Use RngLib interface to generate random entropy in rand_pool
  2) Remove dependency on TimerLib in OpensslLib
  3) Add a new version of RngLib implemented by TimerLib
  4) Add a new version of RngLib implemented by EFI_RNG_PROTOCOL
  5) Add RngLib to platforms in EDK2 such as ArmVirtPkg and OvmfPkg

Since this changes the dependencies of OpenSSL, this has the potential of being
a breaking change for platforms in edk2-platforms. The easiest solution is just
to use the RngLib that uses the TimerLib as this closely mimics the behavior of
OpenSSL prior to this patch series. There is also a null version of RngLib for
CI environments that need this change
(https://edk2.groups.io/g/devel/message/50432). Though it should be pointed out
that in CI environments, the null version of BaseCryptLib or OpenSSL should be
used.

In addition, it has been suggested that
1) Add AsmRdSeed to BaseLib.
2) Update BaseRngLib to use AsmRdSeed() for the random number,
if RdSeed is supported (CPUID BIT18)

However, this is largely out of scope for this particular patch series and
will likely need to be in a follow-up series later.

It is my understanding that the OpenSSL code uses the values provided as a
randomness pool rather than a seed or random numbers itself, so the
requirements for randomness are not quite as stringent as other applications.

For the ArmVirtPkg and OvmfPkg platforms, the patch series here just adds in
the TimerLib based RngLib as that is similar to the functionality of before.
It is added as a common library so any custom RngLib defined in the DSC
should take precedence over the TimerLibRngLib.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Patch Series History:
v10 - addressed comments from Liming removing magic numbers and adding DebugLib 
to TimerRngLib 
v8 - addressed comments from Ard and Mike around code style for DxeRngLib and 
BaseRngLibTimerLib
v7 - addressed comments from Lazlo and Ard for further fixes around OvmfPkg
v6 - addressed comments from Lazlo and Ard for fixes around OvmfPkg
v5 - moved additions for OvmfPkg and ArmVirtPkg to correct positions
v4 - added more information to various commit messages
v3 - addressed comments from Mike K around fixes to BaseRngLibTimer delays
v2 - renamed some libraries to fit with naming conventions

Cc: Ard Biesheuvel 
Cc: Anthony Perard 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Julien Grall 
Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Liming Gao 
Cc: Leif Lindholm 
Cc: Michael D Kinney 
Cc: Xiaoyu Lu 
Cc: Zhiguang Liu 
Cc: Sean Brogan 

Signed-off-by: Matthew Carlson 

Matthew Carlson (5):
  MdePkg: TimerRngLib: Added RngLib that uses TimerLib
  

[edk2-devel] [PATCH v10 5/5] CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool

[Matthew Carlson]

From: Matthew Carlson 

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Changes OpenSSL to no longer depend on TimerLib and instead use RngLib.
This allows platforms to decide for themsevles what sort of entropy source
they provide to OpenSSL and TlsLib.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Xiaoyu Lu 

Acked-by: Ard Biesheuvel 
Reviewed-by: Jiewen Yao 
Signed-off-by: Matthew Carlson 
---
 CryptoPkg/Library/OpensslLib/rand_pool.c   | 269 +---
 CryptoPkg/Library/OpensslLib/rand_pool_noise.c |  29 ---
 CryptoPkg/Library/OpensslLib/rand_pool_noise_tsc.c |  43 
 CryptoPkg/CryptoPkg.ci.yaml|   4 +-
 CryptoPkg/CryptoPkg.dsc|   1 +
 CryptoPkg/Library/OpensslLib/OpensslLib.inf|  15 +-
 CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  |  15 +-
 CryptoPkg/Library/OpensslLib/rand_pool_noise.h |  29 ---
 8 files changed, 68 insertions(+), 337 deletions(-)

diff --git a/CryptoPkg/Library/OpensslLib/rand_pool.c 
b/CryptoPkg/Library/OpensslLib/rand_pool.c
index 9e0179b03490..6218ae0c1cd7 100644
--- a/CryptoPkg/Library/OpensslLib/rand_pool.c
+++ b/CryptoPkg/Library/OpensslLib/rand_pool.c
@@ -2,8 +2,8 @@
   OpenSSL_1_1_1b doesn't implement rand_pool_* functions for UEFI.
   The file implement these functions.
 
-Copyright (c) 2019, Intel Corporation. All rights reserved.
-SPDX-License-Identifier: BSD-2-Clause-Patent
+  Copyright (c) 2019, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
@@ -11,53 +11,18 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 
 #include 
-#include 
-
-#include "rand_pool_noise.h"
-
-/**
-  Get some randomness from low-order bits of GetPerformanceCounter results.
-  And combine them to the 64-bit value
-
-  @param[out] RandBuffer pointer to store the 64-bit random value.
-
-  @retval TRUERandom number generated successfully.
-  @retval FALSE   Failed to generate.
-**/
-STATIC
-BOOLEAN
-EFIAPI
-GetRandNoise64FromPerformanceCounter(
-  OUT UINT64  *Rand
-  )
-{
-  UINT32 Index;
-  UINT32 *RandPtr;
-
-  if (NULL == Rand) {
-return FALSE;
-  }
-
-  RandPtr = (UINT32 *) Rand;
-
-  for (Index = 0; Index < 2; Index ++) {
-*RandPtr = (UINT32) (GetPerformanceCounter () & 0xFF);
-MicroSecondDelay (10);
-RandPtr++;
-  }
-
-  return TRUE;
-}
+#include 
 
 /**
   Calls RandomNumber64 to fill
   a buffer of arbitrary size with random bytes.
+  This is a shim layer to RngLib.
 
   @param[in]   LengthSize of the buffer, in bytes,  to fill with.
   @param[out]  RandBufferPointer to the buffer to store the random result.
 
-  @retval EFI_SUCCESSRandom bytes generation succeeded.
-  @retval EFI_NOT_READY  Failed to request random bytes.
+  @retval TRUERandom bytes generation succeeded.
+  @retval FALSE   Failed to request random bytes.
 
 **/
 STATIC
@@ -65,7 +30,7 @@ BOOLEAN
 EFIAPI
 RandGetBytes (
   IN UINTN Length,
-  OUT UINT8*RandBuffer
+  OUT UINT8   *RandBuffer
   )
 {
   BOOLEAN Ret;
@@ -73,17 +38,17 @@ RandGetBytes (
 
   Ret = FALSE;
 
+  if (RandBuffer == NULL) {
+DEBUG((DEBUG_ERROR, "[OPENSSL_RAND_POOL] NULL RandBuffer. No random 
numbers are generated and your system is not secure\n"));
+ASSERT (RandBuffer != NULL); // Since we can't generate random numbers, we 
should assert. Otherwise we will just blow up later.
+return Ret;
+  }
+
+
   while (Length > 0) {
-//
-// Get random noise from platform.
-// If it failed, fallback to PerformanceCounter
-// If you really care about security, you must override
-// GetRandomNoise64FromPlatform.
-//
-Ret = GetRandomNoise64 ();
-if (Ret == FALSE) {
-  Ret = GetRandNoise64FromPerformanceCounter ();
-}
+// Use RngLib to get random number
+Ret = GetRandomNumber64 ();
+
 if (!Ret) {
   return Ret;
 }
@@ -91,7 +56,8 @@ RandGetBytes (
   *((UINT64*) RandBuffer) = TempRand;
   RandBuffer += sizeof (UINT64);
   Length -= sizeof (TempRand);
-} else {
+}
+else {
   CopyMem (RandBuffer, , Length);
   Length = 0;
 }
@@ -100,125 +66,6 @@ RandGetBytes (
   return Ret;
 }
 
-/**
-  Creates a 128bit random value that is fully forward and backward prediction 
resistant,
-  suitable for seeding a NIST SP800-90 Compliant.
-  This function takes multiple random numbers from PerformanceCounter to 
ensure reseeding
-  and performs AES-CBC-MAC over the data to compute the seed value.
-
-  @param[out]  SeedBufferPointer to a 128bit buffer to store the random 
seed.
-
-  @retval TRUERandom seed generation succeeded.
-  @retval FALSE  Failed to request random bytes.
-
-**/
-STATIC
-BOOLEAN
-EFIAPI
-RandGetSeed128 (
-  OUT UINT8*SeedBuffer
-  )
-{
-  BOOLEAN Ret;
-  UINT8   RandByte[16];
-  UINT8   Key[16];
-  UINT8   

[edk2-devel] [PATCH v10 1/5] MdePkg: TimerRngLib: Added RngLib that uses TimerLib

[Matthew Carlson]

From: Matthew Carlson 

Added a new RngLib that provides random numbers from the TimerLib
using the performance counter. This is meant to be used for OpenSSL
to replicate past behavior. This should not be used in production as
a real source of entropy.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Cc: Ard Biesheuvel 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Signed-off-by: Matthew Carlson 
---
 MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c  | 189 

 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf |  36 
 MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.uni |  15 ++
 MdePkg/MdePkg.dsc|   3 +-
 4 files changed, 242 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c 
b/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
new file mode 100644
index ..54d29d96f3d3
--- /dev/null
+++ b/MdePkg/Library/BaseRngLibTimerLib/RngLibTimer.c
@@ -0,0 +1,189 @@
+/** @file
+  BaseRng Library that uses the TimerLib to provide reasonably random numbers.
+  Do not use this on a production system.
+
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include 
+#include 
+#include 
+#include 
+
+#define DEFAULT_DELAY_TIME_IN_MICROSECONDS 10
+
+/**
+ Using the TimerLib GetPerformanceCounterProperties() we delay
+ for enough time for the PerformanceCounter to increment.
+
+ If the return value from GetPerformanceCounterProperties (TimerLib)
+ is zero, this function will return 10 and attempt to assert.
+ **/
+STATIC
+UINT32
+CalculateMinimumDecentDelayInMicroseconds (
+  VOID
+  )
+{
+  UINT64 CounterHz;
+
+  // Get the counter properties
+  CounterHz = GetPerformanceCounterProperties (NULL, NULL);
+  // Make sure we won't divide by zero
+  if (CounterHz == 0) {
+ASSERT(CounterHz != 0); // Assert so the developer knows something is wrong
+return DEFAULT_DELAY_TIME_IN_MICROSECONDS;
+  }
+  // Calculate the minimum delay based on 1.5 microseconds divided by the 
hertz.
+  // We calculate the length of a cycle (1/CounterHz) and multiply it by 1.5 
microseconds
+  // This ensures that the performance counter has increased by at least one
+  return (UINT32)(MAX (DivU64x64Remainder (150,CounterHz, NULL), 1));
+}
+
+
+/**
+  Generates a 16-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand Buffer pointer to store the 16-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber16 (
+  OUT UINT16*Rand
+  )
+{
+  UINT32  Index;
+  UINT8  *RandPtr;
+  UINT32  DelayInMicroSeconds;
+
+  ASSERT (Rand != NULL);
+
+  if (Rand == NULL) {
+return FALSE;
+  }
+  DelayInMicroSeconds = CalculateMinimumDecentDelayInMicroseconds ();
+  RandPtr = (UINT8*)Rand;
+  // Get 2 bytes of random ish data
+  for (Index = 0; Index < sizeof(UINT16); Index ++) {
+*RandPtr = (UINT8)(GetPerformanceCounter () & 0xFF);
+// Delay to give the performance counter a chance to change
+MicroSecondDelay (DelayInMicroSeconds);
+RandPtr++;
+  }
+  return TRUE;
+}
+
+/**
+  Generates a 32-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand Buffer pointer to store the 32-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber32 (
+  OUT UINT32*Rand
+  )
+{
+  UINT32  Index;
+  UINT8  *RandPtr;
+  UINT32  DelayInMicroSeconds;
+
+  ASSERT (Rand != NULL);
+
+  if (NULL == Rand) {
+return FALSE;
+  }
+
+  RandPtr = (UINT8 *) Rand;
+  DelayInMicroSeconds = CalculateMinimumDecentDelayInMicroseconds ();
+  // Get 4 bytes of random ish data
+  for (Index = 0; Index < sizeof(UINT32); Index ++) {
+*RandPtr = (UINT8)(GetPerformanceCounter () & 0xFF);
+// Delay to give the performance counter a chance to change
+MicroSecondDelay (DelayInMicroSeconds);
+RandPtr++;
+  }
+  return TRUE;
+}
+
+/**
+  Generates a 64-bit random number.
+
+  if Rand is NULL, then ASSERT().
+
+  @param[out] Rand Buffer pointer to store the 64-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber64 (
+  OUT UINT64*Rand
+  )
+{
+  UINT32  Index;
+  UINT8  *RandPtr;
+  UINT32  DelayInMicroSeconds;
+
+  ASSERT (Rand != NULL);
+
+  if (NULL == Rand) {
+return FALSE;
+  }
+
+  RandPtr = (UINT8 *)Rand;
+  DelayInMicroSeconds = CalculateMinimumDecentDelayInMicroseconds ();
+  // Get 8 bytes of random ish data
+  for (Index = 0; Index < sizeof(UINT64); Index ++) {
+*RandPtr = 

[edk2-devel] [PATCH v10 2/5] MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe

[Matthew Carlson]

From: Matthew Carlson 

This adds a RngLib that uses the RngProtocol to provide randomness.
This means that the RngLib is meant to be used with DXE_DRIVERS.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Cc: Ard Biesheuvel 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Zhiguang Liu 
Signed-off-by: Matthew Carlson 
---
 MdePkg/Library/DxeRngLib/DxeRngLib.c   | 199 
 MdePkg/Library/DxeRngLib/DxeRngLib.inf |  38 
 MdePkg/Library/DxeRngLib/DxeRngLib.uni |  15 ++
 MdePkg/MdePkg.dsc  |   4 +-
 4 files changed, 255 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/DxeRngLib/DxeRngLib.c 
b/MdePkg/Library/DxeRngLib/DxeRngLib.c
new file mode 100644
index ..9c3d67b5a62d
--- /dev/null
+++ b/MdePkg/Library/DxeRngLib/DxeRngLib.c
@@ -0,0 +1,199 @@
+/** @file
+ Provides an implementation of the library class RngLib that uses the Rng 
protocol.
+
+ Copyright (c) Microsoft Corporation. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+  Routine Description:
+
+  Generates a random number via the NIST
+  800-9A algorithm.  Refer to
+  http://csrc.nist.gov/groups/STM/cavp/documents/drbg/DRBGVS.pdf
+  for more information.
+
+  @param[out] Buffer  Buffer to receive the random number.
+  @param[in]  BufferSize  Number of bytes in Buffer.
+
+  @retval EFI_SUCCESS or underlying failure code.
+**/
+STATIC
+EFI_STATUS
+GenerateRandomNumberViaNist800Algorithm (
+  OUT UINT8  *Buffer,
+  IN  UINTN  BufferSize
+  )
+{
+  EFI_STATUSStatus;
+  EFI_RNG_PROTOCOL  *RngProtocol;
+
+  RngProtocol = NULL;
+
+  if (Buffer == NULL) {
+  DEBUG((DEBUG_ERROR, "%a: Buffer == NULL.\n", __FUNCTION__));
+  return EFI_INVALID_PARAMETER;
+  }
+
+  Status = gBS->LocateProtocol (, NULL, (VOID 
**));
+  if (EFI_ERROR (Status) || RngProtocol == NULL) {
+  DEBUG((DEBUG_ERROR, "%a: Could not locate RNG prototocol, Status = 
%r\n", __FUNCTION__, Status));
+  return Status;
+  }
+
+  Status = RngProtocol->GetRNG (RngProtocol, 
, BufferSize, Buffer);
+  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm CTR-256 - Status = %r\n", 
__FUNCTION__, Status));
+  if (!EFI_ERROR (Status)) {
+return Status;
+  }
+
+  Status = RngProtocol->GetRNG (RngProtocol, 
, BufferSize, Buffer);
+  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm HMAC-256 - Status = %r\n", 
__FUNCTION__, Status));
+  if (!EFI_ERROR (Status)) {
+return Status;
+  }
+
+  Status = RngProtocol->GetRNG (RngProtocol, 
, BufferSize, Buffer);
+  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm Hash-256 - Status = %r\n", 
__FUNCTION__, Status));
+  if (!EFI_ERROR (Status)) {
+return Status;
+  }
+  // If all the other methods have failed, use the default method from the 
RngProtocol
+  Status = RngProtocol->GetRNG (RngProtocol, NULL, BufferSize, Buffer);
+  DEBUG((DEBUG_INFO, "%a: GetRNG algorithm Hash-256 - Status = %r\n", 
__FUNCTION__, Status));
+  if (!EFI_ERROR (Status)) {
+return Status;
+  }
+  // If we get to this point, we have failed
+  DEBUG((DEBUG_ERROR, "%a: GetRNG() failed, staus = %r\n", __FUNCTION__, 
Status));
+
+  return Status;
+}// GenerateRandomNumberViaNist800Algorithm()
+
+
+/**
+  Generates a 16-bit random number.
+
+  if Rand is NULL, return FALSE.
+
+  @param[out] Rand Buffer pointer to store the 16-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber16 (
+  OUT UINT16  *Rand
+  )
+{
+  EFI_STATUS Status;
+
+  if (Rand == NULL)
+  {
+return FALSE;
+  }
+
+  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8 *)Rand, 
sizeof(UINT16));
+  if (EFI_ERROR (Status)) {
+return FALSE;
+  }
+  return TRUE;
+}
+
+/**
+  Generates a 32-bit random number.
+
+  if Rand is NULL, return FALSE.
+
+  @param[out] Rand Buffer pointer to store the 32-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber32 (
+  OUT UINT32 *Rand
+  )
+{
+  EFI_STATUS Status;
+
+  if (Rand == NULL) {
+return FALSE;
+  }
+
+  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8*)Rand, 
sizeof(UINT32));
+  if (EFI_ERROR (Status)) {
+return FALSE;
+  }
+  return TRUE;
+}
+
+/**
+  Generates a 64-bit random number.
+
+  if Rand is NULL, return FALSE.
+
+  @param[out] Rand Buffer pointer to store the 64-bit random value.
+
+  @retval TRUE Random number generated successfully.
+  @retval FALSEFailed to generate the random number.
+
+**/
+BOOLEAN
+EFIAPI
+GetRandomNumber64 (
+  OUT UINT64 *Rand
+  )
+{
+  EFI_STATUS Status;
+
+  if (Rand == NULL) {
+return FALSE;
+  }
+
+  Status = GenerateRandomNumberViaNist800Algorithm ((UINT8*)Rand, 
sizeof(UINT64));
+  if (EFI_ERROR (Status)) {
+return 

[edk2-devel] [PATCH v10 0/5] Use RngLib instead of TimerLib for OpensslLib

[Matthew Carlson]

From: Matthew Carlson 

Hello all,

This patch contains a fix for Bugzilla 1871.
There's been a good bit of community discussion around the topic,
so below follows a general overview of the discussion and what this patch does.

This is the seventh iteration of this patch series, focused on code style and a
few functions being renamed to comply with style.

Back in Devel message#40590 (https://edk2.groups.io/g/devel/message/40590)
around the patch series that updates OpenSSL to 1.1.1b, a comment was made
that suggested that platforms be in charge of the entropy/randomness that
is provided to OpenSSL as currently the entropry source seems to be a
hand-rolled random number generator that uses the PerformanceCounter from
TimerLib. This causes OpenSSL to depend on TimerLib, which is often platform
specific. In addition to being a potentially weaker source of randomness,
this also poses a challenge to compile BaseCryptLibOnProtocol with a platform-
agnostic version of TimerLib that works universally.

The solution here is to allow platform to specify their source of entropy in
addition to providing two new RngLibs: one that uses the TimerLib as well as
one that uses RngProtocol to provide randomness. Then the decision to use
RDRAND or other entropy sources is up to the platform. Mixing various entropy
sources is the onus of the platform. It has been suggested on Devel#40590 and
BZ#1871 that there should be mixing of the PerformanceCounter and RDRAND using
something similar to the yarrow alogirthm that FreeBSD uses for example. This
patch series doesn't offer an RngLib that offers that sort of mixing as the
ultimate source of random is defined by the platform.

This patch series offers three benefits:
1. Dependency reduction: Removes the need for a platform specific timer
library.  We publish a single binary used on numerous platforms for
crypto and the introduced timer lib dependency caused issues because we
could not fulfill our platform needs with one library instance.

2. Code maintenance: Removing this additional code and leveraging an existing
library within Edk2 means less code to maintain.

3. Platform defined quality: A platform can choose which instance to use and
the implications of that instance.

This patch series seeks to address five seperate issues.
  1) Use RngLib interface to generate random entropy in rand_pool
  2) Remove dependency on TimerLib in OpensslLib
  3) Add a new version of RngLib implemented by TimerLib
  4) Add a new version of RngLib implemented by EFI_RNG_PROTOCOL
  5) Add RngLib to platforms in EDK2 such as ArmVirtPkg and OvmfPkg

Since this changes the dependencies of OpenSSL, this has the potential of being
a breaking change for platforms in edk2-platforms. The easiest solution is just
to use the RngLib that uses the TimerLib as this closely mimics the behavior of
OpenSSL prior to this patch series. There is also a null version of RngLib for
CI environments that need this change
(https://edk2.groups.io/g/devel/message/50432). Though it should be pointed out
that in CI environments, the null version of BaseCryptLib or OpenSSL should be
used.

In addition, it has been suggested that
1) Add AsmRdSeed to BaseLib.
2) Update BaseRngLib to use AsmRdSeed() for the random number,
if RdSeed is supported (CPUID BIT18)

However, this is largely out of scope for this particular patch series and
will likely need to be in a follow-up series later.

It is my understanding that the OpenSSL code uses the values provided as a
randomness pool rather than a seed or random numbers itself, so the
requirements for randomness are not quite as stringent as other applications.

For the ArmVirtPkg and OvmfPkg platforms, the patch series here just adds in
the TimerLib based RngLib as that is similar to the functionality of before.
It is added as a common library so any custom RngLib defined in the DSC
should take precedence over the TimerLibRngLib.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Patch Series History:
v10 - addressed comments from Liming removing magic numbers and adding DebugLib 
to TimerRngLib 
v8 - addressed comments from Ard and Mike around code style for DxeRngLib and 
BaseRngLibTimerLib
v7 - addressed comments from Lazlo and Ard for further fixes around OvmfPkg
v6 - addressed comments from Lazlo and Ard for fixes around OvmfPkg
v5 - moved additions for OvmfPkg and ArmVirtPkg to correct positions
v4 - added more information to various commit messages
v3 - addressed comments from Mike K around fixes to BaseRngLibTimer delays
v2 - renamed some libraries to fit with naming conventions


Matthew Carlson (5):
  MdePkg: TimerRngLib: Added RngLib that uses TimerLib
  MdePkg: BaseRngLibDxe: Add RngLib that uses RngDxe
  OvmfPkg: Add RngLib based on TimerLib for Crypto
  ArmVirtPkg: Add RngLib based on TimerLib for CryptoPkg
  CryptoPkg: OpensslLib: Use RngLib to generate entropy in rand_pool

 CryptoPkg/Library/OpensslLib/rand_pool.c 

[edk2-devel] [PATCH v10 4/5] ArmVirtPkg: Add RngLib based on TimerLib for CryptoPkg

[Matthew Carlson]

From: Matthew Carlson 

Updates the DSC for the ArmVirtPkg platform to add a RngLib that uses the
TimerLib. This is due to a later change that adds TimerLib as a dependency
for OpenSSL. The TimerLib based RngLib mimics the behavior of OpenSSL
previously and it is recommended to switch to a better source of
entropy than the system's performance counter.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 

Reviewed-by: Laszlo Ersek 
Signed-off-by: Matthew Carlson 
---
 ArmVirtPkg/ArmVirt.dsc.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index cf44fc73890b..cb3845d2bd37 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -160,6 +160,7 @@
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
   #
   # Secure Boot dependencies
-- 
2.28.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64914): https://edk2.groups.io/g/devel/message/64914
Mute This Topic: https://groups.io/mt/76563986/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v10 3/5] OvmfPkg: Add RngLib based on TimerLib for Crypto

[Matthew Carlson]

From: Matthew Carlson 

Updates the DSC's for Ovmf based platforms to add a RngLib that uses the
TimerLib. This is due to a later change that adds TimerLib as a dependency
for OpenSSL. The TimerLib based RngLib mimics the behavior of OpenSSL
previously and it is recommended to switch to a better source of
entropy than the system's performance counter.

Ref: https://github.com/tianocore/edk2/pull/845
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1871

Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Anthony Perard 
Cc: Julien Grall 

Reviewed-by: Laszlo Ersek 
Signed-off-by: Matthew Carlson 
---
 OvmfPkg/Bhyve/BhyveX64.dsc | 1 +
 OvmfPkg/OvmfPkgIa32.dsc| 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
 OvmfPkg/OvmfPkgX64.dsc | 1 +
 OvmfPkg/OvmfXen.dsc| 1 +
 5 files changed, 5 insertions(+)

diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index d2e9edfaa6b8..16d2233d7788 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -185,6 +185,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   
PlatformSecureLib|OvmfPkg/Bhyve/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 133a9a93c071..fa18adeb5c5a 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -189,6 +189,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 338c38db29b5..7456a154168d 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -193,6 +193,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index b80710fbdca4..5bda143fd14d 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -193,6 +193,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 37b63a874067..e562abd7175d 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -179,6 +179,7 @@
 !else
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
 !endif
+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
 
   
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
   VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
-- 
2.28.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64913): https://edk2.groups.io/g/devel/message/64913
Mute This Topic: https://groups.io/mt/76563985/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] SecurityPkg: Initailize variable Status before it is consumed.

[Laszlo Ersek]

On 09/01/20 09:02, Laszlo Ersek wrote:
> On 09/01/20 02:55, Zhiguang Liu wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2945
>>
>> V2: Move "Status = EFI_SUCCESS;" before the EDKII_TCG_PRE_HASH check.
>>
>> Cc: Jiewen Yao 
>> Cc: Jian J Wang 
>> Cc: Qi Zhang 
>> Cc: Rahul Kumar 
>> Cc: Laszlo Ersek 
>> Reviewed-by: Jiewen Yao 
>> Signed-off-by: Zhiguang Liu 
>> ---
>>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c 
>> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
>> index 0e770f4485..93a8803ff6 100644
>> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
>> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
>> @@ -456,6 +456,7 @@ HashLogExtendEvent (
>>if ((Flags & EDKII_TCG_PRE_HASH) != 0 || (Flags & 
>> EDKII_TCG_PRE_HASH_LOG_ONLY) != 0) {
>>  ZeroMem (, sizeof(DigestList));
>>  CopyMem (, HashData, sizeof(DigestList));
>> +Status = EFI_SUCCESS;
>>  if ((Flags & EDKII_TCG_PRE_HASH) !=0 ) {
>>Status = Tpm2PcrExtend (
>> NewEventHdr->PCRIndex,
>>
> 
> Reviewed-by: Laszlo Ersek 
> 
> I'll let Jiewen or Jian merge this.
> 
> Please change the status of TianoCore#2945 to IN_PROGRESS, and link both
> versions of this patch into the ticket as well (in a new comment).

Merged as commit 46db105b7b77, via
.

BZ updated.

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64909): https://edk2.groups.io/g/devel/message/64909
Mute This Topic: https://groups.io/mt/76530112/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 3/3] SecurityPkg/DxeImageVerificationLib: catch alignment overflow (CVE-2019-14562)

[Laszlo Ersek]

On 09/01/20 17:53, Philippe Mathieu-Daudé wrote:
> Hi Laszlo,
>
> On 9/1/20 11:12 AM, Laszlo Ersek wrote:
>> The DxeImageVerificationHandler() function currently checks whether
>> "SecDataDir" has enough room for "WinCertificate->dwLength". However, for
>> advancing "OffSet", "WinCertificate->dwLength" is aligned to the next
>> multiple of 8. If "WinCertificate->dwLength" is large enough, the
>> alignment will return 0, and "OffSet" will be stuck at the same value.
>>
>> Check whether "SecDataDir" has room left for both
>> "WinCertificate->dwLength" and the alignment.
>>
>> Cc: Jian J Wang 
>> Cc: Jiewen Yao 
>> Cc: Min Xu 
>> Cc: Wenyi Xie 
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215
>> Signed-off-by: Laszlo Ersek 
>> ---
>>  SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 4 
>> +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git 
>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>> index 100739eb3eb6..11154b6cc58a 100644
>> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>> @@ -1860,7 +1860,9 @@ DxeImageVerificationHandler (
>>break;
>>  }
>>  WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
>> -if (SecDataDirLeft < WinCertificate->dwLength) {
>> +if (SecDataDirLeft < WinCertificate->dwLength ||
>> +(SecDataDirLeft - WinCertificate->dwLength <
>> + ALIGN_SIZE (WinCertificate->dwLength))) {
>
> I dare to ask (probably again, I remember some similar boundary check
> style question once), why not as (which is simpler for me to review):
>
>if (SecDataDirLeft < WinCertificate->dwLength + ALIGN_SIZE 
> (WinCertificate->dwLength)) {

Indeed, the mathematical relation that we want to catch (and reject) is:

  SecDataDirLeft < WinCertificate->dwLength + ALIGN_SIZE 
(WinCertificate->dwLength)

But the problem is precisely with the addition on the right hand side.

(1) The "WinCertificate->dwLength" field is of type UINT32 (meaning, in
standard C terms, "unsigned int").

(2) The ALIGN_SIZE() macro is defined as follows, in
"SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.h":

  #define ALIGNMENT_SIZE8
  #define ALIGN_SIZE(a) (((a) % ALIGNMENT_SIZE) ? ALIGNMENT_SIZE - ((a) % 
ALIGNMENT_SIZE) : 0)

If you substitute "WinCertificate->dwLength" for "a", and 8 for
ALIGNMENT_SIZE, you get the following replacement text for the macro
invocation:

  (((WinCertificate->dwLength) % 8) ? 8 - ((WinCertificate->dwLength) % 8) 
: 0)

Here the third operand of the conditional operator (that is, 0) is
of type "int".

The 2nd operand of the conditional operator ultimately has type
"unsigned int". That's because the type of
"WinCertificate->dwLength" conceptually "propagates" outwards across
the "%" and "-" operators, via the "usual arithmetic conversions".
Basically you have

  (unsigned int) % (int) -->
  converts the int to unsigned int, produces unsigned int

  (int) - (unsigned int) -->
  converts the int to unsigned int, produces unsigned int

And then we have the following, for the conditional operator too
(C99 6.5.15p5):

  If both the second and third operands have arithmetic type, the
  result type that would be determined by the usual arithmetic
  conversions, were they applied to those two operands, is the type
  of the result.

Which means that ALIGN_SIZE (WinCertificate->dwLength) produces a
UINT32 (unsigned int) as well.

(3) Furthermore, "SecDataDirLeft" is of type UINT32 (unsigned int) too.

(4) Ultimately, in the 2nd subcondition of the "if", we would have

  UINT32 < UINT32 + UINT32

If the addition overflows on the right hand size (which is well
defined behavior for "unsigned int"s), possibly to a really small
number, then the comparison may evaluate to FALSE, even though the
*mathematical* result would be TRUE.

Specifically, if

  WinCertificate->dwLength >= MAX_UINT32 - 6

then the sum

  WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength)

wraps around to zero precisely, and then the 2nd subcondition would
decay to

  SecDataDirLeft < 0

which would always evaluate to FALSE.


We can avoid this by taking the mathematically relevant expression

  SecDataDirLeft < WinCertificate->dwLength + ALIGN_SIZE 
(WinCertificate->dwLength)

and by subtracting "WinCertificate->dwLength" from both sides. That
yields the following (mathematically equivalent) inequality:

  SecDataDirLeft - WinCertificate->dwLength < ALIGN_SIZE 
(WinCertificate->dwLength)

which we can safely evaluate in C.

The reason for that "safety" is the *directly preceding* subcondition in
the "if" statement:

  if 

Re: [edk2-devel] [PATCH 3/3] SecurityPkg/DxeImageVerificationLib: catch alignment overflow (CVE-2019-14562)

[Philippe Mathieu-Daudé]

Hi Laszlo,

On 9/1/20 11:12 AM, Laszlo Ersek wrote:
> The DxeImageVerificationHandler() function currently checks whether
> "SecDataDir" has enough room for "WinCertificate->dwLength". However, for
> advancing "OffSet", "WinCertificate->dwLength" is aligned to the next
> multiple of 8. If "WinCertificate->dwLength" is large enough, the
> alignment will return 0, and "OffSet" will be stuck at the same value.
> 
> Check whether "SecDataDir" has room left for both
> "WinCertificate->dwLength" and the alignment.
> 
> Cc: Jian J Wang 
> Cc: Jiewen Yao 
> Cc: Min Xu 
> Cc: Wenyi Xie 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215
> Signed-off-by: Laszlo Ersek 
> ---
>  SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 4 
> +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git 
> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index 100739eb3eb6..11154b6cc58a 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> @@ -1860,7 +1860,9 @@ DxeImageVerificationHandler (
>break;
>  }
>  WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
> -if (SecDataDirLeft < WinCertificate->dwLength) {
> +if (SecDataDirLeft < WinCertificate->dwLength ||
> +(SecDataDirLeft - WinCertificate->dwLength <
> + ALIGN_SIZE (WinCertificate->dwLength))) {

I dare to ask (probably again, I remember some similar boundary check
style question once), why not as (which is simpler for me to review):

   if (SecDataDirLeft < WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate->dwLength)) {

At any rate, for this patch:
Reviewed-by: Philippe Mathieu-Daude 

>break;
>  }
>  
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64907): https://edk2.groups.io/g/devel/message/64907
Mute This Topic: https://groups.io/mt/76552541/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec: add FspMeasurementLib.h

[Laszlo Ersek]

On 09/01/20 02:50, Qi Zhang wrote:
> Thanks Chasel!
> 
> Hi, Liming
> 
> I request this change to catch the stable release. Thanks!
> 
> BRs
> Qi Zhang
> 
>> -Original Message-
>> From: Chiu, Chasel 
>> Sent: Monday, August 31, 2020 10:54 AM
>> To: Zhang, Qi1 ; devel@edk2.groups.io
>> Cc: Desimone, Nathaniel L ; Liming Gao
>> ; Dong, Eric 
>> Subject: RE: [PATCH] IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec: add
>> FspMeasurementLib.h
>>
>>
>> Reviewed-by: Chasel Chiu 

Merged as commit 0c5c45a1337f, via
.

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64906): https://edk2.groups.io/g/devel/message/64906
Mute This Topic: https://groups.io/mt/76526695/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/3] SecurityPkg/DxeImageVerificationLib: assign WinCertificate after size check

[Philippe Mathieu-Daudé]

On 9/1/20 11:12 AM, Laszlo Ersek wrote:
> Currently the (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) check only
> guards the de-referencing of the "WinCertificate" pointer. It does not
> guard the calculation of the pointer itself:
> 
>   WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
> 
> This is wrong; if we don't know for sure that we have enough room for a
> WIN_CERTIFICATE, then even creating such a pointer, not just
> de-referencing it, may invoke undefined behavior.

Tricky to catch...

Reviewed-by: Philippe Mathieu-Daude 

> 
> Move the pointer calculation after the size check.
> 
> Cc: Jian J Wang 
> Cc: Jiewen Yao 
> Cc: Min Xu 
> Cc: Wenyi Xie 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215
> Signed-off-by: Laszlo Ersek 
> ---
>  SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 8 
> +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git 
> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index 377feebb205a..100739eb3eb6 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> @@ -1855,10 +1855,12 @@ DxeImageVerificationHandler (
>for (OffSet = SecDataDir->VirtualAddress;
> OffSet < SecDataDirEnd;
> OffSet += (WinCertificate->dwLength + ALIGN_SIZE 
> (WinCertificate->dwLength))) {
> -WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
>  SecDataDirLeft = SecDataDirEnd - OffSet;
> -if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
> -SecDataDirLeft < WinCertificate->dwLength) {
> +if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) {
> +  break;
> +}
> +WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
> +if (SecDataDirLeft < WinCertificate->dwLength) {
>break;
>  }
>  
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64905): https://edk2.groups.io/g/devel/message/64905
Mute This Topic: https://groups.io/mt/76552539/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/3] SecurityPkg/DxeImageVerificationLib: extract SecDataDirEnd, SecDataDirLeft

[Philippe Mathieu-Daudé]

On 9/1/20 11:12 AM, Laszlo Ersek wrote:
> The following two quantities:
> 
>   SecDataDir->VirtualAddress + SecDataDir->Size
>   SecDataDir->VirtualAddress + SecDataDir->Size - OffSet
> 
> are used multiple times in DxeImageVerificationHandler(). Introduce helper
> variables for them: "SecDataDirEnd" and "SecDataDirLeft", respectively.
> This saves us multiple calculations and significantly simplifies the code.
> 
> Note that all three summands above have type UINT32, therefore the new
> variables are also of type UINT32.
> 
> This patch does not change behavior.
> 
> (Note that the code already handles the case when the
> 
>   SecDataDir->VirtualAddress + SecDataDir->Size
> 
> UINT32 addition overflows -- namely, in that case, the certificate loop is
> never entered, and the corruption check right after the loop fires.)
> 
> Cc: Jian J Wang 
> Cc: Jiewen Yao 
> Cc: Min Xu 
> Cc: Wenyi Xie 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Philippe Mathieu-Daude 

> ---
>  SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 12 
> 
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git 
> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index b08fe24e85aa..377feebb205a 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> @@ -1652,6 +1652,8 @@ DxeImageVerificationHandler (
>UINT8*AuthData;
>UINTNAuthDataSize;
>EFI_IMAGE_DATA_DIRECTORY *SecDataDir;
> +  UINT32   SecDataDirEnd;
> +  UINT32   SecDataDirLeft;
>UINT32   OffSet;
>CHAR16   *NameStr;
>RETURN_STATUSPeCoffStatus;
> @@ -1849,12 +1851,14 @@ DxeImageVerificationHandler (
>// "Attribute Certificate Table".
>// The first certificate starts at offset (SecDataDir->VirtualAddress) 
> from the start of the file.
>//
> +  SecDataDirEnd = SecDataDir->VirtualAddress + SecDataDir->Size;
>for (OffSet = SecDataDir->VirtualAddress;
> -   OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
> +   OffSet < SecDataDirEnd;
> OffSet += (WinCertificate->dwLength + ALIGN_SIZE 
> (WinCertificate->dwLength))) {
>  WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
> -if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof 
> (WIN_CERTIFICATE) ||
> -(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < 
> WinCertificate->dwLength) {
> +SecDataDirLeft = SecDataDirEnd - OffSet;
> +if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
> +SecDataDirLeft < WinCertificate->dwLength) {
>break;
>  }
>  
> @@ -1948,7 +1952,7 @@ DxeImageVerificationHandler (
>  }
>}
>  
> -  if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size)) {
> +  if (OffSet != SecDataDirEnd) {
>  //
>  // The Size in Certificate Table or the attribute certificate table is 
> corrupted.
>  //
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64904): https://edk2.groups.io/g/devel/message/64904
Mute This Topic: https://groups.io/mt/76552540/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: 回复: 回复: [edk2-devel] Patch List for 202008 stable tag

[Laszlo Ersek]

On 09/01/20 16:57, gaoliming wrote:
> Hi, all
>   Now, we are in Hard Feature Freeze phase. 202008 stable tag will be
> created on 2020-09-04 Date (00:00:00 UTC-8).
>   If you request any bug fix to catch this stable tag, please send your
> request one day before 2020-09-04.
>   And, also make sure this patch pass code review one day before 2020-09-04.
> Then, we have time to merge it. 
> 
> Laszlo:
>   I add my comments.
> 
> Thanks
> Liming
>> -邮件原件-
>> 发件人: Laszlo Ersek 
>> 发送时间: 2020年9月1日 20:34
>> 收件人: gaoliming ; devel@edk2.groups.io; 'Leif
>> Lindholm' ; af...@apple.com; 'Kinney, Michael D'
>> ; 'Guptha, Soumya K'
>> ; 'Zhiguang Liu' ;
>> 'Zhang, Qi1' 
>> 主题: Re: 回复: [edk2-devel] Patch List for 202008 stable tag
>>
>> On 09/01/20 11:35, gaoliming wrote:
>>> Hi, all
>>> There are still some patches to be merged for 202008 stable tag. They
>>> are all bug fixes. The risk is low. I agree to catch them in this
>>> stable tag. If you have other comments, please reply this mail.
>>>
>>> https://edk2.groups.io/g/devel/message/64873
>>> [PATCH v3] MdeModulePkg/Library: add PEIM and SEC module type to
>> TpmMeasurementLibNull
>>
>> Needs maintainer approval, and then it should be merged, yes.
>>
>>
>>> https://edk2.groups.io/g/devel/message/64865
>>> [PATCH] SecurityPkg: Initailize variable Status before it is consumed.
>>
>> Needs maintainer approval, and then it should be merged, yes.
>>
> https://edk2.groups.io/g/devel/message/64843 Jiewen has given reviewed-by
> for this patch. 

Ah, you are correct. It was even correctly included in v2. Sorry about
missing it!

Thanks!
Laszlo

> 
>>
>>> https://edk2.groups.io/g/devel/message/64812
>>> [PATCH] IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec: add
>> FspMeasurementLib.h
>>
>> Yes, should be merged (it has been Reviewed-by: Chasel Chiu
>> ).
>>
>>
>> More candidates (bugfixes):
>>
>> * [Patch V2] BaseTools/Ecc: Fix an issue of path separator compatibility
>>   https://edk2.groups.io/g/devel/message/64887
>>   http://mid.mail-archive.com/
>> <20200901102315.38840-1-bob.c.f...@intel.com
>>
>> Needs maintainer review, and then it should be merged.
>>
> Agree. I give Reviewed-by. https://edk2.groups.io/g/devel/message/64900
> 
>>
>> * [PATCH EDK2 v1 0/1] EmulatorPkg/host: fix overflow in Mult
>>   https://edk2.groups.io/g/devel/message/64890
>>   http://mid.mail-archive.com/
>> <1598957888-128729-1-git-send-email-xiewen...@huawei.com
>>
>> Needs a review and a maintainer decision whether it is material for the
>> stable tag.
>>
>>
>> * [PATCH 0/3] SecurityPkg/DxeImageVerificationLib: catch alignment
> overflow
>> (CVE-2019-14562)
>>   https://edk2.groups.io/g/devel/message/64882
>>   http://mid.mail-archive.com/
>> <20200901091221.20948-1-ler...@redhat.com
>>
>> Needs maintainer review, plus testing from Wenyi Xie with their
>> reproducer, plus a maintainer decision whether it is material for the
>> stable tag.
>>
>> Thanks
>> Laszlo
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64903): https://edk2.groups.io/g/devel/message/64903
Mute This Topic: https://groups.io/mt/76558068/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

回复: 回复: [edk2-devel] Patch List for 202008 stable tag

[gaoliming]

Hi, all
  Now, we are in Hard Feature Freeze phase. 202008 stable tag will be
created on 2020-09-04 Date (00:00:00 UTC-8).
  If you request any bug fix to catch this stable tag, please send your
request one day before 2020-09-04.
  And, also make sure this patch pass code review one day before 2020-09-04.
Then, we have time to merge it. 

Laszlo:
  I add my comments.

Thanks
Liming
> -邮件原件-
> 发件人: Laszlo Ersek 
> 发送时间: 2020年9月1日 20:34
> 收件人: gaoliming ; devel@edk2.groups.io; 'Leif
> Lindholm' ; af...@apple.com; 'Kinney, Michael D'
> ; 'Guptha, Soumya K'
> ; 'Zhiguang Liu' ;
> 'Zhang, Qi1' 
> 主题: Re: 回复: [edk2-devel] Patch List for 202008 stable tag
> 
> On 09/01/20 11:35, gaoliming wrote:
> > Hi, all
> > There are still some patches to be merged for 202008 stable tag. They
> > are all bug fixes. The risk is low. I agree to catch them in this
> > stable tag. If you have other comments, please reply this mail.
> >
> > https://edk2.groups.io/g/devel/message/64873
> > [PATCH v3] MdeModulePkg/Library: add PEIM and SEC module type to
> TpmMeasurementLibNull
> 
> Needs maintainer approval, and then it should be merged, yes.
> 
> 
> > https://edk2.groups.io/g/devel/message/64865
> > [PATCH] SecurityPkg: Initailize variable Status before it is consumed.
> 
> Needs maintainer approval, and then it should be merged, yes.
> 
https://edk2.groups.io/g/devel/message/64843 Jiewen has given reviewed-by
for this patch. 

> 
> > https://edk2.groups.io/g/devel/message/64812
> > [PATCH] IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec: add
> FspMeasurementLib.h
> 
> Yes, should be merged (it has been Reviewed-by: Chasel Chiu
> ).
> 
> 
> More candidates (bugfixes):
> 
> * [Patch V2] BaseTools/Ecc: Fix an issue of path separator compatibility
>   https://edk2.groups.io/g/devel/message/64887
>   http://mid.mail-archive.com/
> <20200901102315.38840-1-bob.c.f...@intel.com
> 
> Needs maintainer review, and then it should be merged.
> 
Agree. I give Reviewed-by. https://edk2.groups.io/g/devel/message/64900

> 
> * [PATCH EDK2 v1 0/1] EmulatorPkg/host: fix overflow in Mult
>   https://edk2.groups.io/g/devel/message/64890
>   http://mid.mail-archive.com/
> <1598957888-128729-1-git-send-email-xiewen...@huawei.com
> 
> Needs a review and a maintainer decision whether it is material for the
> stable tag.
> 
> 
> * [PATCH 0/3] SecurityPkg/DxeImageVerificationLib: catch alignment
overflow
> (CVE-2019-14562)
>   https://edk2.groups.io/g/devel/message/64882
>   http://mid.mail-archive.com/
> <20200901091221.20948-1-ler...@redhat.com
> 
> Needs maintainer review, plus testing from Wenyi Xie with their
> reproducer, plus a maintainer decision whether it is material for the
> stable tag.
> 
> Thanks
> Laszlo




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64902): https://edk2.groups.io/g/devel/message/64902
Mute This Topic: https://groups.io/mt/76558068/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] TCP Port for ISCSI Connection

[Sivaraman Nainar]

Maciej,

Thanks for the clarification. I agree with you.

-Siva
From: Rabeda, Maciej [mailto:maciej.rab...@linux.intel.com]
Sent: Tuesday, September 1, 2020 8:12 PM
To: Sivaraman Nainar; devel@edk2.groups.io
Subject: Re: [edk2-devel] TCP Port for ISCSI Connection

Siva,

iSCSI targets can be configured to listen on any TCP port.
Changing the UEFI iSCSI Client's TargetPort min/max will effectively prevent 
the user from communicating with the targets configured to listen on ports 
outside that range (in your example: below 1024).
I do not see a reason behind removing that flexibility. If user sets iSCSI 
target and client ports to values shared by other services, it is that user's 
mistake.

Thanks,
Maciej
On 01-Sep-20 16:31, Sivaraman Nainar wrote:
Hello Maciej:

The ports numbers from 0 to 1023 are having specific roles.

Ex: 80 for HTTTP, 443 for HTTPS.

In the case can we set Min as 1024 and Max as 65535.

Thanks
Siva
From: devel@edk2.groups.io 
[mailto:devel@edk2.groups.io] On Behalf Of Maciej Rabeda
Sent: Tuesday, September 1, 2020 7:32 PM
To: devel@edk2.groups.io; Sivaraman Nainar
Subject: Re: [edk2-devel] TCP Port for ISCSI Connection

Hi Siva,

What seems to be the problem at hand? What kind of range of values for 
TargetPort do you propose?
TARGET_PORT_MIN/MAX_NUM refer to a range of values (0-65535) that be set in 
TargetPort field in iSCSI HII. 3260 is a default TCP port for iSCSI.
I see nothing wrong with that.

Thanks,
Maciej
On 31-Aug-20 11:05, Sivaraman Nainar wrote:
Rabeda:

Could you please provide your comment on this.

-Siva
From: Sivaraman Nainar
Sent: Tuesday, July 28, 2020 12:15 PM
To: jiaxin...@intel.com
Cc: devel@edk2.groups.io
Subject: RE: TCP Port for ISCSI Connection

Jiaxin:

Can you please comment on the below query.

-Siva
From: devel@edk2.groups.io 
[mailto:devel@edk2.groups.io] On Behalf Of Sivaraman Nainar
Sent: Friday, July 17, 2020 5:39 PM
To: devel@edk2.groups.io
Cc: jiaxin...@intel.com
Subject: [edk2-devel] Reg: TCP Port for ISCSI Connection

Hello all:

In the ISCSI driver, Target TCP Port Input shows the default port as 3260. 
Which can be set from 0 to 65535

As per below RFC it talks about the Default Port only. Still it not clearly 
said if we can use the numbers 49152-65535 which is reserved.
https://tools.ietf.org/html/rfc3720

13. IANA Considerations

This section conforms to [RFC2434].

The well-known user TCP port number for iSCSI connections assigned by
IANA is 3260 and this is the default iSCSI port. Implementations
needing a system TCP port number may use port 860, the port assigned
by IANA as the iSCSI system port; however in order to use port 860,
it MUST be explicitly specified - implementations MUST NOT default to
use of port 860, as 3260 is the only allowed default.

with my understanding, it wouid be good if we can change the below Min and  MAX 
port number ranges with right ranges.

#define TARGET_PORT_MIN_NUM   0
#define TARGET_PORT_MAX_NUM   65535

Thanks
Siva
This e-mail is intended for the use of the addressee only and may contain 
privileged, confidential, or proprietary information that is exempt from 
disclosure under law. If you have received this message in error, please inform 
us promptly by reply e-mail, then delete the e-mail and destroy any printed 
copy. Thank you.




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64901): https://edk2.groups.io/g/devel/message/64901
Mute This Topic: https://groups.io/mt/76530384/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

回复: [edk2-devel] [Patch V2] BaseTools/Ecc: Fix an issue of path separator compatibility

[gaoliming]

Reviewed-by: Liming Gao 

> -邮件原件-
> 发件人: bounce+27952+64887+4905953+8761...@groups.io
>  代表 Bob Feng
> 发送时间: 2020年9月1日 18:23
> 收件人: devel@edk2.groups.io
> 抄送: Liming Gao ; Yuwei Chen
> ; Shenglei Zhang 
> 主题: [edk2-devel] [Patch V2] BaseTools/Ecc: Fix an issue of path
separator
> compatibility
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2904
> 
> The path separator is different in Windows and Linux, the
> original code does not handle this difference. This patch
> is to fix this issue.
> 
> Signed-off-by: Bob Feng 
> Cc: Liming Gao 
> Cc: Yuwei Chen 
> Cc: Shenglei Zhang 
> ---
> V2
> Change to a better method to get path separator.
> 
>  BaseTools/Source/Python/Ecc/Check.py | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/Ecc/Check.py
> b/BaseTools/Source/Python/Ecc/Check.py
> index 0fdc7e35c1..6087abfa4d 100644
> --- a/BaseTools/Source/Python/Ecc/Check.py
> +++ b/BaseTools/Source/Python/Ecc/Check.py
> @@ -1101,15 +1101,15 @@ class Check(object):
>  for Item in InfPathSet:
>  if Item[0] not in InfPathList:
>  InfPathList.append(Item[0])
>  SqlCommand = """
>   select ID, Path, FullPath from File where
> upper(FullPath) not in
> -(select upper(A.Path) || '\\' ||
> upper(B.Value1) from File as A, INF as B
> +(select upper(A.Path) || '%s' ||
> upper(B.Value1) from File as A, INF as B
>  where A.ID in (select BelongsToFile from
> INF where Model = %s group by BelongsToFile) and
>  B.BelongsToFile = A.ID and B.Model = %s)
>  and (Model = %s or Model = %s)
> -""" % (MODEL_EFI_SOURCE_FILE,
> MODEL_EFI_SOURCE_FILE, MODEL_FILE_C, MODEL_FILE_H)
> +""" % (os.sep, MODEL_EFI_SOURCE_FILE,
> MODEL_EFI_SOURCE_FILE, MODEL_FILE_C, MODEL_FILE_H)
>  RecordSet = EccGlobalData.gDb.TblInf.Exec(SqlCommand)
>  for Record in RecordSet:
>  Path = Record[1]
>  Path = Path.upper().replace('\X64', '').replace('\IA32',
> '').replace('\EBC', '').replace('\IPF', '').replace('\ARM', '')
>  if Path in InfPathList:
> @@ -1130,13 +1130,13 @@ class Check(object):
>  if Pcd[3]:
>  PcdName = Pcd[3]
>  BelongsToFile = Pcd[4]
>  SqlCommand = """
>   select ID from File where FullPath in
> -(select B.Path || '\\' || A.Value1 from INF
> as A, File as B where A.Model = %s and A.BelongsToFile = %s
> +(select B.Path || '%s' || A.Value1 from
> INF as A, File as B where A.Model = %s and A.BelongsToFile = %s
>   and B.ID = %s and (B.Model = %s or
> B.Model = %s))
> - """ % (MODEL_EFI_SOURCE_FILE,
> BelongsToFile, BelongsToFile, MODEL_FILE_C, MODEL_FILE_H)
> + """ % (os.sep,
> MODEL_EFI_SOURCE_FILE, BelongsToFile, BelongsToFile, MODEL_FILE_C,
> MODEL_FILE_H)
>  TableSet =
> EccGlobalData.gDb.TblFile.Exec(SqlCommand)
>  for Tbl in TableSet:
>  TblName = 'Identifier' + str(Tbl[0])
>  SqlCommand = """
>   select Name, ID from %s where
> value like '%s' and Model = %s
> --
> 2.20.1.windows.1
> 
> 
> 




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64900): https://edk2.groups.io/g/devel/message/64900
Mute This Topic: https://groups.io/mt/76557763/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] TCP Port for ISCSI Connection

[Maciej Rabeda]

Siva,

iSCSI targets can be configured to listen on any TCP port.
Changing the UEFI iSCSI Client's TargetPort min/max will effectively 
prevent the user from communicating with the targets configured to 
listen on ports outside that range (in your example: below 1024).
I do not see a reason behind removing that flexibility. If user sets 
iSCSI target and client ports to values shared by other services, it is 
that user's mistake.


Thanks,
Maciej

On 01-Sep-20 16:31, Sivaraman Nainar wrote:


Hello Maciej:

The ports numbers from 0 to 1023 are having specific roles.

Ex: 80 for HTTTP, 443 for HTTPS.

In the case can we set Min as 1024 and Max as 65535.

Thanks

Siva

*From:* devel@edk2.groups.io [mailto:devel@edk2.groups.io] *On Behalf 
Of *Maciej Rabeda

*Sent:* Tuesday, September 1, 2020 7:32 PM
*To:* devel@edk2.groups.io; Sivaraman Nainar
*Subject:* Re: [edk2-devel] TCP Port for ISCSI Connection

Hi Siva,

What seems to be the problem at hand? What kind of range of values for 
TargetPort do you propose?
TARGET_PORT_MIN/MAX_NUM refer to a range of values (0-65535) that be 
set in TargetPort field in iSCSI HII. 3260 is a default TCP port for 
iSCSI.

I see nothing wrong with that.

Thanks,
Maciej

On 31-Aug-20 11:05, Sivaraman Nainar wrote:

Rabeda:

Could you please provide your comment on this.

-Siva

*From:* Sivaraman Nainar
*Sent:* Tuesday, July 28, 2020 12:15 PM
*To:* jiaxin...@intel.com 
*Cc:* devel@edk2.groups.io 
*Subject:* RE: TCP Port for ISCSI Connection

Jiaxin:

Can you please comment on the below query.

-Siva

*From:* devel@edk2.groups.io 
[mailto:devel@edk2.groups.io] *On Behalf Of *Sivaraman Nainar
*Sent:* Friday, July 17, 2020 5:39 PM
*To:* devel@edk2.groups.io 
*Cc:* jiaxin...@intel.com 
*Subject:* [edk2-devel] Reg: TCP Port for ISCSI Connection

Hello all:

In the ISCSI driver, Target TCP Port Input shows the default port
as 3260. Which can be set from 0 to 65535

As per below RFC it talks about the Default Port only. Still it
not clearly said if we can use the numbers 49152-65535 which is
reserved.

https://tools.ietf.org/html/rfc3720

13. IANA Considerations

This section conforms to [RFC2434].

The well-known user TCP port number for iSCSI connections assigned by
IANA is 3260 and this is the default iSCSI port. Implementations
needing a system TCP port number may use port 860, the port assigned
by IANA as the iSCSI system port; however in order to use port 860,
it MUST be explicitly specified - implementations MUST NOT default to
use of port 860, as 3260 is the only allowed default.

with my understanding, it wouid be good if we can change the below
Min and  MAX port number ranges with right ranges.

#define TARGET_PORT_MIN_NUM   0

#define TARGET_PORT_MAX_NUM   65535

Thanks

Siva

This e-mail is intended for the use of the addressee only and may
contain privileged, confidential, or proprietary information that
is exempt from disclosure under law. If you have received this
message in error, please inform us promptly by reply e-mail, then
delete the e-mail and destroy any printed copy. Thank you.






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64899): https://edk2.groups.io/g/devel/message/64899
Mute This Topic: https://groups.io/mt/76530384/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 2/3] IntelSiliconPkg/IntelVTdPmrPei: Fix PMR enabling setting confilct

[Ni, Ray]

Reviewed-by: Ray Ni 

> -Original Message-
> From: Sheng, W 
> Sent: Monday, August 31, 2020 2:38 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Chaganty, Rangasai V 
> 
> Subject: [PATCH v3 2/3] IntelSiliconPkg/IntelVTdPmrPei: Fix PMR enabling 
> setting confilct
> 
> PMR enabling set by pre-boot DMA protection is cleared by RC
> when boot guard is enabled. Pre-boot DMA protection should only
> reset VT-d BAR when it is 0 and reset PMR region when it is
> not programmed to protect all memory address.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2867
> 
> Change-Id: Ic5370f474a43a94903871782ace5cce186b4ddc0
> Cc: Ray Ni 
> Cc: Rangasai V Chaganty 
> Signed-off-by: Sheng Wei 
> ---
>  .../Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c| 14 +++
>  .../Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.h| 15 +++
>  .../Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.inf  |  1 +
>  .../Feature/VTd/IntelVTdPmrPei/VtdReg.c| 47 
> ++
>  4 files changed, 77 insertions(+)
> 
> diff --git 
> a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c
> index ea944aa4..31a14f28 100644
> --- 
> a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c
> +++ 
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.c
> @@ -745,7 +745,21 @@ VTdInfoNotify (
>  // Protect all system memory
>  //
>  InitVTdInfo ();
> +
> +Hob = GetFirstGuidHob ();
> +VTdInfo = GET_GUID_HOB_DATA(Hob);
> +
> +//
> +// NOTE: We need check if PMR is enabled or not.
> +//
> +EnabledEngineMask = GetDmaProtectionEnabledEngineMask (VTdInfo, 
> VTdInfo->EngineMask);
> +if (EnabledEngineMask != 0) {
> +  Status = PreMemoryEnableVTdTranslationProtection (VTdInfo, 
> EnabledEngineMask);
> +}
>  InitVTdPmrForAll ();
> +if (((EnabledEngineMask != 0) && (!EFI_ERROR (Status {
> +  DisableVTdTranslationProtection (VTdInfo, EnabledEngineMask);
> +}
> 
>  //
>  // Install PPI.
> diff --git 
> a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.h
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.h
> index 58e6afad..ffed2c5b 100644
> --- 
> a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.h
> +++ 
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.h
> @@ -97,6 +97,21 @@ GetHighMemoryAlignment (
>IN UINT64EngineMask
>);
> 
> +/**
> +  Enable VTd translation table protection in pre-memory phase.
> +
> +  @param VTdInfoThe VTd engine context information.
> +  @param EngineMask The mask of the VTd engine to be accessed.
> +
> +  @retval EFI_SUCCESS   DMAR translation protection is enabled.
> +  @retval EFI_UNSUPPORTED   Null Root Entry Table is not supported.
> +**/
> +EFI_STATUS
> +PreMemoryEnableVTdTranslationProtection (
> +  IN VTD_INFO  *VTdInfo,
> +  IN UINT64EngineMask
> +  );
> +
>  /**
>Enable VTd translation table protection.
> 
> diff --git 
> a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.inf
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.inf
> index 3eb2b510..1e613ddd 100644
> --- 
> a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.inf
> +++ 
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/IntelVTdPmrPei.inf
> @@ -48,6 +48,7 @@
>gEdkiiVTdInfoPpiGuid## CONSUMES
>gEfiPeiMemoryDiscoveredPpiGuid  ## CONSUMES
>gEfiEndOfPeiSignalPpiGuid   ## CONSUMES
> +  gEdkiiVTdNullRootEntryTableGuid ## PRODUCES
> 
>  [Pcd]
>gIntelSiliconPkgTokenSpaceGuid.PcdVTdPolicyPropertyMask   ## CONSUMES
> diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/VtdReg.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/VtdReg.c
> index c9669426..2e252fe5 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/VtdReg.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/VtdReg.c
> @@ -13,8 +13,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
> +#include 
> 
>  #include "IntelVTdPmrPei.h"
> 
> @@ -246,6 +248,51 @@ DisableDmar (
>return EFI_SUCCESS;
>  }
> 
> +/**
> +  Enable VTd translation table protection in pre-memory phase.
> +
> +  @param VTdInfoThe VTd engine context information.
> +  @param EngineMask The mask of the VTd engine to be accessed.
> +
> +  @retval EFI_SUCCESS   DMAR translation protection is enabled.
> +  @retval EFI_UNSUPPORTED   Null Root Entry Table is not supported.
> +**/
> +EFI_STATUS
> +PreMemoryEnableVTdTranslationProtection (
> +  IN VTD_INFO  *VTdInfo,
> +  IN UINT64EngineMask
> +  )
> +{
> +  EFI_STATUSStatus;
> +  UINTN 

Re: [edk2-devel] [PATCH v3 1/3] IntelSiliconPkg/VtdInfo: Add Null Root Entry Table PPI

[Ni, Ray]

Reviewed-by: Ray Ni 

> -Original Message-
> From: Sheng, W 
> Sent: Monday, August 31, 2020 2:38 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray ; Chaganty, Rangasai V 
> 
> Subject: [PATCH v3 1/3] IntelSiliconPkg/VtdInfo: Add Null Root Entry Table PPI
> 
> Null root entry table address is a fixed silicon reserved address,
> which is used to block the DMA transfer.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2867
> 
> Change-Id: I3aa2b2e7a11e0327857c6ed9bc92cd209d3ade9d
> Cc: Ray Ni 
> Cc: Rangasai V Chaganty 
> Signed-off-by: Sheng Wei 
> ---
>  .../Include/Ppi/VtdNullRootEntryTable.h| 28 
> ++
>  Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec  |  1 +
>  2 files changed, 29 insertions(+)
>  create mode 100644 
> Silicon/Intel/IntelSiliconPkg/Include/Ppi/VtdNullRootEntryTable.h
> 
> diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Ppi/VtdNullRootEntryTable.h
> b/Silicon/Intel/IntelSiliconPkg/Include/Ppi/VtdNullRootEntryTable.h
> new file mode 100644
> index ..d79b5fd9
> --- /dev/null
> +++ b/Silicon/Intel/IntelSiliconPkg/Include/Ppi/VtdNullRootEntryTable.h
> @@ -0,0 +1,28 @@
> +/** @file
> +  The definition for VTD Null Root Entry Table PPI.
> +
> +  This is a lightweight VTd null root entry table report in PEI phase.
> +
> +  Copyright (c) 2020, Intel Corporation. All rights reserved.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef __VTD_NULL_ROOT_ENTRY_TABLE_PPI_H__
> +#define __VTD_NULL_ROOT_ENTRY_TABLE_PPI_H__
> +
> +#define EDKII_VTD_NULL_ROOT_ENTRY_TABLE_PPI_GUID \
> +{ \
> +  0x3de0593f, 0x6e3e, 0x4542, { 0xa1, 0xcb, 0xcb, 0xb2, 0xdb, 0xeb, 
> 0xd8, 0xff } \
> +}
> +
> +//
> +// Null root entry table address is a fixed silicon reserved address,
> +//   which is used to block the DMA transfer.
> +//
> +typedef UINT64  EDKII_VTD_NULL_ROOT_ENTRY_TABLE_PPI;
> +
> +extern EFI_GUID gEdkiiVTdNullRootEntryTableGuid;
> +
> +#endif
> +
> diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec 
> b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> index e4a7fec3..284820af 100644
> --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> +++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> @@ -68,6 +68,7 @@
> 
>  [Ppis]
>gEdkiiVTdInfoPpiGuid = { 0x8a59fcb3, 0xf191, 0x400c, { 0x97, 0x67, 0x67, 
> 0xaf, 0x2b, 0x25, 0x68, 0x4a } }
> +  gEdkiiVTdNullRootEntryTableGuid = { 0x3de0593f, 0x6e3e, 0x4542, { 0xa1, 
> 0xcb, 0xcb, 0xb2, 0xdb, 0xeb, 0xd8, 0xff } }
> 
>  [Protocols]
>gEdkiiPlatformVTdPolicyProtocolGuid = { 0x3d17e448, 0x466, 0x4e20, { 0x99, 
> 0x9f, 0xb2, 0xe1, 0x34, 0x88, 0xee, 0x22 }}
> --
> 2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64897): https://edk2.groups.io/g/devel/message/64897
Mute This Topic: https://groups.io/mt/76529333/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] TCP Port for ISCSI Connection

[Sivaraman Nainar]

Hello Maciej:

The ports numbers from 0 to 1023 are having specific roles.

Ex: 80 for HTTTP, 443 for HTTPS.

In the case can we set Min as 1024 and Max as 65535.

Thanks
Siva
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Maciej 
Rabeda
Sent: Tuesday, September 1, 2020 7:32 PM
To: devel@edk2.groups.io; Sivaraman Nainar
Subject: Re: [edk2-devel] TCP Port for ISCSI Connection

Hi Siva,

What seems to be the problem at hand? What kind of range of values for 
TargetPort do you propose?
TARGET_PORT_MIN/MAX_NUM refer to a range of values (0-65535) that be set in 
TargetPort field in iSCSI HII. 3260 is a default TCP port for iSCSI.
I see nothing wrong with that.

Thanks,
Maciej
On 31-Aug-20 11:05, Sivaraman Nainar wrote:
Rabeda:

Could you please provide your comment on this.

-Siva
From: Sivaraman Nainar
Sent: Tuesday, July 28, 2020 12:15 PM
To: jiaxin...@intel.com
Cc: devel@edk2.groups.io
Subject: RE: TCP Port for ISCSI Connection

Jiaxin:

Can you please comment on the below query.

-Siva
From: devel@edk2.groups.io 
[mailto:devel@edk2.groups.io] On Behalf Of Sivaraman Nainar
Sent: Friday, July 17, 2020 5:39 PM
To: devel@edk2.groups.io
Cc: jiaxin...@intel.com
Subject: [edk2-devel] Reg: TCP Port for ISCSI Connection

Hello all:

In the ISCSI driver, Target TCP Port Input shows the default port as 3260. 
Which can be set from 0 to 65535

As per below RFC it talks about the Default Port only. Still it not clearly 
said if we can use the numbers 49152-65535 which is reserved.
https://tools.ietf.org/html/rfc3720

13. IANA Considerations

This section conforms to [RFC2434].

The well-known user TCP port number for iSCSI connections assigned by
IANA is 3260 and this is the default iSCSI port. Implementations
needing a system TCP port number may use port 860, the port assigned
by IANA as the iSCSI system port; however in order to use port 860,
it MUST be explicitly specified - implementations MUST NOT default to
use of port 860, as 3260 is the only allowed default.

with my understanding, it wouid be good if we can change the below Min and  MAX 
port number ranges with right ranges.

#define TARGET_PORT_MIN_NUM   0
#define TARGET_PORT_MAX_NUM   65535

Thanks
Siva
This e-mail is intended for the use of the addressee only and may contain 
privileged, confidential, or proprietary information that is exempt from 
disclosure under law. If you have received this message in error, please inform 
us promptly by reply e-mail, then delete the e-mail and destroy any printed 
copy. Thank you.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64896): https://edk2.groups.io/g/devel/message/64896
Mute This Topic: https://groups.io/mt/76530384/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] TCP Port for ISCSI Connection

[Maciej Rabeda]

Hi Siva,

What seems to be the problem at hand? What kind of range of values for 
TargetPort do you propose?
TARGET_PORT_MIN/MAX_NUM refer to a range of values (0-65535) that be set 
in TargetPort field in iSCSI HII. 3260 is a default TCP port for iSCSI.

I see nothing wrong with that.

Thanks,
Maciej

On 31-Aug-20 11:05, Sivaraman Nainar wrote:


Rabeda:

Could you please provide your comment on this.

-Siva

*From:* Sivaraman Nainar
*Sent:* Tuesday, July 28, 2020 12:15 PM
*To:* jiaxin...@intel.com
*Cc:* devel@edk2.groups.io
*Subject:* RE: TCP Port for ISCSI Connection

Jiaxin:

Can you please comment on the below query.

-Siva

*From:* devel@edk2.groups.io  
[mailto:devel@edk2.groups.io] *On Behalf Of *Sivaraman Nainar

*Sent:* Friday, July 17, 2020 5:39 PM
*To:* devel@edk2.groups.io 
*Cc:* jiaxin...@intel.com 
*Subject:* [edk2-devel] Reg: TCP Port for ISCSI Connection

Hello all:

In the ISCSI driver, Target TCP Port Input shows the default port as 
3260. Which can be set from 0 to 65535


As per below RFC it talks about the Default Port only. Still it not 
clearly said if we can use the numbers 49152-65535 which is reserved.


https://tools.ietf.org/html/rfc3720

13. IANA Considerations

This section conforms to [RFC2434].

The well-known user TCP port number for iSCSI connections assigned by
IANA is 3260 and this is the default iSCSI port. Implementations
needing a system TCP port number may use port 860, the port assigned
by IANA as the iSCSI system port; however in order to use port 860,
it MUST be explicitly specified - implementations MUST NOT default to
use of port 860, as 3260 is the only allowed default.

with my understanding, it wouid be good if we can change the below Min 
and  MAX port number ranges with right ranges.


#define TARGET_PORT_MIN_NUM   0

#define TARGET_PORT_MAX_NUM   65535

Thanks

Siva

This e-mail is intended for the use of the addressee only and may 
contain privileged, confidential, or proprietary information that is 
exempt from disclosure under law. If you have received this message in 
error, please inform us promptly by reply e-mail, then delete the 
e-mail and destroy any printed copy. Thank you.






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64895): https://edk2.groups.io/g/devel/message/64895
Mute This Topic: https://groups.io/mt/76530384/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: 回复: [edk2-devel] Patch List for 202008 stable tag

[Laszlo Ersek]

On 09/01/20 11:35, gaoliming wrote:
> Hi, all
> There are still some patches to be merged for 202008 stable tag. They
> are all bug fixes. The risk is low. I agree to catch them in this
> stable tag. If you have other comments, please reply this mail.
>
> https://edk2.groups.io/g/devel/message/64873
> [PATCH v3] MdeModulePkg/Library: add PEIM and SEC module type to 
> TpmMeasurementLibNull

Needs maintainer approval, and then it should be merged, yes.


> https://edk2.groups.io/g/devel/message/64865
> [PATCH] SecurityPkg: Initailize variable Status before it is consumed.

Needs maintainer approval, and then it should be merged, yes.


> https://edk2.groups.io/g/devel/message/64812
> [PATCH] IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec: add FspMeasurementLib.h

Yes, should be merged (it has been Reviewed-by: Chasel Chiu
).


More candidates (bugfixes):

* [Patch V2] BaseTools/Ecc: Fix an issue of path separator compatibility
  https://edk2.groups.io/g/devel/message/64887
  http://mid.mail-archive.com/  <20200901102315.38840-1-bob.c.f...@intel.com

Needs maintainer review, and then it should be merged.


* [PATCH EDK2 v1 0/1] EmulatorPkg/host: fix overflow in Mult
  https://edk2.groups.io/g/devel/message/64890
  http://mid.mail-archive.com/  
<1598957888-128729-1-git-send-email-xiewen...@huawei.com

Needs a review and a maintainer decision whether it is material for the
stable tag.


* [PATCH 0/3] SecurityPkg/DxeImageVerificationLib: catch alignment overflow 
(CVE-2019-14562)
  https://edk2.groups.io/g/devel/message/64882
  http://mid.mail-archive.com/  <20200901091221.20948-1-ler...@redhat.com

Needs maintainer review, plus testing from Wenyi Xie with their
reproducer, plus a maintainer decision whether it is material for the
stable tag.

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64894): https://edk2.groups.io/g/devel/message/64894
Mute This Topic: https://groups.io/mt/76552730/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 0/6] Platform/RasberryPi: Thermal zone

[Ard Biesheuvel]

On 8/31/20 7:25 PM, Jeremy Linton wrote:

This set creates a basic thermal zone, which reads the
SOC temp via a direct register read in AML. It also
adds an active cooling policy using a GPIO pin for fan
control that can optionally be enabled/disabled on either
GPIO18 (commercial fan shim board) or GPIO19 by the
user from the BDS.

it should now be possible when booted in ACPI mode to:

# sensors
acpitz-acpi-0
Adapter: ACPI interface
temp1:+57.6C  (crit = +90.0C)

and the fan state, if enabled may be read with:

/sys/bus/acpi/devices/PNP0C06:00/PNP0C0B:00/physical_node/thermal_cooling/cur_state

while the kernel will automatically cycle the fan if the SoC
temp exceeds 60C.

Included as a byproduct of this set is a generic method
for updating values in ACPI DSDT/SSDT tables as they
are installed.

v3->v4:
   Allow the user to set the fan trip point
   Extend ACPI device names to 4 characters

v2->v3:
   Make DSDT/SSDT PCD->Nameop update code generic
   Move the fan SSDT code back into the AcpiTable directory

v1->v2:
   Add choice of GPIO pins to the BDS menu
   Correct whitespace/etc issues from v1 review
   Add an additional cleanup patch for contextual whitespace issues

Cc: Leif Lindholm 
Cc: Pete Batard 
Cc: Andrei Warkentin 
Cc: Ard Biesheuvel 
Cc: Samer El-Haj-Mahmoud 

Jeremy Linton (6):
   Platform/RaspberryPi4: Add a basic thermal zone
   Platform/RaspberryPi: Monitor ACPI Table installs
   Platform/RaspberryPi: Add entry for user fan control
   Platform/RaspberryPi4: Create ACPI fan object
   Platform/RaspberryPi4: Allow the user to set Temp
   Platform/RaspberryPi: Trivial whitespace cleanup



Series pushed as 50639477fc0f..e08fccd8bed5

Note that I incorporated the changes suggested by Pete, as well as the 
Mmio Or/And tweak I mentioned in response to patch #6. I also applied 
some touch ups to patch #2, to make the coding style more idiomatic for 
EDK2, and to use CONST structures where possible.


Thanks all. This is a really nice feature to have, especially as it can 
serve as an example for other platforms as well.





  Platform/RaspberryPi/AcpiTables/AcpiTables.inf |   1 +
  Platform/RaspberryPi/AcpiTables/Dsdt.asl   |  31 +++
  Platform/RaspberryPi/AcpiTables/SsdtThermal.asl|  77 
  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 214 +++--
  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf|   3 +
  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni |   9 +
  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr |  34 
  Platform/RaspberryPi/Include/ConfigVars.h  |   8 +
  Platform/RaspberryPi/RPi3/RPi3.dsc |   6 +
  Platform/RaspberryPi/RPi4/RPi4.dsc |   9 +
  Platform/RaspberryPi/RaspberryPi.dec   |   2 +
  .../Bcm27xx/Include/IndustryStandard/Bcm2711.h |   2 +
  12 files changed, 382 insertions(+), 14 deletions(-)
  create mode 100644 Platform/RaspberryPi/AcpiTables/SsdtThermal.asl




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64893): https://edk2.groups.io/g/devel/message/64893
Mute This Topic: https://groups.io/mt/76538742/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 6/6] Platform/RaspberryPi: Trivial whitespace cleanup

[Pete Batard]

Hi Ard,

On 2020.09.01 12:39, Ard Biesheuvel wrote:

On 8/31/20 7:25 PM, Jeremy Linton wrote:

Pete's review pointed out some whitespace issues in the
context of a previous patch. Since there are a number of
similar errors in the file lets fix them separately.

Cc: Leif Lindholm 
Cc: Pete Batard 
Cc: Andrei Warkentin 
Cc: Ard Biesheuvel 
Cc: Samer El-Haj-Mahmoud 
Signed-off-by: Jeremy Linton 
Reviewed-by: Pete Batard <@pbatard>
---
  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 24 
+++---

  1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c

index e8f964a329..4ed294cdfe 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -209,9 +209,9 @@ SetupVariables (
    }
    Size = sizeof (UINT32);
-  Status = gRT->GetVariable(L"CustomCpuClock",
-    ,
-    NULL, , );
+  Status = gRT->GetVariable (L"CustomCpuClock",
+ ,
+ NULL, , );
    if (EFI_ERROR (Status)) {
  Status = PcdSet32S (PcdCustomCpuClock, PcdGet32 
(PcdCustomCpuClock));

  ASSERT_EFI_ERROR (Status);
@@ -266,7 +266,7 @@ SetupVariables (
    Size = sizeof (AssetTagVar);
-  Status = gRT->GetVariable(L"AssetTag",
+  Status = gRT->GetVariable (L"AssetTag",
    ,
    NULL, , AssetTagVar);
@@ -275,7 +275,7 @@ SetupVariables (
  L"AssetTag",
  ,
  EFI_VARIABLE_NON_VOLATILE | 
EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,

-    sizeof(AssetTagVar),
+    sizeof (AssetTagVar),
  AssetTagVar
  );
    }
@@ -441,9 +441,9 @@ ApplyVariables (
   * spaces. SystemMemorySizeBelow4GB tracks the maximum memory 
below 4GB

   * line, factoring in the limit imposed by the SoC register range.
   */
-    SystemMemorySizeBelow4GB = MIN(SystemMemorySize, 4UL * SIZE_1GB);
-    SystemMemorySizeBelow4GB = MIN(SystemMemorySizeBelow4GB, 
BCM2836_SOC_REGISTERS);
-    SystemMemorySizeBelow4GB = MIN(SystemMemorySizeBelow4GB, 
BCM2711_SOC_REGISTERS);

+    SystemMemorySizeBelow4GB = MIN (SystemMemorySize, 4UL * SIZE_1GB);
+    SystemMemorySizeBelow4GB = MIN (SystemMemorySizeBelow4GB, 
BCM2836_SOC_REGISTERS);
+    SystemMemorySizeBelow4GB = MIN (SystemMemorySizeBelow4GB, 
BCM2711_SOC_REGISTERS);

  ASSERT (SystemMemorySizeBelow4GB > 3UL * SIZE_1GB);
@@ -536,14 +536,14 @@ ApplyVariables (
    /*
 * SD card pins go to Arasan.
 */
-  MmioWrite32((GPIO_BASE_ADDRESS + 0xD0),
-  MmioRead32(GPIO_BASE_ADDRESS + 0xD0) | 0x2);
+  MmioWrite32 ((GPIO_BASE_ADDRESS + 0xD0),
+  MmioRead32 (GPIO_BASE_ADDRESS + 0xD0) | 0x2);
  } else {
    /*
 * SD card pins back to eMMC2.
 */
-  MmioWrite32((GPIO_BASE_ADDRESS + 0xD0),
-  MmioRead32(GPIO_BASE_ADDRESS + 0xD0) & ~0x2);
+  MmioWrite32 ((GPIO_BASE_ADDRESS + 0xD0),
+  MmioRead32 (GPIO_BASE_ADDRESS + 0xD0) & ~0x2);


Anyone mind if I replace these with MmioOr32 / MmioAnd32 ?


No objection from me. This should indeed make the code cleaner.

Thanks,

/Pete





    /*
 * WiFi back to Arasan.
 */






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64892): https://edk2.groups.io/g/devel/message/64892
Mute This Topic: https://groups.io/mt/76538751/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v4 6/6] Platform/RaspberryPi: Trivial whitespace cleanup

[Ard Biesheuvel]

On 8/31/20 7:25 PM, Jeremy Linton wrote:

Pete's review pointed out some whitespace issues in the
context of a previous patch. Since there are a number of
similar errors in the file lets fix them separately.

Cc: Leif Lindholm 
Cc: Pete Batard 
Cc: Andrei Warkentin 
Cc: Ard Biesheuvel 
Cc: Samer El-Haj-Mahmoud 
Signed-off-by: Jeremy Linton 
Reviewed-by: Pete Batard <@pbatard>
---
  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 24 +++---
  1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index e8f964a329..4ed294cdfe 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -209,9 +209,9 @@ SetupVariables (
}
  
Size = sizeof (UINT32);

-  Status = gRT->GetVariable(L"CustomCpuClock",
-,
-NULL, , );
+  Status = gRT->GetVariable (L"CustomCpuClock",
+ ,
+ NULL, , );
if (EFI_ERROR (Status)) {
  Status = PcdSet32S (PcdCustomCpuClock, PcdGet32 (PcdCustomCpuClock));
  ASSERT_EFI_ERROR (Status);
@@ -266,7 +266,7 @@ SetupVariables (
  
  
Size = sizeof (AssetTagVar);

-  Status = gRT->GetVariable(L"AssetTag",
+  Status = gRT->GetVariable (L"AssetTag",
,
NULL, , AssetTagVar);
  
@@ -275,7 +275,7 @@ SetupVariables (

  L"AssetTag",
  ,
  EFI_VARIABLE_NON_VOLATILE | 
EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
-sizeof(AssetTagVar),
+sizeof (AssetTagVar),
  AssetTagVar
  );
}
@@ -441,9 +441,9 @@ ApplyVariables (
   * spaces. SystemMemorySizeBelow4GB tracks the maximum memory below 4GB
   * line, factoring in the limit imposed by the SoC register range.
   */
-SystemMemorySizeBelow4GB = MIN(SystemMemorySize, 4UL * SIZE_1GB);
-SystemMemorySizeBelow4GB = MIN(SystemMemorySizeBelow4GB, 
BCM2836_SOC_REGISTERS);
-SystemMemorySizeBelow4GB = MIN(SystemMemorySizeBelow4GB, 
BCM2711_SOC_REGISTERS);
+SystemMemorySizeBelow4GB = MIN (SystemMemorySize, 4UL * SIZE_1GB);
+SystemMemorySizeBelow4GB = MIN (SystemMemorySizeBelow4GB, 
BCM2836_SOC_REGISTERS);
+SystemMemorySizeBelow4GB = MIN (SystemMemorySizeBelow4GB, 
BCM2711_SOC_REGISTERS);
  
  ASSERT (SystemMemorySizeBelow4GB > 3UL * SIZE_1GB);
  
@@ -536,14 +536,14 @@ ApplyVariables (

/*
 * SD card pins go to Arasan.
 */
-  MmioWrite32((GPIO_BASE_ADDRESS + 0xD0),
-  MmioRead32(GPIO_BASE_ADDRESS + 0xD0) | 0x2);
+  MmioWrite32 ((GPIO_BASE_ADDRESS + 0xD0),
+  MmioRead32 (GPIO_BASE_ADDRESS + 0xD0) | 0x2);
  } else {
/*
 * SD card pins back to eMMC2.
 */
-  MmioWrite32((GPIO_BASE_ADDRESS + 0xD0),
-  MmioRead32(GPIO_BASE_ADDRESS + 0xD0) & ~0x2);
+  MmioWrite32 ((GPIO_BASE_ADDRESS + 0xD0),
+  MmioRead32 (GPIO_BASE_ADDRESS + 0xD0) & ~0x2);


Anyone mind if I replace these with MmioOr32 / MmioAnd32 ?



/*
 * WiFi back to Arasan.
 */




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64891): https://edk2.groups.io/g/devel/message/64891
Mute This Topic: https://groups.io/mt/76538751/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH EDK2 v1 0/1] EmulatorPkg/host: fix overflow in Mult

[wenyi,xie via groups.io]

Main Changes :
convert int to uint64 to avoid overflow when Multiply two int.

Wenyi Xie (1):
  EmulatorPkg/host: fix overflow in Mult

 EmulatorPkg/Win/Host/WinHost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.20.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64890): https://edk2.groups.io/g/devel/message/64890
Mute This Topic: https://groups.io/mt/76553536/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH EDK2 v1 1/1] EmulatorPkg/host: fix overflow in Mult

[wenyi,xie via groups.io]

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

When calculating memory regions and store the information in the
gSystemMemory in file WinHost.c, the code below will cause overflow,
because _wtoi (MemorySizeStr) return an int value and SIZE_1MB is
also an int value, if MemorySizeStr is lager for example 2048, then
result of multiplication will overflow.

for (Index = 0, Done = FALSE; !Done; Index++) {
  //
  // Save the size of the memory and make a Unicode filename SystemMemory00
  //
  gSystemMemory[Index].Size = _wtoi (MemorySizeStr) * SIZE_1MB;

Cc: Jordan Justen 
Cc: Andrew Fish 
Cc: Ray Ni 
Signed-off-by: Wenyi Xie 
---
 EmulatorPkg/Win/Host/WinHost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmulatorPkg/Win/Host/WinHost.c b/EmulatorPkg/Win/Host/WinHost.c
index 0838c56ddea8..876cb8d4be8b 100644
--- a/EmulatorPkg/Win/Host/WinHost.c
+++ b/EmulatorPkg/Win/Host/WinHost.c
@@ -577,7 +577,7 @@ Returns:
 //
 // Save the size of the memory and make a Unicode filename SystemMemory00, 
...
 //
-gSystemMemory[Index].Size = _wtoi (MemorySizeStr) * SIZE_1MB;
+gSystemMemory[Index].Size = ((UINT64)_wtoi (MemorySizeStr)) * 
((UINT64)SIZE_1MB);
 
 //
 // Find the next region
-- 
2.20.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64889): https://edk2.groups.io/g/devel/message/64889
Mute This Topic: https://groups.io/mt/76553535/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch 1/1] BaseTools/Ecc: Fix an issue of path separator compatibility

[Bob Feng]

Laszlo,

1) there is no special reason not using os.sep, I sent out a new patch to use 
it.

Liming,

2) If this patch can be included in this stable tag, that will great.  From the 
Bugzilla, I see this issue will block patch merging.

Thanks,
Bob

-Original Message-
From: Laszlo Ersek  
Sent: Tuesday, September 1, 2020 4:06 PM
To: devel@edk2.groups.io; Feng, Bob C 
Cc: Liming Gao ; Chen, Christine 
; Zhang, Shenglei 
Subject: Re: [edk2-devel] [Patch 1/1] BaseTools/Ecc: Fix an issue of path 
separator compatibility

On 08/31/20 18:14, Bob Feng wrote:
> From: "Bob Feng" 
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2904
> 
> The path separator is different in Windows and Linux, the original 
> code does not handle this difference. This patch is to fix this issue.
> 
> Signed-off-by: Bob Feng 
> Cc: Liming Gao 
> Cc: Yuwei Chen 
> Cc: Shenglei Zhang 
> ---
>  BaseTools/Source/Python/Ecc/Check.py | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/Ecc/Check.py 
> b/BaseTools/Source/Python/Ecc/Check.py
> index 0fdc7e35c18a..ca24c8acffeb 100644
> --- a/BaseTools/Source/Python/Ecc/Check.py
> +++ b/BaseTools/Source/Python/Ecc/Check.py
> @@ -13,10 +13,11 @@ from Ecc.EccToolError import *  from 
> Ecc.MetaDataParser import ParseHeaderCommentSection  from Ecc import 
> EccGlobalData  from Ecc import c  from Common.LongFilePathSupport 
> import OpenLongFilePath as open  from Common.MultipleWorkspace import 
> MultipleWorkspace as mws
> +import platform
>  
>  ## Check
>  #
>  # This class is to define checkpoints used by ECC tool  # @@ -1099,17 
> +1100,18 @@ class Check(object):
>  InfPathSet = EccGlobalData.gDb.TblInf.Exec(SqlCommand)
>  InfPathList = []
>  for Item in InfPathSet:
>  if Item[0] not in InfPathList:
>  InfPathList.append(Item[0])
> +pathsep = """'\\'""" if platform.system() == 'Windows' else 
> """'/'"""
>  SqlCommand = """
>   select ID, Path, FullPath from File where 
> upper(FullPath) not in
> -(select upper(A.Path) || '\\' || upper(B.Value1) 
> from File as A, INF as B
> +(select upper(A.Path) || %s || 
> + upper(B.Value1) from File as A, INF as B
>  where A.ID in (select BelongsToFile from INF 
> where Model = %s group by BelongsToFile) and
>  B.BelongsToFile = A.ID and B.Model = %s)
>  and (Model = %s or Model = %s)
> -""" % (MODEL_EFI_SOURCE_FILE, MODEL_EFI_SOURCE_FILE, 
> MODEL_FILE_C, MODEL_FILE_H)
> +""" % (pathsep, MODEL_EFI_SOURCE_FILE, 
> + MODEL_EFI_SOURCE_FILE, MODEL_FILE_C, MODEL_FILE_H)
>  RecordSet = EccGlobalData.gDb.TblInf.Exec(SqlCommand)
>  for Record in RecordSet:
>  Path = Record[1]
>  Path = Path.upper().replace('\X64', '').replace('\IA32', 
> '').replace('\EBC', '').replace('\IPF', '').replace('\ARM', '')
>  if Path in InfPathList:
> @@ -1122,21 +1124,22 @@ class Check(object):
>  EdkLogger.quiet("Checking for pcd type in c code function usage 
> ...")
>  SqlCommand = """
>   select ID, Model, Value1, Value2, BelongsToFile 
> from INF where Model > %s and Model < %s
>   """ % (MODEL_PCD, MODEL_META_DATA_HEADER)
>  PcdSet = EccGlobalData.gDb.TblInf.Exec(SqlCommand)
> +pathsep = """'\\'""" if platform.system() == 'Windows' else 
> """'/'"""
>  for Pcd in PcdSet:
>  Model = Pcd[1]
>  PcdName = Pcd[2]
>  if Pcd[3]:
>  PcdName = Pcd[3]
>  BelongsToFile = Pcd[4]
>  SqlCommand = """
>   select ID from File where FullPath in
> -(select B.Path || '\\' || A.Value1 from INF as 
> A, File as B where A.Model = %s and A.BelongsToFile = %s
> +(select B.Path || %s || A.Value1 from INF 
> + as A, File as B where A.Model = %s and A.BelongsToFile = %s
>   and B.ID = %s and (B.Model = %s or B.Model = 
> %s))
> - """ % (MODEL_EFI_SOURCE_FILE, BelongsToFile, 
> BelongsToFile, MODEL_FILE_C, MODEL_FILE_H)
> + """ % (pathsep, MODEL_EFI_SOURCE_FILE, 
> + BelongsToFile, BelongsToFile, MODEL_FILE_C, MODEL_FILE_H)
>  TableSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
>  for Tbl in TableSet:
>  TblName = 'Identifier' + str(Tbl[0])
>  SqlCommand = """
>   select Name, ID from %s where value 
> like '%s' and Model = %s
> 

(1) any reason for not using os.sep (aka os.path.sep) 

[edk2-devel] [Patch V2] BaseTools/Ecc: Fix an issue of path separator compatibility

[Bob Feng]

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

The path separator is different in Windows and Linux, the
original code does not handle this difference. This patch
is to fix this issue.

Signed-off-by: Bob Feng 
Cc: Liming Gao 
Cc: Yuwei Chen 
Cc: Shenglei Zhang 
---
V2
Change to a better method to get path separator.

 BaseTools/Source/Python/Ecc/Check.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/Python/Ecc/Check.py 
b/BaseTools/Source/Python/Ecc/Check.py
index 0fdc7e35c1..6087abfa4d 100644
--- a/BaseTools/Source/Python/Ecc/Check.py
+++ b/BaseTools/Source/Python/Ecc/Check.py
@@ -1101,15 +1101,15 @@ class Check(object):
 for Item in InfPathSet:
 if Item[0] not in InfPathList:
 InfPathList.append(Item[0])
 SqlCommand = """
  select ID, Path, FullPath from File where 
upper(FullPath) not in
-(select upper(A.Path) || '\\' || upper(B.Value1) 
from File as A, INF as B
+(select upper(A.Path) || '%s' || upper(B.Value1) 
from File as A, INF as B
 where A.ID in (select BelongsToFile from INF where 
Model = %s group by BelongsToFile) and
 B.BelongsToFile = A.ID and B.Model = %s)
 and (Model = %s or Model = %s)
-""" % (MODEL_EFI_SOURCE_FILE, MODEL_EFI_SOURCE_FILE, 
MODEL_FILE_C, MODEL_FILE_H)
+""" % (os.sep, MODEL_EFI_SOURCE_FILE, 
MODEL_EFI_SOURCE_FILE, MODEL_FILE_C, MODEL_FILE_H)
 RecordSet = EccGlobalData.gDb.TblInf.Exec(SqlCommand)
 for Record in RecordSet:
 Path = Record[1]
 Path = Path.upper().replace('\X64', '').replace('\IA32', 
'').replace('\EBC', '').replace('\IPF', '').replace('\ARM', '')
 if Path in InfPathList:
@@ -1130,13 +1130,13 @@ class Check(object):
 if Pcd[3]:
 PcdName = Pcd[3]
 BelongsToFile = Pcd[4]
 SqlCommand = """
  select ID from File where FullPath in
-(select B.Path || '\\' || A.Value1 from INF as A, 
File as B where A.Model = %s and A.BelongsToFile = %s
+(select B.Path || '%s' || A.Value1 from INF as A, 
File as B where A.Model = %s and A.BelongsToFile = %s
  and B.ID = %s and (B.Model = %s or B.Model = %s))
- """ % (MODEL_EFI_SOURCE_FILE, BelongsToFile, 
BelongsToFile, MODEL_FILE_C, MODEL_FILE_H)
+ """ % (os.sep, MODEL_EFI_SOURCE_FILE, 
BelongsToFile, BelongsToFile, MODEL_FILE_C, MODEL_FILE_H)
 TableSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
 for Tbl in TableSet:
 TblName = 'Identifier' + str(Tbl[0])
 SqlCommand = """
  select Name, ID from %s where value like '%s' 
and Model = %s
-- 
2.20.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64887): https://edk2.groups.io/g/devel/message/64887
Mute This Topic: https://groups.io/mt/76553178/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

回复: [edk2-devel] Patch List for 202008 stable tag

[gaoliming]

Hi, all
  There are still some patches to be merged for 202008 stable tag. They are
all bug fixes. The risk is low. I agree to catch them in this stable tag. If
you have other comments, please reply this mail. 

https://edk2.groups.io/g/devel/message/64873 [PATCH v3]
MdeModulePkg/Library: add PEIM and SEC module type to TpmMeasurementLibNull
https://edk2.groups.io/g/devel/message/64865 [PATCH] SecurityPkg: Initailize
variable Status before it is consumed.
https://edk2.groups.io/g/devel/message/64812 [PATCH]
IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec: add FspMeasurementLib.h

Thanks
Liming
> -邮件原件-
> 发件人: bounce+27952+64554+4905953+8761...@groups.io
>  代表 gaoliming
> 发送时间: 2020年8月22日 8:16
> 收件人: devel@edk2.groups.io; ler...@redhat.com; 'Leif Lindholm'
> ; af...@apple.com; 'Kinney, Michael D'
> ; 'Guptha, Soumya K'
> ; 'Chang, Abner (HPS SW/FW Technologist)'
> ; 'Vladimir Olovyannikov'
> ; 'Tom Lendacky'
> ; gaolim...@byosoft.com.cn
> 主题: 答复: [edk2-devel] Patch List for 202008 stable tag
> 
> Laszlo:
> 
> > -邮件原件-
> > 发件人: bounce+27952+64532+4905953+8761...@groups.io
> > [mailto:bounce+27952+64532+4905953+8761...@groups.io] 代表 Laszlo
> > Ersek
> > 发送时间: 2020年8月21日 19:07
> > 收件人: Gao, Liming ; Leif Lindholm
> > ; af...@apple.com; Kinney, Michael D
> > ; Guptha, Soumya K
> > ; Chang, Abner (HPS SW/FW Technologist)
> > ; Vladimir Olovyannikov
> > ; Tom Lendacky
> > 
> > 抄送: devel@edk2.groups.io
> > 主题: Re: [edk2-devel] Patch List for 202008 stable tag
> >
> > On 08/20/20 13:30, Gao, Liming wrote:
> > > Hi Stewards and all:
> > >   I collect current patch lists in devel mail list. Those patch
> contributors
> > request to add them for 202008 stable tag. Because we have enter into
Soft
> > Feature Freeze, I want to collect your feedback for them. If any patches
> are
> > missing, please reply this mail to add them.
> > >
> > > Feature List:
> > > https://edk2.groups.io/g/devel/message/63767 [PATCH]
> > > EmbeddedPkg/libfdt: Add strncmp macro to use AsciiStrnCmp [Liming]
> This
> > patch pass code review after Soft Feature Freeze (SFF) starts. According
> to SFF
> > definition, it should not be merged for this stable tag. But, the patch
> submitter
> > says this patch is important to RISC-V community. To catch it for this
> stable tag,
> > Laszlo proposed the solution to defer SFF start day from 2020-08-14 to
> > 2020-08-21, then hard feature freeze and release date will also defer
one
> > week. Any patches those pass review before new SFF start day can be
> merged.
> > @ Stewards, please give your comments to defer this stable tag release
by
> > one week.
> >
> > Thank you, very nice summary.
> >
> > As stated earlier, I'm OK with 1 week (or even two weeks, if needed)
> > extension. I invite Andrew, Leif and Mike to comment.
> >
> [Liming] I don't see any other urgent request. I think it is enough to
defer
> 1 week for this stable tag.
> So far, there is objection for this defer. I will send the announcement
> mail.
> 
> > >
> > > https://edk2.groups.io/g/devel/message/63348 [PATCH v5 1/1]
> > > ShellPkg/DynamicCommand: add HttpDynamicCommand [Liming] This
> > patch has collected the review comment. New version will be sent. I have
> no
> > information how important it is. @Vladimir, does this patch must catch
> this
> > stable tag? If yes, can you give the reason?
> >
> > This is a feature addition. While it's a super useful feature, it needs
to
> mature
> > (mainly from the coding style perspective, as I gather).
> > Posting of new versions and review are on-going.
> >
> > Most likely material for the next stable tag. In case we extend the
> deadlines,
> > and Maciej and the ShellPkg maintainers approve one of the upcoming
> > versions until the new SFF, then yes, we can merge that too.
> >
> [Liming] So, this is not urgent. Let's defer it to next stable tag.
> 
> Thanks
> Liming
> 
> > > Bug List:
> > > https://edk2.groups.io/g/devel/message/64383 [PATCH 1/1]
> > > UefiCpuPkg/MpInitLib: Always initialize the DoDecrement variable
> [Liming]
> > This patch has collected the review comment. New version will be sent.
> >
> > This is a bugfix. Certainly a build fix on CLANGPDB. I tried to do some
> > (lightweight) analysis to see whether the bug is a functional one as
well
> (i.e.
> > whether the bad code path that CLANG warns about can actually occur in
> > practice), but in the end I didn't want to spend much time on it, as the
> build
> > breakage needs to be fixed anyway.
> >
> > So, this patch (more precisely, v2 of this patch) would even qualify
> during the
> > hard feature freeze (as it's clearly a bugfix).
> >
> > >
> > > https://edk2.groups.io/g/devel/message/50406 [PATCH 1/1]
> > > MdePkg/Include: Add missing definitions of SMBIOS type 42h in SmBios.h
> > [Liming] This patch passed review early. But, it is not merged. I will
> merge it.
> >
> > Right, it was approved on 2019-Nov-18. If it still applies cleanly, it
> should be
> > merged, before we enter the Hard Feature Freeze.
> >
> > 

[edk2-devel] [PATCH 1/3] SecurityPkg/DxeImageVerificationLib: extract SecDataDirEnd, SecDataDirLeft

[Laszlo Ersek]

The following two quantities:

  SecDataDir->VirtualAddress + SecDataDir->Size
  SecDataDir->VirtualAddress + SecDataDir->Size - OffSet

are used multiple times in DxeImageVerificationHandler(). Introduce helper
variables for them: "SecDataDirEnd" and "SecDataDirLeft", respectively.
This saves us multiple calculations and significantly simplifies the code.

Note that all three summands above have type UINT32, therefore the new
variables are also of type UINT32.

This patch does not change behavior.

(Note that the code already handles the case when the

  SecDataDir->VirtualAddress + SecDataDir->Size

UINT32 addition overflows -- namely, in that case, the certificate loop is
never entered, and the corruption check right after the loop fires.)

Cc: Jian J Wang 
Cc: Jiewen Yao 
Cc: Min Xu 
Cc: Wenyi Xie 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215
Signed-off-by: Laszlo Ersek 
---
 SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 12 

 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git 
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index b08fe24e85aa..377feebb205a 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1652,6 +1652,8 @@ DxeImageVerificationHandler (
   UINT8*AuthData;
   UINTNAuthDataSize;
   EFI_IMAGE_DATA_DIRECTORY *SecDataDir;
+  UINT32   SecDataDirEnd;
+  UINT32   SecDataDirLeft;
   UINT32   OffSet;
   CHAR16   *NameStr;
   RETURN_STATUSPeCoffStatus;
@@ -1849,12 +1851,14 @@ DxeImageVerificationHandler (
   // "Attribute Certificate Table".
   // The first certificate starts at offset (SecDataDir->VirtualAddress) from 
the start of the file.
   //
+  SecDataDirEnd = SecDataDir->VirtualAddress + SecDataDir->Size;
   for (OffSet = SecDataDir->VirtualAddress;
-   OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
+   OffSet < SecDataDirEnd;
OffSet += (WinCertificate->dwLength + ALIGN_SIZE 
(WinCertificate->dwLength))) {
 WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
-if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof 
(WIN_CERTIFICATE) ||
-(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < 
WinCertificate->dwLength) {
+SecDataDirLeft = SecDataDirEnd - OffSet;
+if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
+SecDataDirLeft < WinCertificate->dwLength) {
   break;
 }
 
@@ -1948,7 +1952,7 @@ DxeImageVerificationHandler (
 }
   }
 
-  if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size)) {
+  if (OffSet != SecDataDirEnd) {
 //
 // The Size in Certificate Table or the attribute certificate table is 
corrupted.
 //
-- 
2.19.1.3.g30247aa5d201



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64884): https://edk2.groups.io/g/devel/message/64884
Mute This Topic: https://groups.io/mt/76552540/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 3/3] SecurityPkg/DxeImageVerificationLib: catch alignment overflow (CVE-2019-14562)

[Laszlo Ersek]

The DxeImageVerificationHandler() function currently checks whether
"SecDataDir" has enough room for "WinCertificate->dwLength". However, for
advancing "OffSet", "WinCertificate->dwLength" is aligned to the next
multiple of 8. If "WinCertificate->dwLength" is large enough, the
alignment will return 0, and "OffSet" will be stuck at the same value.

Check whether "SecDataDir" has room left for both
"WinCertificate->dwLength" and the alignment.

Cc: Jian J Wang 
Cc: Jiewen Yao 
Cc: Min Xu 
Cc: Wenyi Xie 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215
Signed-off-by: Laszlo Ersek 
---
 SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git 
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 100739eb3eb6..11154b6cc58a 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1860,7 +1860,9 @@ DxeImageVerificationHandler (
   break;
 }
 WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
-if (SecDataDirLeft < WinCertificate->dwLength) {
+if (SecDataDirLeft < WinCertificate->dwLength ||
+(SecDataDirLeft - WinCertificate->dwLength <
+ ALIGN_SIZE (WinCertificate->dwLength))) {
   break;
 }
 
-- 
2.19.1.3.g30247aa5d201


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64885): https://edk2.groups.io/g/devel/message/64885
Mute This Topic: https://groups.io/mt/76552541/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 0/3] SecurityPkg/DxeImageVerificationLib: catch alignment overflow (CVE-2019-14562)

[Laszlo Ersek]

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

I'm neutral on whether this becomes part of edk2-stable202008.

Cc: Jian J Wang 
Cc: Jiewen Yao 
Cc: Min Xu 
Cc: Wenyi Xie 

Thanks,
Laszlo

Laszlo Ersek (3):
  SecurityPkg/DxeImageVerificationLib: extract SecDataDirEnd,
SecDataDirLeft
  SecurityPkg/DxeImageVerificationLib: assign WinCertificate after size
check
  SecurityPkg/DxeImageVerificationLib: catch alignment overflow
(CVE-2019-14562)

 SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 16 

 1 file changed, 12 insertions(+), 4 deletions(-)

-- 
2.19.1.3.g30247aa5d201


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64882): https://edk2.groups.io/g/devel/message/64882
Mute This Topic: https://groups.io/mt/76552538/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 2/3] SecurityPkg/DxeImageVerificationLib: assign WinCertificate after size check

[Laszlo Ersek]

Currently the (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) check only
guards the de-referencing of the "WinCertificate" pointer. It does not
guard the calculation of the pointer itself:

  WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);

This is wrong; if we don't know for sure that we have enough room for a
WIN_CERTIFICATE, then even creating such a pointer, not just
de-referencing it, may invoke undefined behavior.

Move the pointer calculation after the size check.

Cc: Jian J Wang 
Cc: Jiewen Yao 
Cc: Min Xu 
Cc: Wenyi Xie 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2215
Signed-off-by: Laszlo Ersek 
---
 SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 8 
+---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git 
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 377feebb205a..100739eb3eb6 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1855,10 +1855,12 @@ DxeImageVerificationHandler (
   for (OffSet = SecDataDir->VirtualAddress;
OffSet < SecDataDirEnd;
OffSet += (WinCertificate->dwLength + ALIGN_SIZE 
(WinCertificate->dwLength))) {
-WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
 SecDataDirLeft = SecDataDirEnd - OffSet;
-if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
-SecDataDirLeft < WinCertificate->dwLength) {
+if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) {
+  break;
+}
+WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
+if (SecDataDirLeft < WinCertificate->dwLength) {
   break;
 }
 
-- 
2.19.1.3.g30247aa5d201



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64883): https://edk2.groups.io/g/devel/message/64883
Mute This Topic: https://groups.io/mt/76552539/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Cancelled Event: TianoCore Bug Triage - APAC / NAMO - Tuesday, 1 September 2020 #cal-cancelled

[devel@edk2.groups.io Calendar]

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Groups.io Inc//Groups.io Calendar//EN
METHOD:CANCELLED
CALSCALE:GREGORIAN
BEGIN:VEVENT
UID:mlda.1580078539586725120.r...@groups.io
DTSTAMP:20200901T085154Z
ORGANIZER;CN=Brian Richardson:mailto:brian.richard...@intel.com
DTSTART:20200902T013000Z
DTEND:20200902T023000Z
SUMMARY:TianoCore Bug Triage - APAC / NAMO
DESCRIPTION:https://www.tianocore.org/bug-triage\n\nMeeting URL\n\nhttps:
 //bluejeans.com/889357567?src=join_info\n\nMeeting ID\n\n889 357 567\n\nW
 ant to dial in from a phone?\n\nDial one of the following numbers:\n\n+1.
 408.740.7256 (US (San Jose))\n\n+1.408.317.9253 (US (Primary\, San Jose))
 \n\n(see all numbers - https://www.bluejeans.com/numbers)\n\nEnter the me
 eting ID and passcode followed by #
LOCATION:https://bluejeans.com/889357567?src=join_info
SEQUENCE:999
STATUS:CANCELLED
END:VEVENT
END:VCALENDAR


invite.ics
Description: application/ics

回复: [edk2-devel] TianoCore Bug Triage - APAC / NAMO - Tue, 09/01/2020 6:30pm-7:30pm #cal-reminder

[gaoliming]

Hi, all

 There is only one new BZ with unassig...@tianocore.org 
 . So, I will cancel this week bug triage 
meeting. 


 

  2946

EDK2

Code

unassig...@tianocore.org

UNCO

BrotliCustomDecompressLib : Destination size and Scratch size are claimed too 
large  

21:28:13

miki.shi...@intel.com

 

Thanks

Liming

 

发件人: bounce+27952+64866+4905953+8761...@groups.io 
 代表 devel@edk2.groups.io Calendar
发送时间: 2020年9月1日 9:30
收件人: devel@edk2.groups.io
主题: [edk2-devel] TianoCore Bug Triage - APAC / NAMO - Tue, 09/01/2020 
6:30pm-7:30pm #cal-reminder

 

Reminder: TianoCore Bug Triage - APAC / NAMO

When: Tuesday, 1 September 2020, 6:30pm to 7:30pm, (GMT-07:00) America/Los 
Angeles 

Where:https://bluejeans.com/889357567?src=join_info

View Event  

Organizer: Brian Richardson brian.richard...@intel.com 


Description: 

https://www.tianocore.org/bug-triage

 

Meeting URL

https://bluejeans.com/889357567?src=join_info

 

Meeting ID

889 357 567

 

Want to dial in from a phone?

Dial one of the following numbers:

+1.408.740.7256 (US (San Jose))

+1.408.317.9253 (US (Primary, San Jose))

 

(see all numbers - https://www.bluejeans.com/numbers)

Enter the meeting ID and passcode followed by #




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64880): https://edk2.groups.io/g/devel/message/64880
Mute This Topic: https://groups.io/mt/76552403/21656
Mute #cal-reminder: https://groups.io/g/edk2/mutehashtag/cal-reminder
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch 1/1] BaseTools/Ecc: Fix an issue of path separator compatibility

[Laszlo Ersek]

On 08/31/20 18:14, Bob Feng wrote:
> From: "Bob Feng" 
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2904
> 
> The path separator is different in Windows and Linux, the
> original code does not handle this difference. This patch
> is to fix this issue.
> 
> Signed-off-by: Bob Feng 
> Cc: Liming Gao 
> Cc: Yuwei Chen 
> Cc: Shenglei Zhang 
> ---
>  BaseTools/Source/Python/Ecc/Check.py | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/Ecc/Check.py 
> b/BaseTools/Source/Python/Ecc/Check.py
> index 0fdc7e35c18a..ca24c8acffeb 100644
> --- a/BaseTools/Source/Python/Ecc/Check.py
> +++ b/BaseTools/Source/Python/Ecc/Check.py
> @@ -13,10 +13,11 @@ from Ecc.EccToolError import *
>  from Ecc.MetaDataParser import ParseHeaderCommentSection
>  from Ecc import EccGlobalData
>  from Ecc import c
>  from Common.LongFilePathSupport import OpenLongFilePath as open
>  from Common.MultipleWorkspace import MultipleWorkspace as mws
> +import platform
>  
>  ## Check
>  #
>  # This class is to define checkpoints used by ECC tool
>  #
> @@ -1099,17 +1100,18 @@ class Check(object):
>  InfPathSet = EccGlobalData.gDb.TblInf.Exec(SqlCommand)
>  InfPathList = []
>  for Item in InfPathSet:
>  if Item[0] not in InfPathList:
>  InfPathList.append(Item[0])
> +pathsep = """'\\'""" if platform.system() == 'Windows' else 
> """'/'"""
>  SqlCommand = """
>   select ID, Path, FullPath from File where 
> upper(FullPath) not in
> -(select upper(A.Path) || '\\' || upper(B.Value1) 
> from File as A, INF as B
> +(select upper(A.Path) || %s || upper(B.Value1) 
> from File as A, INF as B
>  where A.ID in (select BelongsToFile from INF 
> where Model = %s group by BelongsToFile) and
>  B.BelongsToFile = A.ID and B.Model = %s)
>  and (Model = %s or Model = %s)
> -""" % (MODEL_EFI_SOURCE_FILE, MODEL_EFI_SOURCE_FILE, 
> MODEL_FILE_C, MODEL_FILE_H)
> +""" % (pathsep, MODEL_EFI_SOURCE_FILE, 
> MODEL_EFI_SOURCE_FILE, MODEL_FILE_C, MODEL_FILE_H)
>  RecordSet = EccGlobalData.gDb.TblInf.Exec(SqlCommand)
>  for Record in RecordSet:
>  Path = Record[1]
>  Path = Path.upper().replace('\X64', '').replace('\IA32', 
> '').replace('\EBC', '').replace('\IPF', '').replace('\ARM', '')
>  if Path in InfPathList:
> @@ -1122,21 +1124,22 @@ class Check(object):
>  EdkLogger.quiet("Checking for pcd type in c code function usage 
> ...")
>  SqlCommand = """
>   select ID, Model, Value1, Value2, BelongsToFile 
> from INF where Model > %s and Model < %s
>   """ % (MODEL_PCD, MODEL_META_DATA_HEADER)
>  PcdSet = EccGlobalData.gDb.TblInf.Exec(SqlCommand)
> +pathsep = """'\\'""" if platform.system() == 'Windows' else 
> """'/'"""
>  for Pcd in PcdSet:
>  Model = Pcd[1]
>  PcdName = Pcd[2]
>  if Pcd[3]:
>  PcdName = Pcd[3]
>  BelongsToFile = Pcd[4]
>  SqlCommand = """
>   select ID from File where FullPath in
> -(select B.Path || '\\' || A.Value1 from INF as 
> A, File as B where A.Model = %s and A.BelongsToFile = %s
> +(select B.Path || %s || A.Value1 from INF as A, 
> File as B where A.Model = %s and A.BelongsToFile = %s
>   and B.ID = %s and (B.Model = %s or B.Model = 
> %s))
> - """ % (MODEL_EFI_SOURCE_FILE, BelongsToFile, 
> BelongsToFile, MODEL_FILE_C, MODEL_FILE_H)
> + """ % (pathsep, MODEL_EFI_SOURCE_FILE, 
> BelongsToFile, BelongsToFile, MODEL_FILE_C, MODEL_FILE_H)
>  TableSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)
>  for Tbl in TableSet:
>  TblName = 'Identifier' + str(Tbl[0])
>  SqlCommand = """
>   select Name, ID from %s where value like 
> '%s' and Model = %s
> 

(1) any reason for not using os.sep (aka os.path.sep) instead?

(2) do you intend this patch for the stable tag?

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64879): https://edk2.groups.io/g/devel/message/64879
Mute This Topic: https://groups.io/mt/76537216/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3] MdeModulePkg/Library: add PEIM and SEC module type to TpmMeasurementLibNull

[Laszlo Ersek]

On 09/01/20 09:26, Qi Zhang wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2940
> 
> Signed-off-by: Qi Zhang 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Jiewen Yao 
> Reviewed-by: Laszlo Ersek 
> ---
>  .../Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git 
> a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf 
> b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
> index 61abcfa2ec..c3be447d40 100644
> --- a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
> +++ b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
> @@ -12,7 +12,7 @@
>FILE_GUID  = 6DFD6E9F-9278-48D8-8F45-B6CFF2C2B69C
>MODULE_TYPE= UEFI_DRIVER
>VERSION_STRING = 1.0
> -  LIBRARY_CLASS  = TpmMeasurementLib|DXE_DRIVER 
> DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
> +  LIBRARY_CLASS  = TpmMeasurementLib|SEC PEIM DXE_DRIVER 
> DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
>MODULE_UNI_FILE= TpmMeasurementLibNull.uni
>  
>  #
> 

Reviewed-by: Laszlo Ersek 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64878): https://edk2.groups.io/g/devel/message/64878
Mute This Topic: https://groups.io/mt/76551779/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

[Laszlo Ersek]

On 09/01/20 09:43, xiewenyi (A) wrote:
> To Jiewen,
> 
> Sorry to make you lost，I mean the patches Laszlo proposed in email，
> https://edk2.groups.io/g/devel/message/64243
> eb0c6bcb-77fb-2fb9-783e-aa5025953a80@redhat.com">http://mid.mail-archive.com/eb0c6bcb-77fb-2fb9-783e-aa5025953a80@redhat.com
> 
> To Laszlo,
> 
> Sure, I will test your patches against my reproducer and add "tested-by:" tag 
> to the patches after you post them.

Thank you!, I'll send them soon.

Cheers!
Laszlo

> 
> Regards
> Wenyi
> 
> On 2020/9/1 15:31, Yao, Jiewen wrote:
>> I am sorry, that I am a little lost here.
>>
>> We have discussed different patches. I am not 100% sure which one is 
>> "Laszlo's patches".
>>
>> To make thing easy and record all actions, would you please reply the 
>> patch(es) you have verified, with your "tested-by:" tag?
>>
>> Thank you
>> Yao Jiewen
>>
>>> -Original Message-
>>> From: devel@edk2.groups.io  On Behalf Of wenyi,xie
>>> via groups.io
>>> Sent: Tuesday, September 1, 2020 3:11 PM
>>> To: Yao, Jiewen ; devel@edk2.groups.io; Laszlo Ersek
>>> ; Wang, Jian J 
>>> Cc: songdongku...@huawei.com; Mathews, John 
>>> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
>>> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
>>>
>>> I think Laszlo's patches is OK, I have applied and tested it using my case. 
>>> It can
>>> catch the issue.
>>> DEBUG code and log below,
>>>
>>>   SecDataDirEnd = SecDataDir->VirtualAddress + SecDataDir->Size;
>>>   DEBUG((DEBUG_INFO, "SecDataDirEnd=0x%x.\n", SecDataDirEnd));
>>>   for (OffSet = SecDataDir->VirtualAddress;
>>>OffSet < SecDataDirEnd;
>>>OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
 dwLength))) {
>>> SecDataDirLeft = SecDataDirEnd - OffSet;
>>> DEBUG((DEBUG_INFO, "OffSet=0x%x, SecDataDirLeft=0x%x.\n", OffSet,
>>> SecDataDirLeft));
>>> if (SecDataDirLeft <= sizeof(WIN_CERTIFICATE)) {
>>>   break;
>>> }
>>> WinCertificate = (WIN_CERTIFICATE *)(mImageBase + OffSet);
>>> DEBUG((DEBUG_INFO, "WinCertificate->dwLength=0x%x, ALIGN_SIZE
>>> (WinCertificate->dwLength)=0x%x.\n", WinCertificate->dwLength,
>>> ALIGN_SIZE(WinCertificate->dwLength)));
>>> if (SecDataDirLeft < WinCertificate->dwLength ||
>>> (SecDataDirLeft - WinCertificate->dwLength <
>>> ALIGN_SIZE(WinCertificate->dwLength))) {
>>>   DEBUG((DEBUG_INFO, "Parameter is invalid and break.\n"));
>>>   break;
>>> }
>>>
>>> SecDataDirEnd=0xFFFC.
>>> OffSet=0xE000, SecDataDirLeft=0x1FFC.
>>> WinCertificate->dwLength=0x1FFB, ALIGN_SIZE (WinCertificate-
 dwLength)=0x5.
>>> Parameter is invalid and break.
>>> The image doesn't pass verification: VenHw(5CF32E0B-8EDF-2E44-9CDA-
>>> 93205E99EC1C,)/VenHw(964E5B22-6459-11D2-8E39-
>>> 00A0C969723B,)/\signed_1234_4G.efi
>>>
>>> Regards
>>> Wenyi
>>>
>>> On 2020/9/1 0:06, Yao, Jiewen wrote:
 Sounds great. Appreciate your hard work on that.

 Will you post a patch to fix the issue again?

 Thank you
 Yao Jiewen

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of
>>> wenyi,xie
> via groups.io
> Sent: Monday, August 31, 2020 7:24 PM
> To: Yao, Jiewen ; devel@edk2.groups.io; Laszlo
>>> Ersek
> ; Wang, Jian J 
> Cc: songdongku...@huawei.com; Mathews, John
>>> 
> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
>
> Hi，Jiewen，
>
> I modify the PE file again, this time it can pass the check in PeCoffLib 
> and
>>> cause
> offset overflow.
>
> First, create a PE file and sign it(only one signature), then using 
> binary edit
>>> tool
> to modify content of PE file like below,
>  1.check the value of SecDataDir->VirtualAddress, in my PE file, it's 
> 0xE000
>  2.changing SecDataDir->Size from 0x5F8 to 0x1FFC
>  3.changing WinCertificate->dwLength from 0x5F1 to 0x1FFB
>  4.padding PE file with 0 until the size of the file is 0xFFFC(it 
> will make the
>>> PE
> file so large)
> OffSet + WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength)
>>> is
> 0xE000 + 0x1FFB + 0x5 = 0x1
>
> Below is the DEBUG code and log, in second loop the offset overflow and
> become 0
>
> for (OffSet = SecDataDir->VirtualAddress;
>  OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
>  OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
>> dwLength))) {
>   WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
>   DEBUG((DEBUG_INFO, "OffSet=0x%x.\n", OffSet));
>   if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof
> (WIN_CERTIFICATE) ||
>   (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < 
> WinCertificate-
>> dwLength) {
> break;
>   }
>   DEBUG((DEBUG_INFO, 

Re: [edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

[wenyi,xie via groups.io]

To Jiewen,

Sorry to make you lost，I mean the patches Laszlo proposed in email，
https://edk2.groups.io/g/devel/message/64243
eb0c6bcb-77fb-2fb9-783e-aa5025953a80@redhat.com">http://mid.mail-archive.com/eb0c6bcb-77fb-2fb9-783e-aa5025953a80@redhat.com

To Laszlo,

Sure, I will test your patches against my reproducer and add "tested-by:" tag 
to the patches after you post them.

Regards
Wenyi

On 2020/9/1 15:31, Yao, Jiewen wrote:
> I am sorry, that I am a little lost here.
> 
> We have discussed different patches. I am not 100% sure which one is 
> "Laszlo's patches".
> 
> To make thing easy and record all actions, would you please reply the 
> patch(es) you have verified, with your "tested-by:" tag?
> 
> Thank you
> Yao Jiewen
> 
>> -Original Message-
>> From: devel@edk2.groups.io  On Behalf Of wenyi,xie
>> via groups.io
>> Sent: Tuesday, September 1, 2020 3:11 PM
>> To: Yao, Jiewen ; devel@edk2.groups.io; Laszlo Ersek
>> ; Wang, Jian J 
>> Cc: songdongku...@huawei.com; Mathews, John 
>> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
>> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
>>
>> I think Laszlo's patches is OK, I have applied and tested it using my case. 
>> It can
>> catch the issue.
>> DEBUG code and log below,
>>
>>   SecDataDirEnd = SecDataDir->VirtualAddress + SecDataDir->Size;
>>   DEBUG((DEBUG_INFO, "SecDataDirEnd=0x%x.\n", SecDataDirEnd));
>>   for (OffSet = SecDataDir->VirtualAddress;
>>OffSet < SecDataDirEnd;
>>OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
>>> dwLength))) {
>> SecDataDirLeft = SecDataDirEnd - OffSet;
>> DEBUG((DEBUG_INFO, "OffSet=0x%x, SecDataDirLeft=0x%x.\n", OffSet,
>> SecDataDirLeft));
>> if (SecDataDirLeft <= sizeof(WIN_CERTIFICATE)) {
>>   break;
>> }
>> WinCertificate = (WIN_CERTIFICATE *)(mImageBase + OffSet);
>> DEBUG((DEBUG_INFO, "WinCertificate->dwLength=0x%x, ALIGN_SIZE
>> (WinCertificate->dwLength)=0x%x.\n", WinCertificate->dwLength,
>> ALIGN_SIZE(WinCertificate->dwLength)));
>> if (SecDataDirLeft < WinCertificate->dwLength ||
>>  (SecDataDirLeft - WinCertificate->dwLength <
>> ALIGN_SIZE(WinCertificate->dwLength))) {
>>   DEBUG((DEBUG_INFO, "Parameter is invalid and break.\n"));
>>   break;
>> }
>>
>> SecDataDirEnd=0xFFFC.
>> OffSet=0xE000, SecDataDirLeft=0x1FFC.
>> WinCertificate->dwLength=0x1FFB, ALIGN_SIZE (WinCertificate-
>>> dwLength)=0x5.
>> Parameter is invalid and break.
>> The image doesn't pass verification: VenHw(5CF32E0B-8EDF-2E44-9CDA-
>> 93205E99EC1C,)/VenHw(964E5B22-6459-11D2-8E39-
>> 00A0C969723B,)/\signed_1234_4G.efi
>>
>> Regards
>> Wenyi
>>
>> On 2020/9/1 0:06, Yao, Jiewen wrote:
>>> Sounds great. Appreciate your hard work on that.
>>>
>>> Will you post a patch to fix the issue again?
>>>
>>> Thank you
>>> Yao Jiewen
>>>
 -Original Message-
 From: devel@edk2.groups.io  On Behalf Of
>> wenyi,xie
 via groups.io
 Sent: Monday, August 31, 2020 7:24 PM
 To: Yao, Jiewen ; devel@edk2.groups.io; Laszlo
>> Ersek
 ; Wang, Jian J 
 Cc: songdongku...@huawei.com; Mathews, John
>> 
 Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
 SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

 Hi，Jiewen，

 I modify the PE file again, this time it can pass the check in PeCoffLib 
 and
>> cause
 offset overflow.

 First, create a PE file and sign it(only one signature), then using binary 
 edit
>> tool
 to modify content of PE file like below,
  1.check the value of SecDataDir->VirtualAddress, in my PE file, it's 
 0xE000
  2.changing SecDataDir->Size from 0x5F8 to 0x1FFC
  3.changing WinCertificate->dwLength from 0x5F1 to 0x1FFB
  4.padding PE file with 0 until the size of the file is 0xFFFC(it will 
 make the
>> PE
 file so large)
 OffSet + WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength)
>> is
 0xE000 + 0x1FFB + 0x5 = 0x1

 Below is the DEBUG code and log, in second loop the offset overflow and
 become 0

 for (OffSet = SecDataDir->VirtualAddress;
  OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
  OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
> dwLength))) {
   WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
   DEBUG((DEBUG_INFO, "OffSet=0x%x.\n", OffSet));
   if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof
 (WIN_CERTIFICATE) ||
   (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < 
 WinCertificate-
> dwLength) {
 break;
   }
   DEBUG((DEBUG_INFO, "WinCertificate->dwLength=0x%x, ALIGN_SIZE
 (WinCertificate->dwLength)=0x%x.\n", WinCertificate->dwLength,
 ALIGN_SIZE(WinCertificate->dwLength)));


 SecDataDir->VirtualAddress=0xE000, SecDataDir->Size=0x1FFC.
 

Re: [edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

[Yao, Jiewen]

I am sorry, that I am a little lost here.

We have discussed different patches. I am not 100% sure which one is "Laszlo's 
patches".

To make thing easy and record all actions, would you please reply the patch(es) 
you have verified, with your "tested-by:" tag?

Thank you
Yao Jiewen

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of wenyi,xie
> via groups.io
> Sent: Tuesday, September 1, 2020 3:11 PM
> To: Yao, Jiewen ; devel@edk2.groups.io; Laszlo Ersek
> ; Wang, Jian J 
> Cc: songdongku...@huawei.com; Mathews, John 
> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
> 
> I think Laszlo's patches is OK, I have applied and tested it using my case. 
> It can
> catch the issue.
> DEBUG code and log below,
> 
>   SecDataDirEnd = SecDataDir->VirtualAddress + SecDataDir->Size;
>   DEBUG((DEBUG_INFO, "SecDataDirEnd=0x%x.\n", SecDataDirEnd));
>   for (OffSet = SecDataDir->VirtualAddress;
>OffSet < SecDataDirEnd;
>OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
> >dwLength))) {
> SecDataDirLeft = SecDataDirEnd - OffSet;
> DEBUG((DEBUG_INFO, "OffSet=0x%x, SecDataDirLeft=0x%x.\n", OffSet,
> SecDataDirLeft));
> if (SecDataDirLeft <= sizeof(WIN_CERTIFICATE)) {
>   break;
> }
> WinCertificate = (WIN_CERTIFICATE *)(mImageBase + OffSet);
> DEBUG((DEBUG_INFO, "WinCertificate->dwLength=0x%x, ALIGN_SIZE
> (WinCertificate->dwLength)=0x%x.\n", WinCertificate->dwLength,
> ALIGN_SIZE(WinCertificate->dwLength)));
> if (SecDataDirLeft < WinCertificate->dwLength ||
>   (SecDataDirLeft - WinCertificate->dwLength <
> ALIGN_SIZE(WinCertificate->dwLength))) {
>   DEBUG((DEBUG_INFO, "Parameter is invalid and break.\n"));
>   break;
> }
> 
> SecDataDirEnd=0xFFFC.
> OffSet=0xE000, SecDataDirLeft=0x1FFC.
> WinCertificate->dwLength=0x1FFB, ALIGN_SIZE (WinCertificate-
> >dwLength)=0x5.
> Parameter is invalid and break.
> The image doesn't pass verification: VenHw(5CF32E0B-8EDF-2E44-9CDA-
> 93205E99EC1C,)/VenHw(964E5B22-6459-11D2-8E39-
> 00A0C969723B,)/\signed_1234_4G.efi
> 
> Regards
> Wenyi
> 
> On 2020/9/1 0:06, Yao, Jiewen wrote:
> > Sounds great. Appreciate your hard work on that.
> >
> > Will you post a patch to fix the issue again?
> >
> > Thank you
> > Yao Jiewen
> >
> >> -Original Message-
> >> From: devel@edk2.groups.io  On Behalf Of
> wenyi,xie
> >> via groups.io
> >> Sent: Monday, August 31, 2020 7:24 PM
> >> To: Yao, Jiewen ; devel@edk2.groups.io; Laszlo
> Ersek
> >> ; Wang, Jian J 
> >> Cc: songdongku...@huawei.com; Mathews, John
> 
> >> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
> >> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
> >>
> >> Hi，Jiewen，
> >>
> >> I modify the PE file again, this time it can pass the check in PeCoffLib 
> >> and
> cause
> >> offset overflow.
> >>
> >> First, create a PE file and sign it(only one signature), then using binary 
> >> edit
> tool
> >> to modify content of PE file like below,
> >>  1.check the value of SecDataDir->VirtualAddress, in my PE file, it's 
> >> 0xE000
> >>  2.changing SecDataDir->Size from 0x5F8 to 0x1FFC
> >>  3.changing WinCertificate->dwLength from 0x5F1 to 0x1FFB
> >>  4.padding PE file with 0 until the size of the file is 0xFFFC(it will 
> >> make the
> PE
> >> file so large)
> >> OffSet + WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength)
> is
> >> 0xE000 + 0x1FFB + 0x5 = 0x1
> >>
> >> Below is the DEBUG code and log, in second loop the offset overflow and
> >> become 0
> >>
> >> for (OffSet = SecDataDir->VirtualAddress;
> >>  OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
> >>  OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
> >>> dwLength))) {
> >>   WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
> >>   DEBUG((DEBUG_INFO, "OffSet=0x%x.\n", OffSet));
> >>   if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof
> >> (WIN_CERTIFICATE) ||
> >>   (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < 
> >> WinCertificate-
> >>> dwLength) {
> >> break;
> >>   }
> >>   DEBUG((DEBUG_INFO, "WinCertificate->dwLength=0x%x, ALIGN_SIZE
> >> (WinCertificate->dwLength)=0x%x.\n", WinCertificate->dwLength,
> >> ALIGN_SIZE(WinCertificate->dwLength)));
> >>
> >>
> >> SecDataDir->VirtualAddress=0xE000, SecDataDir->Size=0x1FFC.
> >> OffSet=0xE000.
> >> WinCertificate->dwLength=0x1FFB, ALIGN_SIZE (WinCertificate-
> >>> dwLength)=0x5.
> >> DxeImageVerificationLib: Image is signed but signature is not allowed by DB
> and
> >> SHA256 hash of image is notOffSet=0x0.
> >> WinCertificate->dwLength=0x5A4D, ALIGN_SIZE (WinCertificate-
> >>> dwLength)=0x3.
> >> OffSet=0x5A50.
> >> WinCertificate->dwLength=0x9024, ALIGN_SIZE (WinCertificate-
> >>> dwLength)=0x4.
> >> OffSet=0xEA78.
> >> WinCertificate->dwLength=0x0, ALIGN_SIZE 

Re: [edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

[Laszlo Ersek]

On 08/31/20 18:06, Yao, Jiewen wrote:
> Sounds great. Appreciate your hard work on that.
> 
> Will you post a patch to fix the issue again?

Should I post the three patches that I had counter-proposed as a
standalone series?

See them at the end of the following message, in-line:

https://edk2.groups.io/g/devel/message/64243
eb0c6bcb-77fb-2fb9-783e-aa5025953a80@redhat.com">http://mid.mail-archive.com/eb0c6bcb-77fb-2fb9-783e-aa5025953a80@redhat.com

Wenyi, if you were OK with my patches, then I'd ask you to test them
against your reproducer (if / when I post the patches).

Thanks,
Laszlo

>> -Original Message-
>> From: devel@edk2.groups.io  On Behalf Of wenyi,xie
>> via groups.io
>> Sent: Monday, August 31, 2020 7:24 PM
>> To: Yao, Jiewen ; devel@edk2.groups.io; Laszlo Ersek
>> ; Wang, Jian J 
>> Cc: songdongku...@huawei.com; Mathews, John 
>> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
>> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
>>
>> Hi，Jiewen，
>>
>> I modify the PE file again, this time it can pass the check in PeCoffLib and 
>> cause
>> offset overflow.
>>
>> First, create a PE file and sign it(only one signature), then using binary 
>> edit tool
>> to modify content of PE file like below,
>>  1.check the value of SecDataDir->VirtualAddress, in my PE file, it's 0xE000
>>  2.changing SecDataDir->Size from 0x5F8 to 0x1FFC
>>  3.changing WinCertificate->dwLength from 0x5F1 to 0x1FFB
>>  4.padding PE file with 0 until the size of the file is 0xFFFC(it will 
>> make the PE
>> file so large)
>> OffSet + WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength) is
>> 0xE000 + 0x1FFB + 0x5 = 0x1
>>
>> Below is the DEBUG code and log, in second loop the offset overflow and
>> become 0
>>
>> for (OffSet = SecDataDir->VirtualAddress;
>>  OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
>>  OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
>>> dwLength))) {
>>   WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
>>   DEBUG((DEBUG_INFO, "OffSet=0x%x.\n", OffSet));
>>   if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof
>> (WIN_CERTIFICATE) ||
>>   (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < 
>> WinCertificate-
>>> dwLength) {
>> break;
>>   }
>>   DEBUG((DEBUG_INFO, "WinCertificate->dwLength=0x%x, ALIGN_SIZE
>> (WinCertificate->dwLength)=0x%x.\n", WinCertificate->dwLength,
>> ALIGN_SIZE(WinCertificate->dwLength)));
>>
>>
>> SecDataDir->VirtualAddress=0xE000, SecDataDir->Size=0x1FFC.
>> OffSet=0xE000.
>> WinCertificate->dwLength=0x1FFB, ALIGN_SIZE (WinCertificate-
>>> dwLength)=0x5.
>> DxeImageVerificationLib: Image is signed but signature is not allowed by DB 
>> and
>> SHA256 hash of image is notOffSet=0x0.
>> WinCertificate->dwLength=0x5A4D, ALIGN_SIZE (WinCertificate-
>>> dwLength)=0x3.
>> OffSet=0x5A50.
>> WinCertificate->dwLength=0x9024, ALIGN_SIZE (WinCertificate-
>>> dwLength)=0x4.
>> OffSet=0xEA78.
>> WinCertificate->dwLength=0x0, ALIGN_SIZE (WinCertificate->dwLength)=0x0.
>> The image doesn't pass verification: VenHw(5CF32E0B-8EDF-2E44-9CDA-
>> 93205E99EC1C,)/VenHw(964E5B22-6459-11D2-8E39-
>> 00A0C969723B,)/\signed_1234_4G.efi
>>
>>
>> Regards
>> Wenyi
>>
>>
>> On 2020/8/28 14:43, Yao, Jiewen wrote:
>>> Apology that I did not say clearly.
>>> I mean you should not modify any code to perform an attack.
>>>
>>> I am not asking you to exploit the system. Attack here just means: to cause
>> system hang or buffer overflow. That is enough.
>>> But you cannot modify code to remove any existing checker.
>>>
>>> Thank you
>>> Yao Jiewen
>>>
 -Original Message-
 From: devel@edk2.groups.io  On Behalf Of
>> wenyi,xie
 via groups.io
 Sent: Friday, August 28, 2020 2:18 PM
 To: Yao, Jiewen ; devel@edk2.groups.io; Laszlo
>> Ersek
 ; Wang, Jian J 
 Cc: songdongku...@huawei.com; Mathews, John
>> 
 Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
 SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

 Hi，Jiewen，

 I don't really get the meaning "create a case that bypass all checks in
>> PeCoffLib",
 do you mean I need to create a PE file that can bypass all check in 
 PeCoffLib
 without modify any
 code and then cause the problem in DxeImageVerificationLib, or just modify
 some code in PeCoffLib to bypass check instead of removing the calling of
 PeCoffLoaderGetImageInfo. Would
 you mind explaining a little more specifically? As far as I tried, it's 
 really hard
>> to
 reproduce the issue without touching any code.

 Thanks
 Wenyi

 On 2020/8/28 11:50, Yao, Jiewen wrote:
> HI Wenyi
> Thank you very much to take time to reproduce.
>
> I am particular interested in below:
>   "As PE file is modified, function PeCoffLoaderGetImageInfo will return
 error, so I have to remove it so that for loop can be 

[edk2-devel] [PATCH v3] MdeModulePkg/Library: add PEIM and SEC module type to TpmMeasurementLibNull

[Qi Zhang]

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

Signed-off-by: Qi Zhang 
Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Jiewen Yao 
Reviewed-by: Laszlo Ersek 
---
 .../Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf 
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
index 61abcfa2ec..c3be447d40 100644
--- a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
+++ b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
@@ -12,7 +12,7 @@
   FILE_GUID  = 6DFD6E9F-9278-48D8-8F45-B6CFF2C2B69C
   MODULE_TYPE= UEFI_DRIVER
   VERSION_STRING = 1.0
-  LIBRARY_CLASS  = TpmMeasurementLib|DXE_DRIVER 
DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+  LIBRARY_CLASS  = TpmMeasurementLib|SEC PEIM DXE_DRIVER 
DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
   MODULE_UNI_FILE= TpmMeasurementLibNull.uni
 
 #
-- 
2.26.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64873): https://edk2.groups.io/g/devel/message/64873
Mute This Topic: https://groups.io/mt/76551779/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] SecurityPkg: Initailize variable Status before it is consumed.

[Zhiguang Liu]

Hi Liming,
Can this patch check in before this stable tag?

Thanks
Zhiguang

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Zhiguang
> Liu
> Sent: Tuesday, September 1, 2020 8:55 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen ; Wang, Jian J ;
> Zhang, Qi1 ; Kumar, Rahul1 ;
> Laszlo Ersek 
> Subject: [edk2-devel] [PATCH] SecurityPkg: Initailize variable Status before 
> it is
> consumed.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2945
> 
> V2: Move "Status = EFI_SUCCESS;" before the EDKII_TCG_PRE_HASH check.
> 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Qi Zhang 
> Cc: Rahul Kumar 
> Cc: Laszlo Ersek 
> Reviewed-by: Jiewen Yao 
> Signed-off-by: Zhiguang Liu 
> ---
>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> index 0e770f4485..93a8803ff6 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> @@ -456,6 +456,7 @@ HashLogExtendEvent (
>if ((Flags & EDKII_TCG_PRE_HASH) != 0 || (Flags &
> EDKII_TCG_PRE_HASH_LOG_ONLY) != 0) {
> 
>  ZeroMem (, sizeof(DigestList));
> 
>  CopyMem (, HashData, sizeof(DigestList));
> 
> +Status = EFI_SUCCESS;
> 
>  if ((Flags & EDKII_TCG_PRE_HASH) !=0 ) {
> 
>Status = Tpm2PcrExtend (
> 
> NewEventHdr->PCRIndex,
> 
> --
> 2.25.1.windows.1
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#64865): https://edk2.groups.io/g/devel/message/64865
> Mute This Topic: https://groups.io/mt/76530112/1779286
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [zhiguang@intel.com]
> -=-=-=-=-=-=


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64872): https://edk2.groups.io/g/devel/message/64872
Mute This Topic: https://groups.io/mt/76530112/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH EDK2 v2 1/1] SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

[wenyi,xie via groups.io]

I think Laszlo's patches is OK, I have applied and tested it using my case. It 
can catch the issue.
DEBUG code and log below,

  SecDataDirEnd = SecDataDir->VirtualAddress + SecDataDir->Size;
  DEBUG((DEBUG_INFO, "SecDataDirEnd=0x%x.\n", SecDataDirEnd));
  for (OffSet = SecDataDir->VirtualAddress;
   OffSet < SecDataDirEnd;
   OffSet += (WinCertificate->dwLength + ALIGN_SIZE 
(WinCertificate->dwLength))) {
SecDataDirLeft = SecDataDirEnd - OffSet;
DEBUG((DEBUG_INFO, "OffSet=0x%x, SecDataDirLeft=0x%x.\n", OffSet, 
SecDataDirLeft));
if (SecDataDirLeft <= sizeof(WIN_CERTIFICATE)) {
  break;
}
WinCertificate = (WIN_CERTIFICATE *)(mImageBase + OffSet);
DEBUG((DEBUG_INFO, "WinCertificate->dwLength=0x%x, ALIGN_SIZE 
(WinCertificate->dwLength)=0x%x.\n", WinCertificate->dwLength, 
ALIGN_SIZE(WinCertificate->dwLength)));
if (SecDataDirLeft < WinCertificate->dwLength ||
(SecDataDirLeft - WinCertificate->dwLength < 
ALIGN_SIZE(WinCertificate->dwLength))) {
  DEBUG((DEBUG_INFO, "Parameter is invalid and break.\n"));
  break;
}

SecDataDirEnd=0xFFFC.
OffSet=0xE000, SecDataDirLeft=0x1FFC.
WinCertificate->dwLength=0x1FFB, ALIGN_SIZE (WinCertificate->dwLength)=0x5.
Parameter is invalid and break.
The image doesn't pass verification: 
VenHw(5CF32E0B-8EDF-2E44-9CDA-93205E99EC1C,)/VenHw(964E5B22-6459-11D2-8E39-00A0C969723B,)/\signed_1234_4G.efi

Regards
Wenyi

On 2020/9/1 0:06, Yao, Jiewen wrote:
> Sounds great. Appreciate your hard work on that.
> 
> Will you post a patch to fix the issue again?
> 
> Thank you
> Yao Jiewen
> 
>> -Original Message-
>> From: devel@edk2.groups.io  On Behalf Of wenyi,xie
>> via groups.io
>> Sent: Monday, August 31, 2020 7:24 PM
>> To: Yao, Jiewen ; devel@edk2.groups.io; Laszlo Ersek
>> ; Wang, Jian J 
>> Cc: songdongku...@huawei.com; Mathews, John 
>> Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
>> SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset
>>
>> Hi，Jiewen，
>>
>> I modify the PE file again, this time it can pass the check in PeCoffLib and 
>> cause
>> offset overflow.
>>
>> First, create a PE file and sign it(only one signature), then using binary 
>> edit tool
>> to modify content of PE file like below,
>>  1.check the value of SecDataDir->VirtualAddress, in my PE file, it's 0xE000
>>  2.changing SecDataDir->Size from 0x5F8 to 0x1FFC
>>  3.changing WinCertificate->dwLength from 0x5F1 to 0x1FFB
>>  4.padding PE file with 0 until the size of the file is 0xFFFC(it will 
>> make the PE
>> file so large)
>> OffSet + WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength) is
>> 0xE000 + 0x1FFB + 0x5 = 0x1
>>
>> Below is the DEBUG code and log, in second loop the offset overflow and
>> become 0
>>
>> for (OffSet = SecDataDir->VirtualAddress;
>>  OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
>>  OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
>>> dwLength))) {
>>   WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
>>   DEBUG((DEBUG_INFO, "OffSet=0x%x.\n", OffSet));
>>   if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof
>> (WIN_CERTIFICATE) ||
>>   (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < 
>> WinCertificate-
>>> dwLength) {
>> break;
>>   }
>>   DEBUG((DEBUG_INFO, "WinCertificate->dwLength=0x%x, ALIGN_SIZE
>> (WinCertificate->dwLength)=0x%x.\n", WinCertificate->dwLength,
>> ALIGN_SIZE(WinCertificate->dwLength)));
>>
>>
>> SecDataDir->VirtualAddress=0xE000, SecDataDir->Size=0x1FFC.
>> OffSet=0xE000.
>> WinCertificate->dwLength=0x1FFB, ALIGN_SIZE (WinCertificate-
>>> dwLength)=0x5.
>> DxeImageVerificationLib: Image is signed but signature is not allowed by DB 
>> and
>> SHA256 hash of image is notOffSet=0x0.
>> WinCertificate->dwLength=0x5A4D, ALIGN_SIZE (WinCertificate-
>>> dwLength)=0x3.
>> OffSet=0x5A50.
>> WinCertificate->dwLength=0x9024, ALIGN_SIZE (WinCertificate-
>>> dwLength)=0x4.
>> OffSet=0xEA78.
>> WinCertificate->dwLength=0x0, ALIGN_SIZE (WinCertificate->dwLength)=0x0.
>> The image doesn't pass verification: VenHw(5CF32E0B-8EDF-2E44-9CDA-
>> 93205E99EC1C,)/VenHw(964E5B22-6459-11D2-8E39-
>> 00A0C969723B,)/\signed_1234_4G.efi
>>
>>
>> Regards
>> Wenyi
>>
>>
>> On 2020/8/28 14:43, Yao, Jiewen wrote:
>>> Apology that I did not say clearly.
>>> I mean you should not modify any code to perform an attack.
>>>
>>> I am not asking you to exploit the system. Attack here just means: to cause
>> system hang or buffer overflow. That is enough.
>>> But you cannot modify code to remove any existing checker.
>>>
>>> Thank you
>>> Yao Jiewen
>>>
 -Original Message-
 From: devel@edk2.groups.io  On Behalf Of
>> wenyi,xie
 via groups.io
 Sent: Friday, August 28, 2020 2:18 PM
 To: Yao, Jiewen ; devel@edk2.groups.io; Laszlo
>> Ersek
 ; Wang, Jian J 
 Cc: songdongku...@huawei.com; Mathews, John
>> 
 

Re: [edk2-devel] [PATCH v2] MdeModulePkg/Library: add PEIM and SEC module type to TpmMeasurementLibNull

[Laszlo Ersek]

On 09/01/20 03:35, Qi Zhang wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2940
> 
> Signed-off-by: Qi Zhang 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Jiewen Yao 
> ---
>  .../Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git 
> a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf 
> b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
> index 61abcfa2ec..327d80f319 100644
> --- a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
> +++ b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
> @@ -12,7 +12,7 @@
>FILE_GUID  = 6DFD6E9F-9278-48D8-8F45-B6CFF2C2B69C
>MODULE_TYPE= UEFI_DRIVER
>VERSION_STRING = 1.0
> -  LIBRARY_CLASS  = TpmMeasurementLib|DXE_DRIVER 
> DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
> +  LIBRARY_CLASS  = TpmMeasurementLib|DXE_DRIVER 
> DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER PEIM SEC
>MODULE_UNI_FILE= TpmMeasurementLibNull.uni
>  
>  #
> 

It tends to improve readability if we keep the "earlier" / "more
primitive" module types to the left, such as:

  SEC, PEIM, DXE_*, UEFI_*

But I don't want to obsess about this:

Reviewed-by: Laszlo Ersek 

Thanks,
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64870): https://edk2.groups.io/g/devel/message/64870
Mute This Topic: https://groups.io/mt/76548148/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] SecurityPkg: Initailize variable Status before it is consumed.

[Laszlo Ersek]

On 09/01/20 02:55, Zhiguang Liu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2945
> 
> V2: Move "Status = EFI_SUCCESS;" before the EDKII_TCG_PRE_HASH check.
> 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Qi Zhang 
> Cc: Rahul Kumar 
> Cc: Laszlo Ersek 
> Reviewed-by: Jiewen Yao 
> Signed-off-by: Zhiguang Liu 
> ---
>  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c 
> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> index 0e770f4485..93a8803ff6 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
> @@ -456,6 +456,7 @@ HashLogExtendEvent (
>if ((Flags & EDKII_TCG_PRE_HASH) != 0 || (Flags & 
> EDKII_TCG_PRE_HASH_LOG_ONLY) != 0) {
>  ZeroMem (, sizeof(DigestList));
>  CopyMem (, HashData, sizeof(DigestList));
> +Status = EFI_SUCCESS;
>  if ((Flags & EDKII_TCG_PRE_HASH) !=0 ) {
>Status = Tpm2PcrExtend (
> NewEventHdr->PCRIndex,
> 

Reviewed-by: Laszlo Ersek 

I'll let Jiewen or Jian merge this.

Please change the status of TianoCore#2945 to IN_PROGRESS, and link both
versions of this patch into the ticket as well (in a new comment).

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64869): https://edk2.groups.io/g/devel/message/64869
Mute This Topic: https://groups.io/mt/76530112/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-