[PATCH bpf-next v2] selftests/bpf: Fix build of task_pt_regs test for arm64

[PATCH bpf-next v2] selftests/bpf: Fix build of task_pt_regs test for arm64

[Jean-Philippe Brucker]

struct pt_regs is not exported to userspace on all archs. arm64 and s390
export "user_pt_regs" instead, which causes build failure at the moment:

  progs/test_task_pt_regs.c:8:16: error: variable has incomplete type 'struct pt_regs'
  struct pt_regs current_regs = {};

Instead of using pt_regs from ptrace.h, use the larger kernel struct
from vmlinux.h directly. Since the test runner task_pt_regs.c does not
have access to the kernel struct definition, copy it into a char array.

Fixes: 576d47bb1a92 ("bpf: selftests: Add bpf_task_pt_regs() selftest")
Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
v2: Work on struct pt_regs from vmlinux.h
v1: https://lore.kernel.org/bpf/20210902090925.2010528-1-jean-philippe@linaro.org/
---
 .../selftests/bpf/prog_tests/task_pt_regs.c   |  1 -
 .../selftests/bpf/progs/test_task_pt_regs.c   | 19 +++++++++++++------
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c b/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
index 53f0e0fa1a53..37c20b5ffa70 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
+++ b/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #define _GNU_SOURCE
 #include <test_progs.h>
-#include <linux/ptrace.h>
 #include "test_task_pt_regs.skel.h"
 
 void test_task_pt_regs(void)
diff --git a/tools/testing/selftests/bpf/progs/test_task_pt_regs.c b/tools/testing/selftests/bpf/progs/test_task_pt_regs.c
index 6c059f1cfa1b..e6cb09259408 100644
--- a/tools/testing/selftests/bpf/progs/test_task_pt_regs.c
+++ b/tools/testing/selftests/bpf/progs/test_task_pt_regs.c
@@ -1,12 +1,17 @@
 // SPDX-License-Identifier: GPL-2.0
 
-#include <linux/ptrace.h>
-#include <linux/bpf.h>
+#include "vmlinux.h"
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
 
-struct pt_regs current_regs = {};
-struct pt_regs ctx_regs = {};
+#define PT_REGS_SIZE sizeof(struct pt_regs)
+
+/*
+ * The kernel struct pt_regs isn't exported in its entirety to userspace.
+ * Pass it as an array to task_pt_regs.c
+ */
+char current_regs[PT_REGS_SIZE] = {};
+char ctx_regs[PT_REGS_SIZE] = {};
 int uprobe_res = 0;
 
 SEC("uprobe/trigger_func")
@@ -17,8 +22,10 @@ int handle_uprobe(struct pt_regs *ctx)
 
 	current = bpf_get_current_task_btf();
 	regs = (struct pt_regs *) bpf_task_pt_regs(current);
-	__builtin_memcpy(&current_regs, regs, sizeof(*regs));
-	__builtin_memcpy(&ctx_regs, ctx, sizeof(*ctx));
+	if (bpf_probe_read_kernel(current_regs, PT_REGS_SIZE, regs))
+		return 0;
+	if (bpf_probe_read_kernel(ctx_regs, PT_REGS_SIZE, ctx))
+		return 0;
 
 	/* Prove that uprobe was run */
 	uprobe_res = 1;
-- 
2.33.0

Re: [PATCH bpf-next v2] selftests/bpf: Fix build of task_pt_regs test for arm64

[Ilya Leoshkevich]

On Mon, 2021-09-06 at 17:36 +0100, Jean-Philippe Brucker wrote:
> struct pt_regs is not exported to userspace on all archs. arm64 and
> s390
> export "user_pt_regs" instead, which causes build failure at the
> moment:
> 
>   progs/test_task_pt_regs.c:8:16: error: variable has incomplete type
> 'struct pt_regs'
>   struct pt_regs current_regs = {};
> 
> Instead of using pt_regs from ptrace.h, use the larger kernel struct
> from vmlinux.h directly. Since the test runner task_pt_regs.c does not
> have access to the kernel struct definition, copy it into a char array.
> 
> Fixes: 576d47bb1a92 ("bpf: selftests: Add bpf_task_pt_regs() selftest")
> Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> v2: Work on struct pt_regs from vmlinux.h
> v1:
> https://lore.kernel.org/bpf/20210902090925.2010528-1-jean-philippe@linaro.org/
> ---
>  .../selftests/bpf/prog_tests/task_pt_regs.c   |  1 -
>  .../selftests/bpf/progs/test_task_pt_regs.c   | 19 +++++++++++++------
>  2 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
> b/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
> index 53f0e0fa1a53..37c20b5ffa70 100644
> --- a/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
> +++ b/tools/testing/selftests/bpf/prog_tests/task_pt_regs.c
> @@ -1,7 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #define _GNU_SOURCE
>  #include <test_progs.h>
> -#include <linux/ptrace.h>
>  #include "test_task_pt_regs.skel.h"
>  
>  void test_task_pt_regs(void)
> diff --git a/tools/testing/selftests/bpf/progs/test_task_pt_regs.c
> b/tools/testing/selftests/bpf/progs/test_task_pt_regs.c
> index 6c059f1cfa1b..e6cb09259408 100644
> --- a/tools/testing/selftests/bpf/progs/test_task_pt_regs.c
> +++ b/tools/testing/selftests/bpf/progs/test_task_pt_regs.c
> @@ -1,12 +1,17 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
> -#include <linux/ptrace.h>
> -#include <linux/bpf.h>
> +#include "vmlinux.h"
>  #include <bpf/bpf_helpers.h>
>  #include <bpf/bpf_tracing.h>
>  
> -struct pt_regs current_regs = {};
> -struct pt_regs ctx_regs = {};
> +#define PT_REGS_SIZE sizeof(struct pt_regs)
> +
> +/*
> + * The kernel struct pt_regs isn't exported in its entirety to
> userspace.
> + * Pass it as an array to task_pt_regs.c
> + */
> +char current_regs[PT_REGS_SIZE] = {};
> +char ctx_regs[PT_REGS_SIZE] = {};
>  int uprobe_res = 0;
>  
>  SEC("uprobe/trigger_func")
> @@ -17,8 +22,10 @@ int handle_uprobe(struct pt_regs *ctx)
>  
>         current = bpf_get_current_task_btf();
>         regs = (struct pt_regs *) bpf_task_pt_regs(current);
> -       __builtin_memcpy(&current_regs, regs, sizeof(*regs));
> -       __builtin_memcpy(&ctx_regs, ctx, sizeof(*ctx));
> +       if (bpf_probe_read_kernel(current_regs, PT_REGS_SIZE, regs))
> +               return 0;
> +       if (bpf_probe_read_kernel(ctx_regs, PT_REGS_SIZE, ctx))
> +               return 0;
>  
>         /* Prove that uprobe was run */
>         uprobe_res = 1;

I've tested this patch on s390 and it does indeed fix the build issue.
Thanks!

Tested-by: Ilya Leoshkevich <iii@linux.ibm.com>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>

Re: [PATCH bpf-next v2] selftests/bpf: Fix build of task_pt_regs test for arm64

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf.git (refs/heads/master):

On Mon,  6 Sep 2021 17:36:38 +0100 you wrote:
> struct pt_regs is not exported to userspace on all archs. arm64 and s390
> export "user_pt_regs" instead, which causes build failure at the moment:
> 
>   progs/test_task_pt_regs.c:8:16: error: variable has incomplete type 'struct pt_regs'
>   struct pt_regs current_regs = {};
> 
> Instead of using pt_regs from ptrace.h, use the larger kernel struct
> from vmlinux.h directly. Since the test runner task_pt_regs.c does not
> have access to the kernel struct definition, copy it into a char array.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2] selftests/bpf: Fix build of task_pt_regs test for arm64
    https://git.kernel.org/bpf/bpf/c/3a029e1f3d6e

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html