Re: [PATCH bpf-next 2/2] selftests/bpf: Add verifier test exercising jit PROBE_MEM logic

[Yonghong Song <yhs@meta.com>]

On 12/13/22 10:27 AM, Dave Marchevsky wrote:
> This patch adds a test exercising logic that was fixed / improved in
> the previous patch in the series, as well as general sanity checking for
> jit's PROBE_MEM logic which should've been unaffected by the previous
> patch.
> 
> The added verifier test does the following:
>    * Acquire a referenced kptr to struct prog_test_ref_kfunc using
>      existing net/bpf/test_run.c kfunc
>      * Helper returns ptr to a specific prog_test_ref_kfunc whose first
>        two fields - both ints - have been prepopulated w/ vals 42 and
>        108, respectively
>    * kptr_xchg the acquired ptr into an arraymap
>    * Do a direct map_value load of the just-added ptr
>      * Goal of all this setup is to get an unreferenced kptr pointing to
>        struct with ints of known value, which is the result of this step
>    * Using unreferenced kptr obtained in previous step, do loads of
>      prog_test_ref_kfunc.a (offset 0) and .b (offset 4)
>    * Then incr the kptr by 8 and load prog_test_ref_kfunc.a again (this
>      time at offset -8)
>    * Add all the loaded ints together and return
> 
> Before the PROBE_MEM fixes in previous patch, the loads at offset 0 and
> 4 would succeed, while the load at offset -8 would incorrectly fail
> runtime check emitted by the JIT and 0 out dst reg as a result. This
> confirmed by retval of 150 for this test before previous patch - since
> second .a read is 0'd out - and a retval of 192 with the fixed logic.
> 
> The test exercises the two optimizations to fixed logic added in last
> patch as well:
>    * BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_0, 0) exercises "insn->off is
>      0, no need to add / sub from src_reg" optimization
>    * BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_0, -8) exercises "src_reg ==
>      dst_reg, no need to restore src_reg after load" optimization
> 
> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>

The test is quite complicated. Is it possible we could write a C code
with every small portion of asm to test jit functionality. For
b = p->a, the asm will simulate like below
	p += offsetof(p_type, a)
	b = *(u32 *)(p - offsetof(p_type, a))
Could the above be a little bit simpler and easy to understand?
I think you might be able to piggy back with some existing selftests.

> ---
>   tools/testing/selftests/bpf/test_verifier.c | 75 ++++++++++++++----
>   tools/testing/selftests/bpf/verifier/jit.c  | 84 +++++++++++++++++++++
>   2 files changed, 146 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index 8c808551dfd7..14f8d0231e3c 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -55,7 +55,7 @@
>   #define MAX_UNEXPECTED_INSNS	32
>   #define MAX_TEST_INSNS	1000000
>   #define MAX_FIXUPS	8
> -#define MAX_NR_MAPS	23
> +#define MAX_NR_MAPS	24
>   #define MAX_TEST_RUNS	8
>   #define POINTER_VALUE	0xcafe4all
>   #define TEST_DATA_LEN	64
> @@ -131,6 +131,7 @@ struct bpf_test {
>   	int fixup_map_ringbuf[MAX_FIXUPS];
>   	int fixup_map_timer[MAX_FIXUPS];
>   	int fixup_map_kptr[MAX_FIXUPS];
> +	int fixup_map_probe_mem_read[MAX_FIXUPS];
>   	struct kfunc_btf_id_pair fixup_kfunc_btf_id[MAX_FIXUPS];
>   	/* Expected verifier log output for result REJECT or VERBOSE_ACCEPT.
>   	 * Can be a tab-separated sequence of expected strings. An empty string
[...]