[edk2-devel] [PATCH v1 1/2] BaseTools: Fix wrong type of arguments to formatting functions
Michael D Kinney
michael.d.kinney at intel.com
Tue Nov 8 23:38:19 UTC 2022
Hi Michael,
2 comments below.
Mike
> -----Original Message-----
> From: mikuback at linux.microsoft.com <mikuback at linux.microsoft.com>
> Sent: Tuesday, November 8, 2022 11:52 AM
> To: devel at edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng at intel.com>; Gao, Liming <gaoliming at byosoft.com.cn>; Chen, Christine <yuwei.chen at intel.com>; Sean
> Brogan <sean.brogan at microsoft.com>; Kinney, Michael D <michael.d.kinney at intel.com>
> Subject: [PATCH v1 1/2] BaseTools: Fix wrong type of arguments to formatting functions
>
> From: Michael Kubacki <michael.kubacki at 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.feng at intel.com>
> Cc: Liming Gao <gaoliming at byosoft.com.cn>
> Cc: Yuwei Chen <yuwei.chen at intel.com>
> Cc: Sean Brogan <sean.brogan at microsoft.com>
> Cc: Michael D Kinney <michael.d.kinney at intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list