subject:"\[edk2\] \[Patch 3\/3\] BaseTools\/GenFw\: Correct datatypes in diagnostic messages and check for string termination"

Re: [edk2] [Patch 3/3] BaseTools/GenFw: Correct datatypes in diagnostic messages and check for string termination

2016-02-17 Thread Zhu, Yonghong

Thanks for report. I will fix it soon.

Best Regards,

Zhu Yonghong

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Wednesday, February 17, 2016 4:28 PM
To: Gao, Liming
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [Patch 3/3] BaseTools/GenFw: Correct datatypes in 
diagnostic messages and check for string termination

On 1 February 2016 at 03:10, Liming Gao  wrote:
> From: Michael LeMay 
>
> This patch revises multiple diagnostic messages to use correct 
> datatypes.  It also checks that a symbol name that is about to be used 
> in a diagnostic message is terminated by a null character within the 
> contents of the string table section so that the print routine does 
> not read past the end of the string table section contents when 
> reading the symbol name.
>
> Signed-off-by: Michael LeMay 

Hello Liming,

This patch is breaking the BaseTools build under GCC:

Elf32Convert.c: In function 'GetSymName':
Elf32Convert.c:317:3: error: 'for' loop initial declarations are only allowed 
in C99 or C11 mode
   for (UINT32 i = Sym->st_name; (i < StrtabShdr->sh_size) && !foundEnd; i++) {
   ^
Elf32Convert.c:317:3: note: use option -std=c99, -std=gnu99, -std=c11 or 
-std=gnu11 to compile your code

Regards,
Ard.

> ---
>  BaseTools/Source/C/GenFw/Elf32Convert.c | 15 ---  
> BaseTools/Source/C/GenFw/Elf64Convert.c | 15 ---
>  2 files changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c 
> b/BaseTools/Source/C/GenFw/Elf32Convert.c
> index a842ceb..d4258e5 100644
> --- a/BaseTools/Source/C/GenFw/Elf32Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
> @@ -21,6 +21,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  #include 
>  #endif
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -310,7 +311,15 @@ GetSymName (
>
>assert(Sym->st_name < StrtabShdr->sh_size);
>
> -  return (UINT8*)mEhdr + StrtabShdr->sh_offset + Sym->st_name;
> +  UINT8* StrtabContents = (UINT8*)mEhdr + StrtabShdr->sh_offset;
> +
> +  bool foundEnd = false;
> +  for (UINT32 i = Sym->st_name; (i < StrtabShdr->sh_size) && !foundEnd; i++) 
> {
> +foundEnd = StrtabContents[i] == 0;  }  assert(foundEnd);
> +
> +  return StrtabContents + Sym->st_name;
>  }
>
>  //
> @@ -539,7 +548,7 @@ ScanSections32 (
>  NtHdr->Pe32.OptionalHeader.Magic = EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC;
>  break;
>default:
> -VerboseMsg ("%s unknown e_machine type. Assume IA-32", 
> (UINTN)mEhdr->e_machine);
> +VerboseMsg ("%s unknown e_machine type %hu. Assume IA-32", 
> + mInImageName, mEhdr->e_machine);
>  NtHdr->Pe32.FileHeader.Machine = EFI_IMAGE_MACHINE_IA32;
>  NtHdr->Pe32.OptionalHeader.Magic = EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC;
>}
> @@ -725,7 +734,7 @@ WriteSections32 (
>}
>
>Error (NULL, 0, 3000, "Invalid",
> - "%s: Bad definition for symbol '%s'@%p or unsupported 
> symbol type.  "
> + "%s: Bad definition for symbol '%s'@%#x or unsupported 
> symbol type.  "
>   "For example, absolute and undefined symbols are not 
> supported.",
>   mInImageName, SymName, Sym->st_value);
>
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
> b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index fad270c..7b7282b 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -21,6 +21,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  #include 
>  #endif
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -302,7 +303,15 @@ GetSymName (
>
>assert(Sym->st_name < StrtabShdr->sh_size);
>
> -  return (UINT8*)mEhdr + StrtabShdr->sh_offset + Sym->st_name;
> +  UINT8* StrtabContents = (UINT8*)mEhdr + StrtabShdr->sh_offset;
> +
> +  bool foundEnd = false;
> +  for (UINT32 i = Sym->st_name; (i < StrtabShdr->sh_size) && !foundEnd; i++) 
> {
> +foundEnd = StrtabContents[i] == 0;  }  assert(foundEnd);
> +
> +  return StrtabContents + Sym->st_name;
>  }
>
>  //
> @@ -337,7 +346,7 @@ ScanSections64 (
>  mCoffOffset += sizeof (EFI_IMAGE_NT_HEADERS64);
>break;
>default:
> -VerboseMsg ("%s unknown e_machine type. Assume X64", 
> (UINTN)mEhdr->e_machine);
> +VerboseMsg ("%s unknown e_machine type %hu. Assume X64",

Re: [edk2] [Patch 3/3] BaseTools/GenFw: Correct datatypes in diagnostic messages and check for string termination

2016-02-17 Thread Ard Biesheuvel

On 1 February 2016 at 03:10, Liming Gao  wrote:
> From: Michael LeMay 
>
> This patch revises multiple diagnostic messages to use correct
> datatypes.  It also checks that a symbol name that is about to be used
> in a diagnostic message is terminated by a null character within the
> contents of the string table section so that the print routine does
> not read past the end of the string table section contents when
> reading the symbol name.
>
> Signed-off-by: Michael LeMay 

Hello Liming,

This patch is breaking the BaseTools build under GCC:

Elf32Convert.c: In function 'GetSymName':
Elf32Convert.c:317:3: error: 'for' loop initial declarations are only
allowed in C99 or C11 mode
   for (UINT32 i = Sym->st_name; (i < StrtabShdr->sh_size) && !foundEnd; i++) {
   ^
Elf32Convert.c:317:3: note: use option -std=c99, -std=gnu99, -std=c11
or -std=gnu11 to compile your code

Regards,
Ard.

> ---
>  BaseTools/Source/C/GenFw/Elf32Convert.c | 15 ---
>  BaseTools/Source/C/GenFw/Elf64Convert.c | 15 ---
>  2 files changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c 
> b/BaseTools/Source/C/GenFw/Elf32Convert.c
> index a842ceb..d4258e5 100644
> --- a/BaseTools/Source/C/GenFw/Elf32Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
> @@ -21,6 +21,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  #include 
>  #endif
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -310,7 +311,15 @@ GetSymName (
>
>assert(Sym->st_name < StrtabShdr->sh_size);
>
> -  return (UINT8*)mEhdr + StrtabShdr->sh_offset + Sym->st_name;
> +  UINT8* StrtabContents = (UINT8*)mEhdr + StrtabShdr->sh_offset;
> +
> +  bool foundEnd = false;
> +  for (UINT32 i = Sym->st_name; (i < StrtabShdr->sh_size) && !foundEnd; i++) 
> {
> +foundEnd = StrtabContents[i] == 0;
> +  }
> +  assert(foundEnd);
> +
> +  return StrtabContents + Sym->st_name;
>  }
>
>  //
> @@ -539,7 +548,7 @@ ScanSections32 (
>  NtHdr->Pe32.OptionalHeader.Magic = EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC;
>  break;
>default:
> -VerboseMsg ("%s unknown e_machine type. Assume IA-32", 
> (UINTN)mEhdr->e_machine);
> +VerboseMsg ("%s unknown e_machine type %hu. Assume IA-32", mInImageName, 
> mEhdr->e_machine);
>  NtHdr->Pe32.FileHeader.Machine = EFI_IMAGE_MACHINE_IA32;
>  NtHdr->Pe32.OptionalHeader.Magic = EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC;
>}
> @@ -725,7 +734,7 @@ WriteSections32 (
>}
>
>Error (NULL, 0, 3000, "Invalid",
> - "%s: Bad definition for symbol '%s'@%p or unsupported 
> symbol type.  "
> + "%s: Bad definition for symbol '%s'@%#x or unsupported 
> symbol type.  "
>   "For example, absolute and undefined symbols are not 
> supported.",
>   mInImageName, SymName, Sym->st_value);
>
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
> b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index fad270c..7b7282b 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -21,6 +21,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  #include 
>  #endif
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -302,7 +303,15 @@ GetSymName (
>
>assert(Sym->st_name < StrtabShdr->sh_size);
>
> -  return (UINT8*)mEhdr + StrtabShdr->sh_offset + Sym->st_name;
> +  UINT8* StrtabContents = (UINT8*)mEhdr + StrtabShdr->sh_offset;
> +
> +  bool foundEnd = false;
> +  for (UINT32 i = Sym->st_name; (i < StrtabShdr->sh_size) && !foundEnd; i++) 
> {
> +foundEnd = StrtabContents[i] == 0;
> +  }
> +  assert(foundEnd);
> +
> +  return StrtabContents + Sym->st_name;
>  }
>
>  //
> @@ -337,7 +346,7 @@ ScanSections64 (
>  mCoffOffset += sizeof (EFI_IMAGE_NT_HEADERS64);
>break;
>default:
> -VerboseMsg ("%s unknown e_machine type. Assume X64", 
> (UINTN)mEhdr->e_machine);
> +VerboseMsg ("%s unknown e_machine type %hu. Assume X64", mInImageName, 
> mEhdr->e_machine);
>  mCoffOffset += sizeof (EFI_IMAGE_NT_HEADERS64);
>break;
>}
> @@ -721,7 +730,7 @@ WriteSections64 (
>}
>
>Error (NULL, 0, 3000, "Invalid",
> - "%s: Bad definition for symbol '%s'@%p or unsupported 
> symbol type.  "
> + "%s: Bad definition for symbol '%s'@%#llx or unsupported 
> symbol type.  "
>   "For example, absolute and undefined symbols are not 
> supported.",
>   mInImageName, SymName, Sym->st_value);
>
> --
> 1.9.5.msysgit.0
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch 3/3] BaseTools/GenFw: Correct datatypes in diagnostic messages and check for string termination

2016-01-31 Thread Liming Gao

From: Michael LeMay 

This patch revises multiple diagnostic messages to use correct
datatypes.  It also checks that a symbol name that is about to be used
in a diagnostic message is terminated by a null character within the
contents of the string table section so that the print routine does
not read past the end of the string table section contents when
reading the symbol name.

Signed-off-by: Michael LeMay 
---
 BaseTools/Source/C/GenFw/Elf32Convert.c | 15 ---
 BaseTools/Source/C/GenFw/Elf64Convert.c | 15 ---
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c 
b/BaseTools/Source/C/GenFw/Elf32Convert.c
index a842ceb..d4258e5 100644
--- a/BaseTools/Source/C/GenFw/Elf32Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
@@ -21,6 +21,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #endif
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -310,7 +311,15 @@ GetSymName (
 
   assert(Sym->st_name < StrtabShdr->sh_size);
 
-  return (UINT8*)mEhdr + StrtabShdr->sh_offset + Sym->st_name;
+  UINT8* StrtabContents = (UINT8*)mEhdr + StrtabShdr->sh_offset;
+
+  bool foundEnd = false;
+  for (UINT32 i = Sym->st_name; (i < StrtabShdr->sh_size) && !foundEnd; i++) {
+foundEnd = StrtabContents[i] == 0;
+  }
+  assert(foundEnd);
+
+  return StrtabContents + Sym->st_name;
 }
 
 //
@@ -539,7 +548,7 @@ ScanSections32 (
 NtHdr->Pe32.OptionalHeader.Magic = EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC;
 break;
   default:
-VerboseMsg ("%s unknown e_machine type. Assume IA-32", 
(UINTN)mEhdr->e_machine);
+VerboseMsg ("%s unknown e_machine type %hu. Assume IA-32", mInImageName, 
mEhdr->e_machine);
 NtHdr->Pe32.FileHeader.Machine = EFI_IMAGE_MACHINE_IA32;
 NtHdr->Pe32.OptionalHeader.Magic = EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC;
   }
@@ -725,7 +734,7 @@ WriteSections32 (
   }
 
   Error (NULL, 0, 3000, "Invalid",
- "%s: Bad definition for symbol '%s'@%p or unsupported symbol 
type.  "
+ "%s: Bad definition for symbol '%s'@%#x or unsupported symbol 
type.  "
  "For example, absolute and undefined symbols are not 
supported.",
  mInImageName, SymName, Sym->st_value);
 
diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index fad270c..7b7282b 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -21,6 +21,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #endif
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -302,7 +303,15 @@ GetSymName (
 
   assert(Sym->st_name < StrtabShdr->sh_size);
 
-  return (UINT8*)mEhdr + StrtabShdr->sh_offset + Sym->st_name;
+  UINT8* StrtabContents = (UINT8*)mEhdr + StrtabShdr->sh_offset;
+
+  bool foundEnd = false;
+  for (UINT32 i = Sym->st_name; (i < StrtabShdr->sh_size) && !foundEnd; i++) {
+foundEnd = StrtabContents[i] == 0;
+  }
+  assert(foundEnd);
+
+  return StrtabContents + Sym->st_name;
 }
 
 //
@@ -337,7 +346,7 @@ ScanSections64 (
 mCoffOffset += sizeof (EFI_IMAGE_NT_HEADERS64);
   break;
   default:
-VerboseMsg ("%s unknown e_machine type. Assume X64", 
(UINTN)mEhdr->e_machine);
+VerboseMsg ("%s unknown e_machine type %hu. Assume X64", mInImageName, 
mEhdr->e_machine);
 mCoffOffset += sizeof (EFI_IMAGE_NT_HEADERS64);
   break;
   }
@@ -721,7 +730,7 @@ WriteSections64 (
   }
 
   Error (NULL, 0, 3000, "Invalid",
- "%s: Bad definition for symbol '%s'@%p or unsupported symbol 
type.  "
+ "%s: Bad definition for symbol '%s'@%#llx or unsupported 
symbol type.  "
  "For example, absolute and undefined symbols are not 
supported.",
  mInImageName, SymName, Sym->st_value);
 
-- 
1.9.5.msysgit.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 3/3] BaseTools/GenFw: Correct datatypes in diagnostic messages and check for string termination

Re: [edk2] [Patch 3/3] BaseTools/GenFw: Correct datatypes in diagnostic messages and check for string termination

[edk2] [Patch 3/3] BaseTools/GenFw: Correct datatypes in diagnostic messages and check for string termination

3 matches

Site Navigation

Mail list logo

Footer information