[v2] pmdinfogen: allow padding after NUL terminator
Checks
Commit Message
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>
---
v2: return helper to coff.py, where it's needed (David Marchand).
buildtools/pmdinfogen.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
2021-05-28 00:24 (UTC+0300), Dmitry Kozlyuk:
> 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>
> ---
> v2: return helper to coff.py, where it's needed (David Marchand).
>
> buildtools/pmdinfogen.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/buildtools/pmdinfogen.py b/buildtools/pmdinfogen.py
> index 7a739ec7d4..2a44f17bda 100755
> --- a/buildtools/pmdinfogen.py
> +++ b/buildtools/pmdinfogen.py
> @@ -28,7 +28,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 coff.decode_asciiz(value) # not COFF-specific
>
> def get_value(self, offset, size):
> section = self._symbol["st_shndx"]
There are CI failures that seem unrelated to this patch:
some tests with NICs that I can't check
and an Arch Linux build failure that I failed to reproduce.
GitHub Actions are passing.
Are these known CI bugs or does this patch need any corrections?
Hi Dmitry,
If the failing test is the unit test func_reentrancy_autotest, that is
being looked into , as it seems like the test case does not reliably run.
It passes / fails between different systems running both on bare metal and
in virtual environments, on different OSes and architectures.
Do you have a link to your patch in patchworks or the lab dashboard?
Cheers,
Lincoln
On Wed, Jun 9, 2021 at 11:52 AM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
wrote:
> 2021-05-28 00:24 (UTC+0300), Dmitry Kozlyuk:
> > 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>
> > ---
> > v2: return helper to coff.py, where it's needed (David Marchand).
> >
> > buildtools/pmdinfogen.py | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/buildtools/pmdinfogen.py b/buildtools/pmdinfogen.py
> > index 7a739ec7d4..2a44f17bda 100755
> > --- a/buildtools/pmdinfogen.py
> > +++ b/buildtools/pmdinfogen.py
> > @@ -28,7 +28,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 coff.decode_asciiz(value) # not COFF-specific
> >
> > def get_value(self, offset, size):
> > section = self._symbol["st_shndx"]
>
> There are CI failures that seem unrelated to this patch:
> some tests with NICs that I can't check
> and an Arch Linux build failure that I failed to reproduce.
> GitHub Actions are passing.
> Are these known CI bugs or does this patch need any corrections?
>
2021-06-09 12:01 (UTC-0400), Lincoln Lavoie:
> Hi Dmitry,
>
> If the failing test is the unit test func_reentrancy_autotest, that is
> being looked into , as it seems like the test case does not reliably run.
> It passes / fails between different systems running both on bare metal and
> in virtual environments, on different OSes and architectures.
>
> Do you have a link to your patch in patchworks or the lab dashboard?
Hi Lincoln,
Yes, this is func_reentrancy_autotest failure.
Dashboard link:
https://lab.dpdk.org/results/dashboard/patchsets/17240/
MTU and stats can hardly be affected by this patch.
Hi Dmitry,
Those failures are the result of a known issue with that machine. Those
tests were disabled on that machine shortly after you submitted that patch.
I've re-run the patch.
Owen Hilyard
On Wed, Jun 9, 2021 at 12:48 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
wrote:
> 2021-06-09 12:01 (UTC-0400), Lincoln Lavoie:
> > Hi Dmitry,
> >
> > If the failing test is the unit test func_reentrancy_autotest, that is
> > being looked into , as it seems like the test case does not reliably run.
> > It passes / fails between different systems running both on bare metal
> and
> > in virtual environments, on different OSes and architectures.
> >
> > Do you have a link to your patch in patchworks or the lab dashboard?
>
> Hi Lincoln,
>
> Yes, this is func_reentrancy_autotest failure.
> Dashboard link:
> https://lab.dpdk.org/results/dashboard/patchsets/17240/
> MTU and stats can hardly be affected by this patch.
>
27/05/2021 23:24, Dmitry Kozlyuk:
> 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>
Applied, thanks
@@ -28,7 +28,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 coff.decode_asciiz(value) # not COFF-specific
def get_value(self, offset, size):
section = self._symbol["st_shndx"]