Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Message-ID: <4ec86365668f6c3b4242232506e896a17aa2af4c.camel@gmail.com>
Subject: Re: [PATCH bpf v2 2/2] selftests/bpf: Add test for immediate
 spilled to stack
From:   Eduard Zingerman <eddyz87@gmail.com>
To:     Hao Sun <sunhao.th@gmail.com>, Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        John Fastabend <john.fastabend@gmail.com>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>,
        Yonghong Song <yonghong.song@linux.dev>,
        KP Singh <kpsingh@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>,
        Mykola Lysenko <mykolal@fb.com>, Shuah Khan <shuah@kernel.org>,
        Shung-Hsi Yu <shung-hsi.yu@suse.com>
Cc:     bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org
Date:   Wed, 01 Nov 2023 13:05:33 +0200
In-Reply-To: <20231101-fix-check-stack-write-v2-2-cb7c17b869b0@gmail.com>
References: <20231101-fix-check-stack-write-v2-0-cb7c17b869b0@gmail.com>
         <20231101-fix-check-stack-write-v2-2-cb7c17b869b0@gmail.com>
Autocrypt: addr=eddyz87@gmail.com; prefer-encrypt=mutual; keydata=mQGNBGKNNQEBDACwcUNXZOGTzn4rr7Sd18SA5Wv0Wna/ONE0ZwZEx+sIjyGrPOIhR14/DsOr3ZJer9UJ/WAJwbxOBj6E5Y2iF7grehljNbLr/jMjzPJ+hJpfOEAb5xjCB8xIqDoric1WRcCaRB+tDSk7jcsIIiMish0diTK3qTdu4MB6i/sh4aeFs2nifkNi3LdBuk8Xnk+RJHRoKFJ+C+EoSmQPuDQIRaF9N2m4yO0eG36N8jLwvUXnZzGvHkphoQ9ztbRJp58oh6xT7uH62m98OHbsVgzYKvHyBu/IU2ku5kVG9pLrFp25xfD4YdlMMkJH6l+jk+cpY0cvMTS1b6/g+1fyPM+uzD8Wy+9LtZ4PHwLZX+t4ONb/48i5AKq/jSsb5HWdciLuKEwlMyFAihZamZpEj+9n91NLPX4n7XeThXHaEvaeVVl4hfW/1Qsao7l1YjU/NCHuLaDeH4U1P59bagjwo9d1n5/PESeuD4QJFNqW+zkmE4tmyTZ6bPV6T5xdDRHeiITGc00AEQEAAbQkRWR1YXJkIFppbmdlcm1hbiA8ZWRkeXo4N0BnbWFpbC5jb20+iQHUBBMBCgA+FiEEx+6LrjApQyqnXCYELgxleklgRAkFAmKNNQECGwMFCQPCZwAFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AACgkQLgxleklgRAlWZAv/cJ5v3zlEyP0/jMKQBqbVCCHTirPEw+nqxbkeSO6r2FUds0NnGA9a6NPOpBH+qW7a6+n6q3sIbvH7jlss4pzLI7LYlDC6z+egTv7KR5X1xFrY1uR5UGs1beAjnzYeV2hK4yqRUfygsT0Wk5e4FiNBv4+DUZ8r0cNDkO6swJxU55DO21mcteC147+4aDoHZ40R0tsAu+brDGSSoOPpb0RWVsEf9XOBJqWWA+T7mluw
 nYzhLWGcczc6J71q1Dje0l5vIPaSFOgwmWD4DA+WvuxM/shH4rtWeodbv iCTce6yYIygHgUAtJcHozAlgRrL0jz44cggBTcoeXp/atckXK546OugZPnl00J3qmm5uWAznU6T5YDv2vCvAMEbz69ib+kHtnOSBvR0Jb86UZZqSb4ATfwMOWe9htGTjKMb0QQOLK0mTcrk/TtymaG+T4Fsos0kgrxqjgfrxxEhYcVNW8v8HISmFGFbqsJmFbVtgk68BcU0wgF8oFxo7u+XYQDdKbI1uQGNBGKNNQEBDADbQIdo8L3sdSWGQtu+LnFqCZoAbYurZCmUjLV3df1b+sg+GJZvVTmMZnzDP/ADufcbjopBBjGTRAY4L76T2niu2EpjclMMM3mtrOc738Kr3+RvPjUupdkZ1ZEZaWpf4cZm+4wH5GUfyu5pmD5WXX2i1r9XaUjeVtebvbuXWmWI1ZDTfOkiz/6Z0GDSeQeEqx2PXYBcepU7S9UNWttDtiZ0+IH4DZcvyKPUcK3tOj4u8GvO3RnOrglERzNCM/WhVdG1+vgU9fXO83TB/PcfAsvxYSie7u792s/I+yA4XKKh82PSTvTzg2/4vEDGpI9yubkfXRkQN28w+HKF5qoRB8/L1ZW/brlXkNzA6SveJhCnH7aOF0Yezl6TfX27w1CW5Xmvfi7X33V/SPvo0tY1THrO1c+bOjt5F+2/K3tvejmXMS/I6URwa8n1e767y5ErFKyXAYRweE9zarEgpNZTuSIGNNAqK+SiLLXt51G7P30TVavIeB6s2lCt1QKt62ccLqUAEQEAAYkBvAQYAQoAJhYhBMfui64wKUMqp1wmBC4MZXpJYEQJBQJijTUBAhsMBQkDwmcAAAoJEC4MZXpJYEQJkRAMAKNvWVwtXm/WxWoiLnXyF2WGXKoDe5+itTLvBmKcV/b1OKZF1s90V7WfSBz712eFAynEzyeezPbwU8QBiTpZcHXwQni3IYKvsh7s
 t1iq+gsfnXbPz5AnS598ScZI1oP7OrPSFJkt/z4acEbOQDQs8aUqrd46PV jsdqGvKnXZxzylux29UTNby4jTlz9pNJM+wPrDRmGfchLDUmf6CffaUYCbu4FiId+9+dcTCDvxbABRy1C3OJ8QY7cxfJ+pEZW18fRJ0XCl/fiV/ecAOfB3HsqgTzAn555h0rkFgay0hAvMU/mAW/CFNSIxV397zm749ZNLA0L2dMy1AKuOqH+/B+/ImBfJMDjmdyJQ8WU/OFRuGLdqOd2oZrA1iuPIa+yUYyZkaZfz/emQwpIL1+Q4p1R/OplA4yc301AqruXXUcVDbEB+joHW3hy5FwK5t5OwTKatrSJBkydSF9zdXy98fYzGniRyRA65P0Ix/8J3BYB4edY2/w0Ip/mdYsYQljBY0A==
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.0 
MIME-Version: 1.0
Precedence: bulk

On Wed, 2023-11-01 at 08:33 +0100, Hao Sun wrote:
> Add a test to check if the verifier correctly reason about the sign
> of an immediate spilled to stack by BPF_ST instruction.
>=20
> Signed-off-by: Hao Sun <sunhao.th@gmail.com>
> ---
>  tools/testing/selftests/bpf/verifier/bpf_st_mem.c | 32 +++++++++++++++++=
++++++
>  1 file changed, 32 insertions(+)
>=20
> diff --git a/tools/testing/selftests/bpf/verifier/bpf_st_mem.c b/tools/te=
sting/selftests/bpf/verifier/bpf_st_mem.c
> index 3af2501082b2..0ba23807c46c 100644
> --- a/tools/testing/selftests/bpf/verifier/bpf_st_mem.c
> +++ b/tools/testing/selftests/bpf/verifier/bpf_st_mem.c
> @@ -65,3 +65,35 @@
>  	.expected_attach_type =3D BPF_SK_LOOKUP,
>  	.runs =3D -1,
>  },
> +{
> +	"BPF_ST_MEM stack imm sign",
> +	/* Check if verifier correctly reasons about sign of an
> +	 * immediate spilled to stack by BPF_ST instruction.
> +	 *
> +	 *   fp[-8] =3D -44;
> +	 *   r0 =3D fp[-8];
> +	 *   if r0 s< 0 goto ret0;
> +	 *   r0 =3D -1;
> +	 *   exit;
> +	 * ret0:
> +	 *   r0 =3D 0;
> +	 *   exit;
> +	 */
> +	.insns =3D {
> +	BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, -44),
> +	BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_10, -8),
> +	BPF_JMP_IMM(BPF_JSLT, BPF_REG_0, 0, 2),
> +	BPF_MOV64_IMM(BPF_REG_0, -1),
> +	BPF_EXIT_INSN(),
> +	BPF_MOV64_IMM(BPF_REG_0, 0),
> +	BPF_EXIT_INSN(),
> +	},
> +	/* Use prog type that requires return value in range [0, 1] */
> +	.prog_type =3D BPF_PROG_TYPE_SK_LOOKUP,
> +	.expected_attach_type =3D BPF_SK_LOOKUP,
> +	.result =3D VERBOSE_ACCEPT,
> +	.runs =3D -1,
> +	.errstr =3D "0: (7a) *(u64 *)(r10 -8) =3D -44        ; R10=3Dfp0 fp-8_w=
=3D-44\
> +	2: (c5) if r0 s< 0x0 goto pc+2\
> +	2: R0_w=3D-44",
> +},
>=20

Please note that this test case fails on CI [0], full log below:

2023-11-01T07:49:51.2841702Z #116/p BPF_ST_MEM stack imm sign FAIL
2023-11-01T07:49:51.2843456Z Unexpected verifier log!
2023-11-01T07:49:51.2844968Z EXP: 2: R0_w=3D-44
2023-11-01T07:49:51.2845583Z RES:
2023-11-01T07:49:51.2846693Z func#0 @0
2023-11-01T07:49:51.2848932Z 0: R1=3Dctx(off=3D0,imm=3D0) R10=3Dfp0
2023-11-01T07:49:51.2853045Z 0: (7a) *(u64 *)(r10 -8) =3D -44        ; R10=
=3Dfp0 fp-8_w=3D-44
2023-11-01T07:49:51.2857391Z 1: (79) r0 =3D *(u64 *)(r10 -8)         ; R0_w=
=3D-44 R10=3Dfp0 fp-8_w=3D-44
2023-11-01T07:49:51.2859127Z 2: (c5) if r0 s< 0x0 goto pc+2
2023-11-01T07:49:51.2862943Z mark_precise: frame0: last_idx 2 first_idx 0 s=
ubseq_idx -1=20
2023-11-01T07:49:51.2867511Z mark_precise: frame0: regs=3Dr0 stack=3D befor=
e 1: (79) r0 =3D *(u64 *)(r10 -8)
2023-11-01T07:49:51.2872217Z mark_precise: frame0: regs=3D stack=3D-8 befor=
e 0: (7a) *(u64 *)(r10 -8) =3D -44
2023-11-01T07:49:51.2872816Z 5: R0_w=3D-44
2023-11-01T07:49:51.2875653Z 5: (b7) r0 =3D 0                        ; R0_w=
=3D0
2023-11-01T07:49:51.2876493Z 6: (95) exit

I suspect that after recent logging fixes instruction number printed
after jump changed and that's why test case no longer passes.

Note: you can check CI status for submitted patch-sets using link [1].

[0] https://github.com/kernel-patches/bpf/actions/runs/6717053909/job/18254=
330860
[1] https://patchwork.kernel.org/project/netdevbpf/list/