Hi Michael, 2 comments below.
Mike > -----Original Message----- > From: mikub...@linux.microsoft.com <mikub...@linux.microsoft.com> > Sent: Tuesday, November 8, 2022 11:52 AM > To: devel@edk2.groups.io > Cc: Feng, Bob C <bob.c.f...@intel.com>; Gao, Liming > <gaolim...@byosoft.com.cn>; Chen, Christine <yuwei.c...@intel.com>; Sean > Brogan <sean.bro...@microsoft.com>; Kinney, Michael D > <michael.d.kin...@intel.com> > Subject: [PATCH v1 1/2] BaseTools: Fix wrong type of arguments to formatting > functions > > From: Michael Kubacki <michael.kuba...@microsoft.com> > > Fixes issues found with the cpp/wrong-type-format-argument CodeQL > rule in BaseTools. > > Reference: > https://cwe.mitre.org/data/definitions/686.html > > The following CodeQL errors are resolved: > > 1. Check failure on line 1115 in > BaseTools/Source/C/EfiRom/EfiRom.c > > - This argument should be of type 'int' but is of type 'char *'. > - This argument should be of type 'int' but is of type 'signed > char *'. > > 2. Check failure on line 359 in > BaseTools/Source/C/GenFw/Elf32Convert.c > > - This argument should be of type 'CHAR8 *' but is of type > 'unsigned int'. > > 3. Check failure on line 1841 in > BaseTools/Source/C/GenFw/Elf64Convert.c > > - This argument should be of type 'unsigned int' but is of type > 'unsigned long long'. > > 4. Check failure on line 1871 in > BaseTools/Source/C/GenFw/Elf64Convert.c > > - This argument should be of type 'unsigned int' but is of type > 'unsigned long long'. > > 5. Check failure on line 2400 in > BaseTools/Source/C/GenFv/GenFvInternalLib.c > > - This argument should be of type 'unsigned long long' but is of > type 'unsigned int'. > > 6. Check failure on line 1099 in > BaseTools/Source/C/GenFw/Elf64Convert.c > > - This argument should be of type 'CHAR8 *' but is of type > 'unsigned int'. > > 7. Check failure on line 1098 in > BaseTools/Source/C/GenSec/GenSec.c > > - This argument should be of type 'CHAR8 *' but is of type > 'char **'. > > 8. Check failure on line 911 in > BaseTools/Source/C/GenSec/GenSec.c > > - This argument should be of type 'CHAR8 *' but is of type > 'char **'. > > Cc: Bob Feng <bob.c.f...@intel.com> > Cc: Liming Gao <gaolim...@byosoft.com.cn> > Cc: Yuwei Chen <yuwei.c...@intel.com> > Cc: Sean Brogan <sean.bro...@microsoft.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Signed-off-by: Michael Kubacki <michael.kuba...@microsoft.com> > --- > BaseTools/Source/C/EfiRom/EfiRom.c | 2 +- > BaseTools/Source/C/GenFv/GenFvInternalLib.c | 2 +- > BaseTools/Source/C/GenFw/Elf32Convert.c | 2 +- > BaseTools/Source/C/GenFw/Elf64Convert.c | 6 +++--- > BaseTools/Source/C/GenSec/GenSec.c | 4 ++-- > 5 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/BaseTools/Source/C/EfiRom/EfiRom.c > b/BaseTools/Source/C/EfiRom/EfiRom.c > index 2506f559d574..fa7bf0e62e6d 100644 > --- a/BaseTools/Source/C/EfiRom/EfiRom.c > +++ b/BaseTools/Source/C/EfiRom/EfiRom.c > @@ -1112,7 +1112,7 @@ Routine Description: > goto Done; > } > if (DebugLevel > 9) { > - Error (NULL, 0, 2000, "Invalid option value", "Debug Level range > is 0-9, current input level is %d", Argv[1]); > + Error (NULL, 0, 2000, "Invalid option value", "Debug Level range > is 0-9, current input level is %llu", DebugLevel); > ReturnStatus = 1; > goto Done; > } > diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c > b/BaseTools/Source/C/GenFv/GenFvInternalLib.c > index b5b942500334..6bd59515b1aa 100644 > --- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c > +++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c > @@ -2397,7 +2397,7 @@ Routine Description: > VerboseMsg("SecCore entry point Address = 0x%llX", (unsigned long long) > SecCoreEntryAddress); > VerboseMsg("BaseAddress = 0x%llX", (unsigned long long) > FvInfo->BaseAddress); > bSecCore = (UINT32)(SecCoreEntryAddress - FvInfo->BaseAddress); > - VerboseMsg("offset = 0x%llX", bSecCore); > + VerboseMsg("offset = 0x%X", bSecCore); > > if(bSecCore > 0x0fffff) { > Error(NULL, 0, 3000, "Invalid", "SEC Entry point must be within 1MB of > start of the FV"); > diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c > b/BaseTools/Source/C/GenFw/Elf32Convert.c > index d917a444c82d..87d7f133f132 100644 > --- a/BaseTools/Source/C/GenFw/Elf32Convert.c > +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c > @@ -356,7 +356,7 @@ ScanSections32 ( > mCoffOffset += sizeof (EFI_IMAGE_NT_HEADERS32); > break; > default: > - VerboseMsg ("%s unknown e_machine type. Assume IA-32", > (UINTN)mEhdr->e_machine); > + VerboseMsg ("%u unknown e_machine type. Assume IA-32", > (UINTN)mEhdr->e_machine); > mCoffOffset += sizeof (EFI_IMAGE_NT_HEADERS32); > break; > } > diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c > b/BaseTools/Source/C/GenFw/Elf64Convert.c > index c6092269e2d1..8b50774beb1e 100644 > --- a/BaseTools/Source/C/GenFw/Elf64Convert.c > +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c > @@ -1096,7 +1096,7 @@ ScanSections64 ( > break; > > default: > - VerboseMsg ("%s unknown e_machine type. Assume X64", > (UINTN)mEhdr->e_machine); > + VerboseMsg ("%u unknown e_machine type. Assume X64", > (UINTN)mEhdr->e_machine); > NtHdr->Pe32Plus.FileHeader.Machine = EFI_IMAGE_MACHINE_X64; > NtHdr->Pe32Plus.OptionalHeader.Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC; > } > @@ -1837,7 +1837,7 @@ WriteRelocations64 ( > case R_X86_64_REX_GOTPCRELX: > break; > case R_X86_64_64: > - VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08X", > + VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08llX", > mCoffSectionsOffset[RelShdr->sh_info] + (Rel->r_offset - > SecShdr->sh_addr)); > CoffAddFixup( > (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info] > @@ -1867,7 +1867,7 @@ WriteRelocations64 ( > // > // case R_X86_64_32S: > case R_X86_64_32: > - VerboseMsg ("EFI_IMAGE_REL_BASED_HIGHLOW Offset: 0x%08X", > + VerboseMsg ("EFI_IMAGE_REL_BASED_HIGHLOW Offset: 0x%08llX", > mCoffSectionsOffset[RelShdr->sh_info] + (Rel->r_offset - > SecShdr->sh_addr)); > CoffAddFixup( > (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info] > diff --git a/BaseTools/Source/C/GenSec/GenSec.c > b/BaseTools/Source/C/GenSec/GenSec.c > index a4c2d19aa6f4..757cb079a3a2 100644 > --- a/BaseTools/Source/C/GenSec/GenSec.c > +++ b/BaseTools/Source/C/GenSec/GenSec.c > @@ -908,7 +908,7 @@ Routine Description: > if (FileBuffer != NULL) { > free (FileBuffer); > } > - Error (NULL, 0, 2000, "Invalid parameter", "the size of input file %s > can't be zero", InputFileName); > + Error (NULL, 0, 2000, "Invalid parameter", "the size of input file %s > can't be zero", InputFileName[0]); InputFileName is type CHAR8 **. I think *InputFileName would be clearer than InputFileName[0]. > return EFI_NOT_FOUND; > } > > @@ -1095,7 +1095,7 @@ Routine Description: > if (FileBuffer != NULL) { > free (FileBuffer); > } > - Error (NULL, 0, 2000, "Invalid parameter", "the size of input file %s > can't be zero", InputFileName); > + Error (NULL, 0, 2000, "Invalid parameter", "the size of input file %s > can't be zero", InputFileName[0]); InputFileName is type CHAR8 **. I think *InputFileName would be clearer than InputFileName[0]; > return EFI_NOT_FOUND; > } > > -- > 2.28.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#96121): https://edk2.groups.io/g/devel/message/96121 Mute This Topic: https://groups.io/mt/94898434/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
