Re: [PATCH 05/10] cmdline: fix port list parsing

[David Marchand <david.marchand@redhat.com>]

On Fri, Jun 20, 2025 at 11:59 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Thu, Jun 19, 2025 at 09:10:31AM +0200, David Marchand wrote:
> > Doing arithmetics with the NULL pointer is undefined.
> >
> > Caught by UBSan:
> >
> > ../lib/cmdline/cmdline_parse_portlist.c:40:19: runtime error:
> >       applying non-zero offset 1 to null pointer
> > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> >       ../lib/cmdline/cmdline_parse_portlist.c:40:19 in
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  lib/cmdline/cmdline_parse_portlist.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/cmdline/cmdline_parse_portlist.c b/lib/cmdline/cmdline_parse_portlist.c
> > index ef6ce223b5..0c07cc02b5 100644
> > --- a/lib/cmdline/cmdline_parse_portlist.c
> > +++ b/lib/cmdline/cmdline_parse_portlist.c
> > @@ -4,6 +4,7 @@
> >   * All rights reserved.
> >   */
> >
> > +#include <stdbool.h>
> >  #include <stdio.h>
> >  #include <stdlib.h>
> >  #include <string.h>
> > @@ -37,10 +38,11 @@ parse_ports(cmdline_portlist_t *pl, const char *str)
> >       const char *first, *last;
> >       char *end;
> >
> > -     for (first = str, last = first;
> > -         first != NULL && last != NULL;
> > -         first = last + 1) {
>
> Maybe I'm a little slow this morning, but I can't see how this is actually
> a problem. By my understanding, the check for "first != NULL && last !=
> NULL" happens before any increment of "first = last + 1", meaning we are
> guaranteed that the last is never null when we increment it.

Well, not sure I follow, but the problem is not at the first
iteration, if this is what you mean.

On the last iteration of the parsing, there is no , left in the string
that is parsed so last = strchr(first, ',') makes last == NULL.
Then the first variable is set to last + 1 *before* evaluating the end
condition.

I removed this patch of the series, rerun the test and I see:

9/75 DPDK:fast-tests / cmdline_autotest               OK              0.22s
09:20:08 DPDK_TEST=cmdline_autotest MALLOC_PERTURB_=169
/home/runner/work/dpdk/dpdk/build/app/dpdk-test --no-huge -m 2048 -d
/home/runner/work/dpdk/dpdk/build/drivers
----------------------------------- output -----------------------------------
stdout:
RTE>>cmdline_autotest
Testind parsing ethernet addresses...
Testind parsing port lists...
Testind parsing numbers...
Testing parsing IP addresses...
Testing parsing strings...
Testing circular buffer...
Testing library functions...
Test OK
RTE>>
stderr:
EAL: Detected CPU lcores: 4
EAL: Detected NUMA nodes: 1
EAL: Detected shared linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
APP: HPET is not enabled, using TSC as default timer
../lib/cmdline/cmdline_parse_portlist.c:44:19: runtime error: applying
non-zero offset 1 to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
../lib/cmdline/cmdline_parse_portlist.c:44:19 in
------------------------------------------------------------------------------


-- 
David Marchand