[edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error

Michael D Kinney posted 1 patch 1 week ago
Failed in applying to current master (apply log)
.../UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c     | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

[edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error

Posted by Michael D Kinney 1 week ago
https://bugzilla.tianocore.org/show_bug.cgi?id=1970

Update local variable in ParseAcpiSlot() to be UINT32
instead of UINT64 to avoid 64-bit multiply operation
in the SLIT_ELEMENT() macro.

Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 .../UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c     | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
index 1f9dac66ee..af85c9aa1c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
@@ -57,8 +57,8 @@ ParseAcpiSlit (
   )
 {
   UINT32 Offset;
-  UINT64 Count;
-  UINT64 Index;
+  UINT32 Count;
+  UINT32 Index;
   UINT64 LocalityCount;
   UINT8* LocalityPtr;
   CHAR16 Buffer[80];  // Used for AsciiName param of ParseAcpi
-- 
2.21.0.windows.1


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

View/Reply Online (#43529): https://edk2.groups.io/g/devel/message/43529
Mute This Topic: https://groups.io/mt/32421790/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error

Posted by Sami Mujawar 1 week ago
Hi Mike,

Since LocalityCount is 64-bit wide the SLIT validation code could possibly end up in an infinite loop. I am not aware of a platform that has a large enough LocalityCount to hit this condition. However, would it be good to have a check that limits the validation to MAX_UINT32?

e.g. Something like
if  (LocalityCount < MAX_UINT32) {
  // Validate
  for (Count = 0; Count < LocalityCount; Count++) {
    for (Index = 0; Index < LocalityCount; Index++) {
  ...
} else {
  Print (L"INFO: Skipping validation of System Localities as locality count is > MAX_UINT32\n");
}

Regards,

Sami Mujawar

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney via Groups.Io
Sent: 10 July 2019 11:35 PM
To: devel@edk2.groups.io
Cc: Jaben Carsey <jaben.carsey@intel.com>; Ray Ni <ray.ni@intel.com>; Zhichao Gao <zhichao.gao@intel.com>
Subject: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error

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

Update local variable in ParseAcpiSlot() to be UINT32 instead of UINT64 to avoid 64-bit multiply operation in the SLIT_ELEMENT() macro.

Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 .../UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c     | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
index 1f9dac66ee..af85c9aa1c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitPars
+++ er.c
@@ -57,8 +57,8 @@ ParseAcpiSlit (
   )
 {
   UINT32 Offset;
-  UINT64 Count;
-  UINT64 Index;
+  UINT32 Count;
+  UINT32 Index;
   UINT64 LocalityCount;
   UINT8* LocalityPtr;
   CHAR16 Buffer[80];  // Used for AsciiName param of ParseAcpi
--
2.21.0.windows.1




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

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

View/Reply Online (#43585): https://edk2.groups.io/g/devel/message/43585
Mute This Topic: https://groups.io/mt/32421790/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error

Posted by Carsey, Jaben 1 week ago
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, July 10, 2019 3:35 PM
> To: devel@edk2.groups.io
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Gao, Zhichao <zhichao.gao@intel.com>
> Subject: [Patch] ShellPkg/AcpiView: Fix IA32 link error
> Importance: High
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1970
> 
> Update local variable in ParseAcpiSlot() to be UINT32
> instead of UINT64 to avoid 64-bit multiply operation
> in the SLIT_ELEMENT() macro.
> 
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  .../UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c     | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
> index 1f9dac66ee..af85c9aa1c 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
> @@ -57,8 +57,8 @@ ParseAcpiSlit (
>    )
>  {
>    UINT32 Offset;
> -  UINT64 Count;
> -  UINT64 Index;
> +  UINT32 Count;
> +  UINT32 Index;
>    UINT64 LocalityCount;
>    UINT8* LocalityPtr;
>    CHAR16 Buffer[80];  // Used for AsciiName param of ParseAcpi
> --
> 2.21.0.windows.1


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

View/Reply Online (#43530): https://edk2.groups.io/g/devel/message/43530
Mute This Topic: https://groups.io/mt/32421790/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-