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
| ||
|
Message-ID: <CADxym3bOgnU84Xngx_H3cwxzEqsaK8JkaYDY3F4dSr74R492ug@mail.gmail.com> Date: Mon, 18 Dec 2023 11:56:18 +0800 From: Menglong Dong <menglong8.dong@...il.com> To: Alexei Starovoitov <alexei.starovoitov@...il.com> Cc: Andrii Nakryiko <andrii@...nel.org>, Eddy Z <eddyz87@...il.com>, Yonghong Song <yonghong.song@...ux.dev>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, John Fastabend <john.fastabend@...il.com>, Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>, KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, bpf <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org> Subject: Re: [PATCH bpf-next v4 2/3] selftests/bpf: activate the OP_NE login in range_cond() On Mon, Dec 18, 2023 at 2:20 AM Alexei Starovoitov <alexei.starovoitov@...il.com> wrote: > > On Sun, Dec 17, 2023 at 5:18 AM Menglong Dong <menglong8.dong@...il.com> wrote: > > > > The edge range checking for the registers is supported by the verifier > > now, so we can activate the extended login in > > tools/testing/selftests/bpf/prog_tests/reg_bounds.c/range_cond() to test > > such logic. > > > > Besides, I added some cases to the "crafted_cases" array for this logic. > > These cases are mainly used to test the edge of the src reg and dst reg. > > > > All reg bounds testings has passed in the SLOW_TESTS mode: > > > > $ export SLOW_TESTS=1 && ./test_progs -t reg_bounds -j > > Summary: 65/18959832 PASSED, 0 SKIPPED, 0 FAILED > > > > Signed-off-by: Menglong Dong <menglong8.dong@...il.com> > > --- > > v3: > > - do some adjustment to the crafted cases that we added > > v2: > > - add some cases to the "crafted_cases" > > --- > > .../selftests/bpf/prog_tests/reg_bounds.c | 20 +++++++++++++------ > > 1 file changed, 14 insertions(+), 6 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/reg_bounds.c b/tools/testing/selftests/bpf/prog_tests/reg_bounds.c > > index 0c9abd279e18..c9dc9fe73211 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/reg_bounds.c > > +++ b/tools/testing/selftests/bpf/prog_tests/reg_bounds.c > > @@ -590,12 +590,7 @@ static void range_cond(enum num_t t, struct range x, struct range y, > > *newy = range(t, max_t(t, x.a, y.a), min_t(t, x.b, y.b)); > > break; > > case OP_NE: > > - /* generic case, can't derive more information */ > > - *newx = range(t, x.a, x.b); > > - *newy = range(t, y.a, y.b); > > - break; > > - > > - /* below extended logic is not supported by verifier just yet */ > > + /* below logic is supported by the verifier now */ > > if (x.a == x.b && x.a == y.a) { > > /* X is a constant matching left side of Y */ > > *newx = range(t, x.a, x.b); > > @@ -2101,6 +2096,19 @@ static struct subtest_case crafted_cases[] = { > > {S32, S64, {(u32)(s32)S32_MIN, (u32)(s32)-255}, {(u32)(s32)-2, 0}}, > > {S32, S64, {0, 1}, {(u32)(s32)S32_MIN, (u32)(s32)S32_MIN}}, > > {S32, U32, {(u32)(s32)S32_MIN, (u32)(s32)S32_MIN}, {(u32)(s32)S32_MIN, (u32)(s32)S32_MIN}}, > > + > > + /* edge overlap testings for BPF_NE, skipped some cases that already > > + * exist above. > > + */ > > + {U64, U64, {0, U64_MAX}, {U64_MAX, U64_MAX}}, > > + {U64, U64, {0, U64_MAX}, {0, 0}}, > > + {S64, U64, {S64_MIN, 0}, {S64_MIN, S64_MIN}}, > > + {S64, U64, {S64_MIN, 0}, {0, 0}}, > > + {S64, U64, {S64_MIN, S64_MAX}, {S64_MAX, S64_MAX}}, > > + {U32, U32, {0, U32_MAX}, {0, 0}}, > > + {S32, U32, {(u32)(s32)S32_MIN, 0}, {0, 0}}, > > + {S32, U32, {(u32)(s32)S32_MIN, 0}, {(u32)(s32)S32_MIN, (u32)(s32)S32_MIN}}, > > + {S32, U32, {(u32)(s32)S32_MIN, S32_MAX}, {S32_MAX, S32_MAX}}, > > I think you're copying the style of the casts from few lines above, > but (s32)S32_MIN is unnecessary. S32_MIN includes the cast already. > Please remove and fix the above lines too. Enn...yes, I simulated the usage of S32_MIN from the lines above. You are right, the s32 casting is unnecessary, I'll just keep the u32 casting. I'll wait a while before sending the next version to see if someone else any comments on this series. Thanks! Menglong Dong
Powered by blists - more mailing lists