On Fri, Oct 20, 2023 at 7:48 AM Kumar Kartikeya Dwivedi
<memxor@xxxxxxxxx> wrote:
>
> The linked list failure test 'pop_front_off' and 'pop_back_off'
> currently rely on matching exact instruction and register values. The
> purpose of the test is to ensure the offset is correctly incremented for
> the returned pointers from list pop helpers, which can then be used with
> container_of to obtain the real object. Hence, somehow obtaining the
> information that the offset is 48 will work for us. Make the test more
> robust by relying on verifier error string of bpf_spin_lock and remove
> dependence on fragile instruction index or register number, which can be
> affected by different clang versions used to build the selftests.
>
> Fixes: 300f19dcdb99 ("selftests/bpf: Add BPF linked list API tests")
> Reported-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> ---
> tools/testing/selftests/bpf/prog_tests/linked_list.c | 10 ++--------
> tools/testing/selftests/bpf/progs/linked_list_fail.c | 4 +++-
> 2 files changed, 5 insertions(+), 9 deletions(-)
>
Thanks for the fix! Applied to bpf-next.
> diff --git a/tools/testing/selftests/bpf/prog_tests/linked_list.c b/tools/testing/selftests/bpf/prog_tests/linked_list.c
> index 69dc31383b78..2fb89de63bd2 100644
> --- a/tools/testing/selftests/bpf/prog_tests/linked_list.c
> +++ b/tools/testing/selftests/bpf/prog_tests/linked_list.c
> @@ -94,14 +94,8 @@ static struct {
> { "incorrect_head_var_off2", "variable ptr_ access var_off=(0x0; 0xffffffff) disallowed" },
> { "incorrect_head_off1", "bpf_list_head not found at offset=25" },
> { "incorrect_head_off2", "bpf_list_head not found at offset=1" },
> - { "pop_front_off",
> - "15: (bf) r1 = r6 ; R1_w=ptr_or_null_foo(id=4,ref_obj_id=4,off=48,imm=0) "
> - "R6_w=ptr_or_null_foo(id=4,ref_obj_id=4,off=48,imm=0) refs=2,4\n"
> - "16: (85) call bpf_this_cpu_ptr#154\nR1 type=ptr_or_null_ expected=percpu_ptr_" },
> - { "pop_back_off",
> - "15: (bf) r1 = r6 ; R1_w=ptr_or_null_foo(id=4,ref_obj_id=4,off=48,imm=0) "
> - "R6_w=ptr_or_null_foo(id=4,ref_obj_id=4,off=48,imm=0) refs=2,4\n"
> - "16: (85) call bpf_this_cpu_ptr#154\nR1 type=ptr_or_null_ expected=percpu_ptr_" },
> + { "pop_front_off", "off 48 doesn't point to 'struct bpf_spin_lock' that is at 40" },
> + { "pop_back_off", "off 48 doesn't point to 'struct bpf_spin_lock' that is at 40" },
> };
>
> static void test_linked_list_fail_prog(const char *prog_name, const char *err_msg)
> diff --git a/tools/testing/selftests/bpf/progs/linked_list_fail.c b/tools/testing/selftests/bpf/progs/linked_list_fail.c
> index f4c63daba229..6438982b928b 100644
> --- a/tools/testing/selftests/bpf/progs/linked_list_fail.c
> +++ b/tools/testing/selftests/bpf/progs/linked_list_fail.c
> @@ -591,7 +591,9 @@ int pop_ptr_off(void *(*op)(void *head))
> n = op(&p->head);
> bpf_spin_unlock(&p->lock);
>
> - bpf_this_cpu_ptr(n);
> + if (!n)
> + return 0;
> + bpf_spin_lock((void *)n);
> return 0;
> }
>
> --
> 2.40.1
>
