回复: [edk2-devel] 回复: [PATCH 2/2] ShellPkg/UefiShellDebug1CommandsLib: Replace hardcoded SMBIOS strings.

["gaoliming" <gaoliming@byosoft.com.cn>]

Giri:
  I create PR https://github.com/tianocore/edk2/pull/4350 to merge this
patch. But, Open CI reports the below issue. Please help fix them. 

ERROR - 
ERROR - EFI coding style error
ERROR - *Error code: 9004
ERROR - *For comment line with '///< ... text ...' format, if it is used, it
should be after the code section
ERROR - *file:
//__w/1/s/Build/.pytool/Plugin/EccCheck/ShellPkg/Library/UefiShellDebug1Comm
andsLib/SmbiosView/PrintInfo.c
ERROR - *Line number: 139
ERROR - *
ERROR - 
ERROR - EFI coding style error
ERROR - *Error code: 9004
ERROR - *For comment line with '///< ... text ...' format, if it is used, it
should be after the code section
ERROR - *file:
//__w/1/s/Build/.pytool/Plugin/EccCheck/ShellPkg/Library/UefiShellDebug1Comm
andsLib/SmbiosView/PrintInfo.c
ERROR - *Line number: 224
ERROR - *

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 gaoliming via
> groups.io
> 发送时间: 2023年5月4日 15:57
> 收件人: 'Giri Mudusuru' <girim@apple.com>; devel@edk2.groups.io
> 抄送: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu'
> <zhiguang.liu@intel.com>; 'Andrew Fish' <afish@apple.com>
> 主题: [edk2-devel] 回复: [PATCH 2/2]
> ShellPkg/UefiShellDebug1CommandsLib: Replace hardcoded SMBIOS strings.
> 
> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> 
> > -----邮件原件-----
> > 发件人: Giri Mudusuru <girim@apple.com>
> > 发送时间: 2023年4月29日 2:53
> > 收件人: devel@edk2.groups.io
> > 抄送: Giri Mudusuru <girim@apple.com>; Michael D Kinney
> > <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> > Zhiguang Liu <zhiguang.liu@intel.com>; Andrew Fish <afish@apple.com>
> > 主题: [PATCH 2/2] ShellPkg/UefiShellDebug1CommandsLib: Replace
> > hardcoded SMBIOS strings.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3805
> >
> > Replace hardcoded SMBIOS Anchor string and size with defines.
> >
> > Fix buffer overflow as described below.
> >
> > Smbios64BitPrintEPSInfo () is coded like:
> > UINT8  Anchor[5];
> >
> > MemToString (Anchor, SmbiosTable->AnchorString, 5);
> >
> > But the definition of MemToString()
> >   Copy Length of Src buffer to Dest buffer,
> >   add a NULL termination to Dest buffer.
> >
> > So Anchor needs to be +1 the size of the SMBIOS Anchor string `_SM3_`.
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Cc: Andrew Fish <afish@apple.com>
> > Signed-off-by: Giri Mudusuru <girim@apple.com>
> > ---
> >  .../UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c    | 9
> +++++----
> >  .../UefiShellDebug1CommandsLib/SmbiosView/SmbiosView.c   | 9
> > +++++----
> >  2 files changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git
> > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > index 1811cf0c44..dd190b006f 100644
> > ---
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > +++
> > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> > @@ -5,6 +5,7 @@
> >    Copyright (c) 1985 - 2022, American Megatrends International LLC.<BR>
> >
> >    (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
> >
> >    (C) Copyright 2015-2019 Hewlett Packard Enterprise Development
> > LP<BR>
> >
> > +  Copyright (c) 2023 Apple Inc. All rights reserved.<BR>
> >
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >
> >
> >  **/
> >
> > @@ -135,7 +136,7 @@ SmbiosPrintEPSInfo (
> >    IN  UINT8                     Option
> >
> >    )
> >
> >  {
> >
> > -  UINT8  Anchor[5];
> >
> > +  UINT8  Anchor[SMBIOS_ANCHOR_STRING_LENGTH + 1]; ///<
> Including
> > terminating NULL character
> >
> >    UINT8  InAnchor[6];
> >
> >
> >
> >    if (SmbiosTable == NULL) {
> >
> > @@ -149,7 +150,7 @@ SmbiosPrintEPSInfo (
> >
> >
> >    if (Option >= SHOW_NORMAL) {
> >
> >      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_SMBIOSVIEW_PRINTINFO_ENTRY_POINT_SIGN),
> > gShellDebug1HiiHandle);
> >
> > -    MemToString (Anchor, SmbiosTable->AnchorString, 4);
> >
> > +    MemToString (Anchor, SmbiosTable->AnchorString,
> > SMBIOS_ANCHOR_STRING_LENGTH);
> >
> >      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_SMBIOSVIEW_PRINTINFO_ANCHOR_STR), gShellDebug1HiiHandle,
> > Anchor);
> >
> >      ShellPrintHiiEx (
> >
> >        -1,
> >
> > @@ -220,7 +221,7 @@ Smbios64BitPrintEPSInfo (
> >    IN  UINT8                         Option
> >
> >    )
> >
> >  {
> >
> > -  UINT8  Anchor[5];
> >
> > +  UINT8  Anchor[SMBIOS_3_0_ANCHOR_STRING_LENGTH + 1]; ///<
> > Including terminating NULL character
> >
> >
> >
> >    if (SmbiosTable == NULL) {
> >
> >      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_SMBIOSVIEW_PRINTINFO_SMBIOSTABLE_NULL),
> > gShellDebug1HiiHandle);
> >
> > @@ -234,7 +235,7 @@ Smbios64BitPrintEPSInfo (
> >    if (Option >= SHOW_NORMAL) {
> >
> >      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_SMBIOSVIEW_PRINTINFO_64_BIT_ENTRY_POINT_SIGN),
> > gShellDebug1HiiHandle);
> >
> >
> >
> > -    MemToString (Anchor, SmbiosTable->AnchorString, 5);
> >
> > +    MemToString (Anchor, SmbiosTable->AnchorString,
> > SMBIOS_3_0_ANCHOR_STRING_LENGTH);
> >
> >      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_SMBIOSVIEW_PRINTINFO_ANCHOR_STR), gShellDebug1HiiHandle,
> > Anchor);
> >
> >
> >
> >      ShellPrintHiiEx (
> >
> > diff --git
> >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView.c
> >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView.c
> > index e9360beb23..7e7eef3fd8 100644
> > ---
> >
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView.c
> > +++
> >
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView.c
> > @@ -3,6 +3,7 @@
> >
> >
> >    (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
> >
> >    Copyright (c) 2005 - 2018, Intel Corporation. All rights
reserved.<BR>
> >
> > +  Copyright (c) 2023 Apple Inc. All rights reserved.<BR>
> >
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >
> >
> >  **/
> >
> > @@ -263,7 +264,7 @@ SMBiosView (
> >      return EFI_BAD_BUFFER_SIZE;
> >
> >    }
> >
> >
> >
> > -  if (CompareMem (SMBiosTable->AnchorString, "_SM_", 4) == 0) {
> >
> > +  if (CompareMem (SMBiosTable->AnchorString,
> SMBIOS_ANCHOR_STRING,
> > SMBIOS_ANCHOR_STRING_LENGTH) == 0) {
> >
> >      //
> >
> >      // Have got SMBIOS table
> >
> >      //
> >
> > @@ -441,7 +442,7 @@ SMBios64View (
> >      return EFI_BAD_BUFFER_SIZE;
> >
> >    }
> >
> >
> >
> > -  if (CompareMem (SMBiosTable->AnchorString, "_SM3_", 5) == 0) {
> >
> > +  if (CompareMem (SMBiosTable->AnchorString,
> > SMBIOS_3_0_ANCHOR_STRING, SMBIOS_3_0_ANCHOR_STRING_LENGTH)
> > == 0) {
> >
> >      //
> >
> >      // Have got SMBIOS table
> >
> >      //
> >
> > @@ -612,7 +613,7 @@ InitSmbiosTableStatistics (
> >      return EFI_NOT_FOUND;
> >
> >    }
> >
> >
> >
> > -  if (CompareMem (SMBiosTable->AnchorString, "_SM_", 4) != 0) {
> >
> > +  if (CompareMem (SMBiosTable->AnchorString,
> SMBIOS_ANCHOR_STRING,
> > SMBIOS_ANCHOR_STRING_LENGTH) != 0) {
> >
> >      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_SMBIOSVIEW_SMBIOSVIEW_SMBIOS_TABLE),
> gShellDebug1HiiHandle);
> >
> >      return EFI_INVALID_PARAMETER;
> >
> >    }
> >
> > @@ -753,7 +754,7 @@ InitSmbios64BitTableStatistics (
> >      return EFI_NOT_FOUND;
> >
> >    }
> >
> >
> >
> > -  if (CompareMem (SMBiosTable->AnchorString, "_SM3_", 5) != 0) {
> >
> > +  if (CompareMem (SMBiosTable->AnchorString,
> > SMBIOS_3_0_ANCHOR_STRING,
> > SMBIOS_3_0_ANCHOR_STRING_LENGTH) != 0) {
> >
> >      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > (STR_SMBIOSVIEW_SMBIOSVIEW_SMBIOS_TABLE),
> gShellDebug1HiiHandle);
> >
> >      return EFI_INVALID_PARAMETER;
> >
> >    }
> >
> > --
> > 2.39.2 (Apple Git-144)
> 
> 
> 
> 
> 
> 
>