linux-kernel - Re: [PATCH v3 2/2] selftests/bpf: Add testcases for BPF_ADD and BPF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAADnVQL6ivxjMdej3Lp+fww4_0C1bKhq3SFtCj597qRjSRjMCQ@mail.gmail.com>
Date: Tue, 24 Jun 2025 18:50:18 -0700
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Eduard Zingerman <eddyz87@...il.com>
Cc: Harishankar Vishwanathan <harishankar.vishwanathan@...il.com>, Alexei Starovoitov <ast@...nel.org>, 
	Matan Shachnai <m.shachnai@...gers.edu>, Srinivas Narayana <srinivas.narayana@...gers.edu>, 
	Santosh Nagarakatte <santosh.nagarakatte@...gers.edu>, Daniel Borkmann <daniel@...earbox.net>, 
	John Fastabend <john.fastabend@...il.com>, Andrii Nakryiko <andrii@...nel.org>, 
	Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, KP Singh <kpsingh@...nel.org>, 
	Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, 
	Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>, Matan Shachnai <m.shachnai@...il.com>, 
	Henriette Herzog <henriette.herzog@....de>, Luis Gerhorst <luis.gerhorst@....de>, 
	Kumar Kartikeya Dwivedi <memxor@...il.com>, bpf <bpf@...r.kernel.org>, 
	LKML <linux-kernel@...r.kernel.org>, 
	"open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH v3 2/2] selftests/bpf: Add testcases for BPF_ADD and BPF_SUB

On Tue, Jun 24, 2025 at 3:45 PM Eduard Zingerman <eddyz87@...il.com> wrote:
>
> On Mon, 2025-06-23 at 00:03 -0400, Harishankar Vishwanathan wrote:
> > The previous commit improves the precision in scalar(32)_min_max_add,
> > and scalar(32)_min_max_sub. The improvement in precision occurs in cases
> > when all outcomes overflow or underflow, respectively.
> >
> > This commit adds selftests that exercise those cases.
> >
> > This commit also adds selftests for cases where the output register
> > state bounds for u(32)_min/u(32)_max are conservatively set to unbounded
> > (when there is partial overflow or underflow).
> >
> > Signed-off-by: Harishankar Vishwanathan <harishankar.vishwanathan@...il.com>
> > Co-developed-by: Matan Shachnai <m.shachnai@...gers.edu>
> > Signed-off-by: Matan Shachnai <m.shachnai@...gers.edu>
> > Suggested-by: Eduard Zingerman <eddyz87@...il.com>
> > ---
>
> Thank you for adding these tests.  Even with "human readable" numbers
> took me 15-20 minutes to verify the numbers :)
>
> Acked-by: Eduard Zingerman <eddyz87@...il.com>
>
> >  .../selftests/bpf/progs/verifier_bounds.c     | 161 ++++++++++++++++++
> >  1 file changed, 161 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> > index 30e16153fdf1..31986f6c609e 100644
> > --- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
> > +++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> > @@ -1371,4 +1371,165 @@ __naked void mult_sign_ovf(void)
> >         __imm(bpf_skb_store_bytes)
> >       : __clobber_all);
> >  }
> > +
> > +SEC("socket")
> > +__description("64-bit addition, all outcomes overflow")
> > +__success __log_level(2)
> > +__msg("5: (0f) r3 += r3 {{.*}} R3_w=scalar(umin=0x4000000000000000,umax=0xfffffffffffffffe)")
> > +__retval(0)
> > +__naked void add64_full_overflow(void)
> > +{
> > +     asm volatile (
> > +     "r4 = 0;"
> > +     "r4 = -r4;"
>
> Nit: there is a change in the workings that would make range
>      propagation in negation instruction, a better way to get unbound
>      scalar here is e.g. call to bpf_get_prandom_u32() or read from a
>      constant global map.
>      Depending on order in which patches would be accepted this rework
>      would be either on you or on the other patch-set author.

Fixed them up like below while applying:

diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c
b/tools/testing/selftests/bpf/progs/verifier_bounds.c
index 31986f6c609e..e52a24e15b34 100644
--- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
+++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c
@@ -1380,15 +1380,15 @@ __retval(0)
 __naked void add64_full_overflow(void)
 {
        asm volatile (
-       "r4 = 0;"
-       "r4 = -r4;"
+       "call %[bpf_get_prandom_u32];"
+       "r4 = r0;"
        "r3 = 0xa000000000000000 ll;"
        "r3 |= r4;"
        "r3 += r3;"
        "r0 = 0;"
        "exit"
        :
-       :
+       : __imm(bpf_get_prandom_u32)
        : __clobber_all);
 }

@@ -1400,15 +1400,15 @@ __retval(0)
 __naked void add64_partial_overflow(void)
 {
        asm volatile (
-       "r4 = 0;"
-       "r4 = -r4;"
+       "call %[bpf_get_prandom_u32];"
+       "r4 = r0;"
        "r3 = 2;"
        "r3 |= r4;"
        "r3 += r3;"
        "r0 = 0;"
        "exit"
        :
-       :
+       : __imm(bpf_get_prandom_u32)
        : __clobber_all);
 }

etc