diff mbox series

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

Message ID 20210906163635.302307-1-jean-philippe@linaro.org
State Accepted
Commit 3a029e1f3d6e2ee809e85abecce619a48016bd4b
Headers show
Series [bpf-next,v2] selftests/bpf: Fix build of task_pt_regs test for arm64 | expand

Commit Message

Jean-Philippe Brucker Sept. 6, 2021, 4:36 p.m. UTC
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(-)

-- 
2.33.0

Comments

Ilya Leoshkevich Sept. 7, 2021, 10:26 a.m. UTC | #1
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>
patchwork-bot+netdevbpf@kernel.org Sept. 7, 2021, 3:30 p.m. UTC | #2
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
diff mbox series

Patch

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;