selftests/bpf: Fix dynptr/test_dynptr_is_null
authorYonghong Song <yhs@fb.com>
Wed, 17 May 2023 04:04:04 +0000 (21:04 -0700)
committerDaniel Borkmann <daniel@iogearbox.net>
Wed, 17 May 2023 14:52:26 +0000 (16:52 +0200)
With latest llvm17, dynptr/test_dynptr_is_null subtest failed in my testing
VM. The failure log looks like below:

  All error logs:
  tester_init:PASS:tester_log_buf 0 nsec
  process_subtest:PASS:obj_open_mem 0 nsec
  process_subtest:PASS:Can't alloc specs array 0 nsec
  verify_success:PASS:dynptr_success__open 0 nsec
  verify_success:PASS:bpf_object__find_program_by_name 0 nsec
  verify_success:PASS:dynptr_success__load 0 nsec
  verify_success:PASS:bpf_program__attach 0 nsec
  verify_success:FAIL:err unexpected err: actual 4 != expected 0
  #65/9    dynptr/test_dynptr_is_null:FAIL

The error happens for bpf prog test_dynptr_is_null in dynptr_success.c:

        if (bpf_dynptr_is_null(&ptr2)) {
                err = 4;
                goto exit;
        }

The bpf_dynptr_is_null(&ptr) unexpectedly returned a non-zero value and
the control went to the error path. Digging further, I found the root cause
is due to function signature difference between kernel and user space.

In kernel, we have ...

  __bpf_kfunc bool bpf_dynptr_is_null(struct bpf_dynptr_kern *ptr)

... while in bpf_kfuncs.h we have:

  extern int bpf_dynptr_is_null(const struct bpf_dynptr *ptr) __ksym;

The kernel bpf_dynptr_is_null disasm code:

  ffffffff812f1a90 <bpf_dynptr_is_null>:
  ffffffff812f1a90: f3 0f 1e fa           endbr64
  ffffffff812f1a94: 0f 1f 44 00 00        nopl    (%rax,%rax)
  ffffffff812f1a99: 53                    pushq   %rbx
  ffffffff812f1a9a: 48 89 fb              movq    %rdi, %rbx
  ffffffff812f1a9d: e8 ae 29 17 00        callq   0xffffffff81464450 <__asan_load8_noabort>
  ffffffff812f1aa2: 48 83 3b 00           cmpq    $0x0, (%rbx)
  ffffffff812f1aa6: 0f 94 c0              sete    %al
  ffffffff812f1aa9: 5b                    popq    %rbx
  ffffffff812f1aaa: c3                    retq

Note that only 1-byte register %al is set and the other 7-bytes are not
touched. In bpf program, the asm code for the above bpf_dynptr_is_null(&ptr2):

       266:       85 10 00 00 ff ff ff ff call -0x1
       267:       b4 01 00 00 04 00 00 00 w1 = 0x4
       268:       16 00 03 00 00 00 00 00 if w0 == 0x0 goto +0x3 <LBB9_8>

Basically, 4-byte subregister is tested. This might cause error as the value
other than the lowest byte might not be 0.

This patch fixed the issue by using the identical func prototype across kernel
and selftest user space. The fixed bpf asm code:

       267:       85 10 00 00 ff ff ff ff call -0x1
       268:       54 00 00 00 01 00 00 00 w0 &= 0x1
       269:       b4 01 00 00 04 00 00 00 w1 = 0x4
       270:       16 00 03 00 00 00 00 00 if w0 == 0x0 goto +0x3 <LBB9_8>

Signed-off-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20230517040404.4023912-1-yhs@fb.com
tools/testing/selftests/bpf/bpf_kfuncs.h
tools/testing/selftests/bpf/progs/dynptr_fail.c
tools/testing/selftests/bpf/progs/dynptr_success.c
tools/testing/selftests/bpf/progs/test_xdp_dynptr.c

index f3c41f8902a04766a002a5c7598e3937cf8ac2bb..821c25b7d0df3e6586b5d6cc441766696b0c6e1f 100644 (file)
@@ -36,7 +36,7 @@ extern void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr *ptr, __u32 offset,
                              void *buffer, __u32 buffer__szk) __ksym;
 
 extern int bpf_dynptr_adjust(const struct bpf_dynptr *ptr, __u32 start, __u32 end) __ksym;
-extern int bpf_dynptr_is_null(const struct bpf_dynptr *ptr) __ksym;
+extern bool bpf_dynptr_is_null(const struct bpf_dynptr *ptr) __ksym;
 extern int bpf_dynptr_is_rdonly(const struct bpf_dynptr *ptr) __ksym;
 extern __u32 bpf_dynptr_size(const struct bpf_dynptr *ptr) __ksym;
 extern int bpf_dynptr_clone(const struct bpf_dynptr *ptr, struct bpf_dynptr *clone__init) __ksym;
index c2f0e18af95169f501e8c119c531684d146bef0c..7ce7e827d5f01c19dc4ab9bb800deb7b35897cab 100644 (file)
@@ -3,6 +3,7 @@
 
 #include <errno.h>
 #include <string.h>
+#include <stdbool.h>
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 #include <linux/if_ether.h>
index 0c053976f8f9af99f5f39da3b7b369847fdead0d..5985920d162e7be608c4412c855dcf6520672cb5 100644 (file)
@@ -2,6 +2,7 @@
 /* Copyright (c) 2022 Facebook */
 
 #include <string.h>
+#include <stdbool.h>
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
index 25ee4a22e48d94c648363490193102216a66dc78..78c368e71797287c8fb1c88e43576ba9d99a58b8 100644 (file)
@@ -2,6 +2,7 @@
 /* Copyright (c) 2022 Meta */
 #include <stddef.h>
 #include <string.h>
+#include <stdbool.h>
 #include <linux/bpf.h>
 #include <linux/if_ether.h>
 #include <linux/if_packet.h>