pmdinfogen: allow padding after NUL terminator

Message ID 20210526214343.31352-1-dmitry.kozliuk@gmail.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series pmdinfogen: allow padding after NUL terminator |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Functional fail Functional Testing issues
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/github-robot success github build: passed
ci/iol-testing fail Testing issues
ci/iol-mellanox-Functional fail Functional Testing issues
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation fail Compilation issues
ci/intel-Testing success Testing PASS

Commit Message

Dmitry Kozlyuk May 26, 2021, 9:43 p.m. UTC
  Size of string constant symbol may be larger than its length
measured up to NUL terminator. In this case pmdinfogen included padding
bytes after NUL terminator in generated source, yielding incorrect code.

Always trim string data to NUL terminator while reading ELF.
It was already done for COFF because there's no symbol size.

Bugzilla ID: 720
Fixes: f0f93a7adfee ("buildtools: use Python pmdinfogen")

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 buildtools/coff.py       |  6 ------
 buildtools/pmdinfogen.py | 10 ++++++++--
 2 files changed, 8 insertions(+), 8 deletions(-)
  

Comments

David Marchand May 27, 2021, 6:53 a.m. UTC | #1
On Wed, May 26, 2021 at 11:44 PM Dmitry Kozlyuk
<dmitry.kozliuk@gmail.com> wrote:
>
> Size of string constant symbol may be larger than its length
> measured up to NUL terminator. In this case pmdinfogen included padding
> bytes after NUL terminator in generated source, yielding incorrect code.
>
> Always trim string data to NUL terminator while reading ELF.
> It was already done for COFF because there's no symbol size.
>
> Bugzilla ID: 720
> Fixes: f0f93a7adfee ("buildtools: use Python pmdinfogen")
>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

Reproduced the issue described in bz on my (old) RHEL7 with clang 3.4.2.

Reviewed-by: David Marchand <david.marchand@redhat.com>

Just to confirm my reading of the C version of pmdinfogen: the C
version formats those symbols fine with printf %s.
So there should be no need for a fix in stable branches, right?
  
Dmitry Kozlyuk May 27, 2021, 7:09 a.m. UTC | #2
On Thu, May 27, 2021, 09:53 David Marchand <david.marchand@redhat.com>
wrote:

> Just to confirm my reading of the C version of pmdinfogen: the C version
> formats those symbols fine with printf %s.
> So there should be no need for a fix in stable branches, right?
>

Yes.

>
  
David Marchand May 27, 2021, 7:31 a.m. UTC | #3
On Wed, May 26, 2021 at 11:44 PM Dmitry Kozlyuk
<dmitry.kozliuk@gmail.com> wrote:
>
> Size of string constant symbol may be larger than its length
> measured up to NUL terminator. In this case pmdinfogen included padding
> bytes after NUL terminator in generated source, yielding incorrect code.
>
> Always trim string data to NUL terminator while reading ELF.
> It was already done for COFF because there's no symbol size.
>
> Bugzilla ID: 720
> Fixes: f0f93a7adfee ("buildtools: use Python pmdinfogen")
>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  buildtools/coff.py       |  6 ------
>  buildtools/pmdinfogen.py | 10 ++++++++--
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/buildtools/coff.py b/buildtools/coff.py
> index 86fb0602b7..a7b6c37e32 100644
> --- a/buildtools/coff.py
> +++ b/buildtools/coff.py
> @@ -146,9 +146,3 @@ def get_section_data(self, number):
>
>      def get_string(self, offset):
>          return decode_asciiz(self._strings[offset:])
                  ^^^^

coff.py still needs this helper.

Caught in the lab:
Traceback (most recent call last):

  File "../buildtools/pmdinfogen.py", line 280, in <module>
    main()
  File "../buildtools/pmdinfogen.py", line 275, in main
    drivers = load_drivers(image)
  File "../buildtools/pmdinfogen.py", line 208, in load_drivers
    for symbol in image.find_by_prefix("this_pmd_name"):
  File "../buildtools/pmdinfogen.py", line 108, in find_by_prefix
    if symbol.name.startswith(prefix):
  File "C:\Users\builder\jenkins\workspace\Windows-Compile-DPDK-Mingw64\dpdk\buildtools\coff.py",
line 84, in name
    return decode_asciiz(bytes(self._coff.name.immediate))
NameError: name 'decode_asciiz' is not defined
Traceback (most recent call last):
  File "../buildtools/gen-pmdinfo-cfile.py", line 20, in <module>
    subprocess.run(pmdinfogen + paths + [output], check=True)
  File "c:\python38\lib\subprocess.py", line 512, in run
    raise CalledProcessError(retcode, process.args,
  

Patch

diff --git a/buildtools/coff.py b/buildtools/coff.py
index 86fb0602b7..a7b6c37e32 100644
--- a/buildtools/coff.py
+++ b/buildtools/coff.py
@@ -146,9 +146,3 @@  def get_section_data(self, number):
 
     def get_string(self, offset):
         return decode_asciiz(self._strings[offset:])
-
-
-def decode_asciiz(data):
-    index = data.find(b'\x00')
-    end = index if index >= 0 else len(data)
-    return data[:end].decode()
diff --git a/buildtools/pmdinfogen.py b/buildtools/pmdinfogen.py
index 7a739ec7d4..8bb59699b8 100755
--- a/buildtools/pmdinfogen.py
+++ b/buildtools/pmdinfogen.py
@@ -19,6 +19,12 @@ 
 import coff
 
 
+def decode_asciiz(data):
+    index = data.find(b'\x00')
+    end = index if index >= 0 else len(data)
+    return data[:end].decode()
+
+
 class ELFSymbol:
     def __init__(self, image, symbol):
         self._image = image
@@ -28,7 +34,7 @@  def __init__(self, image, symbol):
     def string_value(self):
         size = self._symbol["st_size"]
         value = self.get_value(0, size)
-        return value[:-1].decode() if value else ""
+        return decode_asciiz(value) if value else ""
 
     def get_value(self, offset, size):
         section = self._symbol["st_shndx"]
@@ -86,7 +92,7 @@  def get_value(self, offset, size):
     @property
     def string_value(self):
         value = self._symbol.get_value(0)
-        return coff.decode_asciiz(value) if value else ''
+        return decode_asciiz(value) if value else ""
 
 
 class COFFImage: