public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next] bpf: fix selftest/xsk single test selection
@ 2026-02-18  9:20 Mahdi Faramarzpour
  2026-02-25  4:05 ` Alexei Starovoitov
  0 siblings, 1 reply; 4+ messages in thread
From: Mahdi Faramarzpour @ 2026-02-18  9:20 UTC (permalink / raw)
  To: bpf
  Cc: Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
	Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Hao Luo, Jiri Olsa, Shuah Khan, netdev,
	linux-kselftest, linux-kernel, Mahdi Faramarzpour

From: Mahdi Faramarzpour <mahdifrmx@gmail.com>

This commit fixes the integer parsing of -t option. The cli parser
only relies on errno to detect parsing errors. The manpage for
strtol (https://man7.org/linux/man-pages/man3/strtol.3.html)
states that the said function "MAY" set errno to EINVAL in case the
conversion fails. Currently on some systems, this leads to a silent
failure with return value not being exactly documented in the
manpages (probably zero). The reliable way to validate the input is
to check whether the endptr has been bumped all the way to the end
of the string or not.

Fixes: 146e30554a53 ("selftests/xsk: add option to run single test")
Signed-off-by: Mahdi Faramarzpour <mahdifrmx@gmail.com>
---
v2:
  - fix style issues
v1: https://lore.kernel.org/all/20260217080326.50564-1-mahdifrmz@gmail.com/
---
 tools/testing/selftests/bpf/xskxceiver.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 05b3cebc5..5e4095fd1 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -200,6 +200,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 	struct ifobject *ifobj;
 	u32 interface_nb = 0;
 	int option_index, c;
+	char *eptr;
 
 	opterr = 0;
 
@@ -248,8 +249,8 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 			break;
 		case 't':
 			errno = 0;
-			opt_run_test = strtol(optarg, NULL, 0);
-			if (errno)
+			opt_run_test = strtol(optarg, &eptr, 0);
+			if (errno || *eptr)
 				print_usage(argv);
 			break;
 		case 'h':
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] bpf: fix selftest/xsk single test selection
  2026-02-18  9:20 [PATCH bpf-next] bpf: fix selftest/xsk single test selection Mahdi Faramarzpour
@ 2026-02-25  4:05 ` Alexei Starovoitov
  2026-02-25  5:59   ` Mahdi Faramarzpour
  0 siblings, 1 reply; 4+ messages in thread
From: Alexei Starovoitov @ 2026-02-25  4:05 UTC (permalink / raw)
  To: Mahdi Faramarzpour
  Cc: bpf, Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
	Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Hao Luo, Jiri Olsa, Shuah Khan,
	Network Development, open list:KERNEL SELFTEST FRAMEWORK, LKML

On Wed, Feb 18, 2026 at 1:20 AM Mahdi Faramarzpour <mahdifrmx@gmail.com> wrote:
>
> From: Mahdi Faramarzpour <mahdifrmx@gmail.com>
>
> This commit fixes the integer parsing of -t option. The cli parser
> only relies on errno to detect parsing errors. The manpage for
> strtol (https://man7.org/linux/man-pages/man3/strtol.3.html)
> states that the said function "MAY" set errno to EINVAL in case the
> conversion fails. Currently on some systems, this leads to a silent
> failure with return value not being exactly documented in the
> manpages (probably zero). The reliable way to validate the input is
> to check whether the endptr has been bumped all the way to the end
> of the string or not.
>
> Fixes: 146e30554a53 ("selftests/xsk: add option to run single test")
> Signed-off-by: Mahdi Faramarzpour <mahdifrmx@gmail.com>
> ---
> v2:
>   - fix style issues
> v1: https://lore.kernel.org/all/20260217080326.50564-1-mahdifrmz@gmail.com/
> ---
>  tools/testing/selftests/bpf/xskxceiver.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index 05b3cebc5..5e4095fd1 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -200,6 +200,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>         struct ifobject *ifobj;
>         u32 interface_nb = 0;
>         int option_index, c;
> +       char *eptr;
>
>         opterr = 0;
>
> @@ -248,8 +249,8 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>                         break;
>                 case 't':
>                         errno = 0;
> -                       opt_run_test = strtol(optarg, NULL, 0);
> -                       if (errno)
> +                       opt_run_test = strtol(optarg, &eptr, 0);
> +                       if (errno || *eptr)

This is unnecessary. It was required we would have hit segfaults by now.

pw-bot: cr

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] bpf: fix selftest/xsk single test selection
  2026-02-25  4:05 ` Alexei Starovoitov
@ 2026-02-25  5:59   ` Mahdi Faramarzpour
  2026-02-25 15:31     ` Alexei Starovoitov
  0 siblings, 1 reply; 4+ messages in thread
From: Mahdi Faramarzpour @ 2026-02-25  5:59 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: bpf, Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
	Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Hao Luo, Jiri Olsa, Shuah Khan,
	Network Development, open list:KERNEL SELFTEST FRAMEWORK, LKML

On Wed, Feb 25, 2026 at 7:35 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Feb 18, 2026 at 1:20 AM Mahdi Faramarzpour <mahdifrmx@gmail.com> wrote:
> >
> > From: Mahdi Faramarzpour <mahdifrmx@gmail.com>
> >
> > This commit fixes the integer parsing of -t option. The cli parser
> > only relies on errno to detect parsing errors. The manpage for
> > strtol (https://man7.org/linux/man-pages/man3/strtol.3.html)
> > states that the said function "MAY" set errno to EINVAL in case the
> > conversion fails. Currently on some systems, this leads to a silent
> > failure with return value not being exactly documented in the
> > manpages (probably zero). The reliable way to validate the input is
> > to check whether the endptr has been bumped all the way to the end
> > of the string or not.
> >
> > Fixes: 146e30554a53 ("selftests/xsk: add option to run single test")
> > Signed-off-by: Mahdi Faramarzpour <mahdifrmx@gmail.com>
> > ---
> > v2:
> >   - fix style issues
> > v1: https://lore.kernel.org/all/20260217080326.50564-1-mahdifrmz@gmail.com/
> > ---
> >  tools/testing/selftests/bpf/xskxceiver.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > index 05b3cebc5..5e4095fd1 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -200,6 +200,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> >         struct ifobject *ifobj;
> >         u32 interface_nb = 0;
> >         int option_index, c;
> > +       char *eptr;
> >
> >         opterr = 0;
> >
> > @@ -248,8 +249,8 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> >                         break;
> >                 case 't':
> >                         errno = 0;
> > -                       opt_run_test = strtol(optarg, NULL, 0);
> > -                       if (errno)
> > +                       opt_run_test = strtol(optarg, &eptr, 0);
> > +                       if (errno || *eptr)
>
> This is unnecessary. It was required we would have hit segfaults by now.
This patch does not prevent segfaults; it prevents silent misparsing.
On systems where strtol() does not set
errno for non-numeric input, passing -t abc results in opt_run_test
being 0 without triggering usage().
Checking endptr is the documented way to validate full-string conversion.
>
> pw-bot: cr

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf-next] bpf: fix selftest/xsk single test selection
  2026-02-25  5:59   ` Mahdi Faramarzpour
@ 2026-02-25 15:31     ` Alexei Starovoitov
  0 siblings, 0 replies; 4+ messages in thread
From: Alexei Starovoitov @ 2026-02-25 15:31 UTC (permalink / raw)
  To: Mahdi Faramarzpour
  Cc: bpf, Magnus Karlsson, Maciej Fijalkowski, Stanislav Fomichev,
	Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Hao Luo, Jiri Olsa, Shuah Khan,
	Network Development, open list:KERNEL SELFTEST FRAMEWORK, LKML

On Tue, Feb 24, 2026 at 9:59 PM Mahdi Faramarzpour <mahdifrmx@gmail.com> wrote:
>
> On Wed, Feb 25, 2026 at 7:35 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Wed, Feb 18, 2026 at 1:20 AM Mahdi Faramarzpour <mahdifrmx@gmail.com> wrote:
> > >
> > > From: Mahdi Faramarzpour <mahdifrmx@gmail.com>
> > >
> > > This commit fixes the integer parsing of -t option. The cli parser
> > > only relies on errno to detect parsing errors. The manpage for
> > > strtol (https://man7.org/linux/man-pages/man3/strtol.3.html)
> > > states that the said function "MAY" set errno to EINVAL in case the
> > > conversion fails. Currently on some systems, this leads to a silent
> > > failure with return value not being exactly documented in the
> > > manpages (probably zero). The reliable way to validate the input is
> > > to check whether the endptr has been bumped all the way to the end
> > > of the string or not.
> > >
> > > Fixes: 146e30554a53 ("selftests/xsk: add option to run single test")
> > > Signed-off-by: Mahdi Faramarzpour <mahdifrmx@gmail.com>
> > > ---
> > > v2:
> > >   - fix style issues
> > > v1: https://lore.kernel.org/all/20260217080326.50564-1-mahdifrmz@gmail.com/
> > > ---
> > >  tools/testing/selftests/bpf/xskxceiver.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > > index 05b3cebc5..5e4095fd1 100644
> > > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > > @@ -200,6 +200,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> > >         struct ifobject *ifobj;
> > >         u32 interface_nb = 0;
> > >         int option_index, c;
> > > +       char *eptr;
> > >
> > >         opterr = 0;
> > >
> > > @@ -248,8 +249,8 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> > >                         break;
> > >                 case 't':
> > >                         errno = 0;
> > > -                       opt_run_test = strtol(optarg, NULL, 0);
> > > -                       if (errno)
> > > +                       opt_run_test = strtol(optarg, &eptr, 0);
> > > +                       if (errno || *eptr)
> >
> > This is unnecessary. It was required we would have hit segfaults by now.
> This patch does not prevent segfaults; it prevents silent misparsing.
> On systems where strtol() does not set
> errno for non-numeric input, passing -t abc results in opt_run_test
> being 0 without triggering usage().
> Checking endptr is the documented way to validate full-string conversion.

Prove me wrong with a selftest.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-02-25 15:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-18  9:20 [PATCH bpf-next] bpf: fix selftest/xsk single test selection Mahdi Faramarzpour
2026-02-25  4:05 ` Alexei Starovoitov
2026-02-25  5:59   ` Mahdi Faramarzpour
2026-02-25 15:31     ` Alexei Starovoitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox