netdev - Re: [PATCH net 2/2] selftests/bpf: Add a test for shifts of values that might be negative

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20171205194018.ta563pgfiaviq5lg@ast-mbp.dhcp.thefacebook.com>
Date:   Tue, 5 Dec 2017 11:40:20 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Edward Cree <ecree@...arflare.com>
Cc:     davem <davem@...emloft.net>, netdev@...r.kernel.org,
        Jann Horn <jannh@...gle.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [PATCH net 2/2] selftests/bpf: Add a test for shifts of values
 that might be negative

On Tue, Dec 05, 2017 at 07:15:57PM +0000, Edward Cree wrote:
> Signed-off-by: Edward Cree <ecree@...arflare.com>
> ---
>  tools/testing/selftests/bpf/test_align.c | 39 ++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/test_align.c b/tools/testing/selftests/bpf/test_align.c
> index 8591c89c0828..24c6757b4c51 100644
> --- a/tools/testing/selftests/bpf/test_align.c
> +++ b/tools/testing/selftests/bpf/test_align.c
> @@ -601,6 +601,45 @@ static struct bpf_align_test tests[] = {
>  			{20, "R5=pkt(id=2,off=0,r=4,umin_value=2,umax_value=1082,var_off=(0x2; 0x7fc))"},
>  		},
>  	},
> +	{
> +		.descr = "unknown shift negative",
> +		/* This isn't really a test of the alignment code, rather of the
> +		 * signed min/max value handling, but it makes use of the
> +		 * register-state-extracting code in do_test_single(), which
> +		 * test_verifier.c doesn't have.
> +		 */
> +		.insns = {
> +			LOAD_UNKNOWN(BPF_REG_3),
> +			BPF_ALU64_IMM(BPF_SUB, BPF_REG_3, 0xff),
> +			BPF_ALU64_IMM(BPF_LSH, BPF_REG_3, 1),
> +			LOAD_UNKNOWN(BPF_REG_4),
> +			BPF_ALU64_IMM(BPF_SUB, BPF_REG_4, 0xff),
> +			BPF_MOV64_REG(BPF_REG_5, BPF_REG_4),
> +			BPF_ALU64_IMM(BPF_RSH, BPF_REG_4, 1),
> +			BPF_ALU64_IMM(BPF_SUB, BPF_REG_5, 1),
> +			BPF_ALU64_IMM(BPF_RSH, BPF_REG_5, 1),
> +			BPF_MOV64_IMM(BPF_REG_0, 0),
> +			BPF_EXIT_INSN(),
> +		},
> +		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
> +		.matches = {
> +			{7, "R0=pkt(id=0,off=8,r=8,imm=0)"},
> +			{7, "R3=inv(id=0,umax_value=255,var_off=(0x0; 0xff))"},
> +			{8, "R3=inv(id=0,smin_value=-255,smax_value=0)"},
> +			/* All the verifier knows is, it's even.  While we could
> +			 * conclude something tighter (the sign bit does not
> +			 * change), the verifier doesn't bother right now.
> +			 */
> +			{9, "R3=inv(id=0,smax_value=9223372036854775806,umax_value=18446744073709551614,var_off=(0x0; 0xfffffffffffffffe))"},
> +			{16, "R3=pkt_end(id=0,off=0,imm=0)"},
> +			{16, "R4=inv(id=0,umax_value=255,var_off=(0x0; 0xff))"},
> +			{17, "R4=inv(id=0,smin_value=-255,smax_value=0)"},
> +			/* both 0 and 0x7f...fff are possible */
> +			{19, "R4=inv(id=0,umax_value=9223372036854775807,var_off=(0x0; 0x7fffffffffffffff))"},
> +			{20, "R5=inv(id=0,umin_value=18446744073709551360,var_off=(0xffffffffffffff00; 0xff))"},
> +			{21, "R5=inv(id=0,umin_value=9223372036854775680,umax_value=9223372036854775807,var_off=(0x7fffffffffffff80; 0x7f))"},

hmm. it doesn't quite look right here and in this form it
already conflicts with net-next.
I would prefer to take only patch 1 into bpf->net and once
bpf->net->linus->net-next merge happens to add the test there.