diff mbox series

[bpf-next,v2] selftests/bpf: fix probe_user test failure with clang build kernel

Message ID 20210929033000.3711921-1-yhs@fb.com (mailing list archive)
State Accepted
Delegated to: BPF
Headers show
Series [bpf-next,v2] selftests/bpf: fix probe_user test failure with clang build kernel | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 11 maintainers not CCed: nathan@kernel.org kpsingh@kernel.org shuah@kernel.org kafai@fb.com netdev@vger.kernel.org linux-kselftest@vger.kernel.org llvm@lists.linux.dev john.fastabend@gmail.com toke@redhat.com ndesaulniers@google.com songliubraving@fb.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 55 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next success VM_Test

Commit Message

Yonghong Song Sept. 29, 2021, 3:30 a.m. UTC
clang build kernel failed the selftest probe_user.
  $ ./test_progs -t probe_user
  $ ...
  $ test_probe_user:PASS:get_kprobe_res 0 nsec
  $ test_probe_user:FAIL:check_kprobe_res wrong kprobe res from probe read: 0.0.0.0:0
  $ #94 probe_user:FAIL

The test attached to kernel function __sys_connect(). In net/socket.c, we have
  int __sys_connect(int fd, struct sockaddr __user *uservaddr, int addrlen)
  {
        ......
  }
  ...
  SYSCALL_DEFINE3(connect, int, fd, struct sockaddr __user *, uservaddr,
                  int, addrlen)
  {
        return __sys_connect(fd, uservaddr, addrlen);
  }

The gcc compiler (8.5.0) does not inline __sys_connect() in syscall entry
function. But latest clang trunk did the inlining. So the bpf program
is not triggered.

To make the test more reliable, let us kprobe the syscall entry function
instead. Note that x86_64, arm64 and s390 have syscall wrappers and they have
to be handled specially.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 .../selftests/bpf/prog_tests/probe_user.c     |  4 +--
 .../selftests/bpf/progs/test_probe_user.c     | 28 +++++++++++++++++--
 2 files changed, 28 insertions(+), 4 deletions(-)

Comments

Andrii Nakryiko Sept. 29, 2021, 6:23 a.m. UTC | #1
On Tue, Sep 28, 2021 at 8:30 PM Yonghong Song <yhs@fb.com> wrote:
>
> clang build kernel failed the selftest probe_user.
>   $ ./test_progs -t probe_user
>   $ ...
>   $ test_probe_user:PASS:get_kprobe_res 0 nsec
>   $ test_probe_user:FAIL:check_kprobe_res wrong kprobe res from probe read: 0.0.0.0:0
>   $ #94 probe_user:FAIL
>
> The test attached to kernel function __sys_connect(). In net/socket.c, we have
>   int __sys_connect(int fd, struct sockaddr __user *uservaddr, int addrlen)
>   {
>         ......
>   }
>   ...
>   SYSCALL_DEFINE3(connect, int, fd, struct sockaddr __user *, uservaddr,
>                   int, addrlen)
>   {
>         return __sys_connect(fd, uservaddr, addrlen);
>   }
>
> The gcc compiler (8.5.0) does not inline __sys_connect() in syscall entry
> function. But latest clang trunk did the inlining. So the bpf program
> is not triggered.
>
> To make the test more reliable, let us kprobe the syscall entry function
> instead. Note that x86_64, arm64 and s390 have syscall wrappers and they have
> to be handled specially.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---

Applied to bpf-next, thanks.

>  .../selftests/bpf/prog_tests/probe_user.c     |  4 +--
>  .../selftests/bpf/progs/test_probe_user.c     | 28 +++++++++++++++++--
>  2 files changed, 28 insertions(+), 4 deletions(-)
>

[...]
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/probe_user.c b/tools/testing/selftests/bpf/prog_tests/probe_user.c
index 95bd12097358..52fe157e2a90 100644
--- a/tools/testing/selftests/bpf/prog_tests/probe_user.c
+++ b/tools/testing/selftests/bpf/prog_tests/probe_user.c
@@ -3,7 +3,7 @@ 
 
 void test_probe_user(void)
 {
-	const char *prog_name = "kprobe/__sys_connect";
+	const char *prog_name = "handle_sys_connect";
 	const char *obj_file = "./test_probe_user.o";
 	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts, );
 	int err, results_map_fd, sock_fd, duration = 0;
@@ -18,7 +18,7 @@  void test_probe_user(void)
 	if (!ASSERT_OK_PTR(obj, "obj_open_file"))
 		return;
 
-	kprobe_prog = bpf_object__find_program_by_title(obj, prog_name);
+	kprobe_prog = bpf_object__find_program_by_name(obj, prog_name);
 	if (CHECK(!kprobe_prog, "find_probe",
 		  "prog '%s' not found\n", prog_name))
 		goto cleanup;
diff --git a/tools/testing/selftests/bpf/progs/test_probe_user.c b/tools/testing/selftests/bpf/progs/test_probe_user.c
index 89b3532ccc75..8812a90da4eb 100644
--- a/tools/testing/selftests/bpf/progs/test_probe_user.c
+++ b/tools/testing/selftests/bpf/progs/test_probe_user.c
@@ -8,13 +8,37 @@ 
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
 
+#if defined(__TARGET_ARCH_x86)
+#define SYSCALL_WRAPPER 1
+#define SYS_PREFIX "__x64_"
+#elif defined(__TARGET_ARCH_s390)
+#define SYSCALL_WRAPPER 1
+#define SYS_PREFIX "__s390x_"
+#elif defined(__TARGET_ARCH_arm64)
+#define SYSCALL_WRAPPER 1
+#define SYS_PREFIX "__arm64_"
+#else
+#define SYSCALL_WRAPPER 0
+#define SYS_PREFIX ""
+#endif
+
 static struct sockaddr_in old;
 
-SEC("kprobe/__sys_connect")
+SEC("kprobe/" SYS_PREFIX "sys_connect")
 int BPF_KPROBE(handle_sys_connect)
 {
-	void *ptr = (void *)PT_REGS_PARM2(ctx);
+#if SYSCALL_WRAPPER == 1
+	struct pt_regs *real_regs;
+#endif
 	struct sockaddr_in new;
+	void *ptr;
+
+#if SYSCALL_WRAPPER == 0
+	ptr = (void *)PT_REGS_PARM2(ctx);
+#else
+	real_regs = (struct pt_regs *)PT_REGS_PARM1(ctx);
+	bpf_probe_read_kernel(&ptr, sizeof(ptr), &PT_REGS_PARM2(real_regs));
+#endif
 
 	bpf_probe_read_user(&old, sizeof(old), ptr);
 	__builtin_memset(&new, 0xab, sizeof(new));