[PATCH bpf-next v2 0/2] Fix test_probe_user on s390x

[PATCH bpf-next v2 0/2] Fix test_probe_user on s390x

[Ilya Leoshkevich]

Hi,

This is a fix for [1]: test_probe_user fails on s390x, because it hooks
only connect(), but not socketcall(SYS_CONNECT).

Patch 1 adds this quirk to BPF_KSYSCALL documentation.
Patch 2 fixes the test by attaching a prog to socketcall().

Best regards,
Ilya

[1] https://lore.kernel.org/bpf/06631b122b9bd6258139a36b971bba3e79543503.camel@linux.ibm.com/

v1: https://lore.kernel.org/bpf/20220723020344.21699-1-iii@linux.ibm.com/
v1 -> v2: Add CONFIG_ prefix to CLONE_BACKWARDS* symbols (Jiri).
          Change the type of prog_names to make checkpatch happy.
          Use prog_count everywhere (Jiri).
          #ifdef out handle_sys_socketcall() on non-s390x (Jiri).

Ilya Leoshkevich (2):
  libbpf: Extend BPF_KSYSCALL documentation
  selftests/bpf: Attach to socketcall() in test_probe_user

 tools/lib/bpf/bpf_tracing.h                   | 15 +++++---
 .../selftests/bpf/prog_tests/probe_user.c     | 35 +++++++++++++------
 .../selftests/bpf/progs/test_probe_user.c     | 32 +++++++++++++++--
 3 files changed, 65 insertions(+), 17 deletions(-)

-- 
2.35.3

[PATCH bpf-next v2 1/2] libbpf: Extend BPF_KSYSCALL documentation

[Ilya Leoshkevich]

Explicitly list known quirks.
Mention that socket-related syscalls can be invoked via socketcall().

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tools/lib/bpf/bpf_tracing.h | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index f4d3e1e2abe2..43ca3aff2292 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -523,10 +523,17 @@ static __always_inline typeof(name(0)) ____##name(struct pt_regs *ctx, ##args)
  * Original struct pt_regs * context is preserved as 'ctx' argument. This might
  * be necessary when using BPF helpers like bpf_perf_event_output().
  *
- * At the moment BPF_KSYSCALL does not handle all the calling convention
- * quirks for mmap(), clone() and compat syscalls transparrently. This may or
- * may not change in the future. User needs to take extra measures to handle
- * such quirks explicitly, if necessary.
+ * At the moment BPF_KSYSCALL does not transparently handle all the calling
+ * convention quirks for the following syscalls:
+ *
+ * - mmap(): __ARCH_WANT_SYS_OLD_MMAP.
+ * - clone(): CONFIG_CLONE_BACKWARDS, CONFIG_CLONE_BACKWARDS2 and
+ *            CONFIG_CLONE_BACKWARDS3.
+ * - socket-related syscalls: __ARCH_WANT_SYS_SOCKETCALL.
+ * - compat syscalls.
+ *
+ * This may or may not change in the future. User needs to take extra measures
+ * to handle such quirks explicitly, if necessary.
  *
  * This macro relies on BPF CO-RE support and virtual __kconfig externs.
  */
-- 
2.35.3

[PATCH bpf-next v2 2/2] selftests/bpf: Attach to socketcall() in test_probe_user

[Ilya Leoshkevich]

test_probe_user fails on architectures where libc uses
socketcall(SYS_CONNECT) instead of connect(). Fix by attaching to
socketcall as well.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 .../selftests/bpf/prog_tests/probe_user.c     | 35 +++++++++++++------
 .../selftests/bpf/progs/test_probe_user.c     | 32 +++++++++++++++--
 2 files changed, 54 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/probe_user.c b/tools/testing/selftests/bpf/prog_tests/probe_user.c
index abf890d066eb..34dbd2adc157 100644
--- a/tools/testing/selftests/bpf/prog_tests/probe_user.c
+++ b/tools/testing/selftests/bpf/prog_tests/probe_user.c
@@ -4,25 +4,35 @@
 /* TODO: corrupts other tests uses connect() */
 void serial_test_probe_user(void)
 {
-	const char *prog_name = "handle_sys_connect";
+	static const char *const prog_names[] = {
+		"handle_sys_connect",
+#if defined(__s390x__)
+		"handle_sys_socketcall",
+#endif
+	};
+	enum { prog_count = ARRAY_SIZE(prog_names) };
 	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;
 	struct sockaddr curr, orig, tmp;
 	struct sockaddr_in *in = (struct sockaddr_in *)&curr;
-	struct bpf_link *kprobe_link = NULL;
-	struct bpf_program *kprobe_prog;
+	struct bpf_link *kprobe_links[prog_count] = {};
+	struct bpf_program *kprobe_progs[prog_count];
 	struct bpf_object *obj;
 	static const int zero = 0;
+	size_t i;
 
 	obj = bpf_object__open_file(obj_file, &opts);
 	if (!ASSERT_OK_PTR(obj, "obj_open_file"))
 		return;
 
-	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;
+	for (i = 0; i < prog_count; i++) {
+		kprobe_progs[i] =
+			bpf_object__find_program_by_name(obj, prog_names[i]);
+		if (CHECK(!kprobe_progs[i], "find_probe",
+			  "prog '%s' not found\n", prog_names[i]))
+			goto cleanup;
+	}
 
 	err = bpf_object__load(obj);
 	if (CHECK(err, "obj_load", "err %d\n", err))
@@ -33,9 +43,11 @@ void serial_test_probe_user(void)
 		  "err %d\n", results_map_fd))
 		goto cleanup;
 
-	kprobe_link = bpf_program__attach(kprobe_prog);
-	if (!ASSERT_OK_PTR(kprobe_link, "attach_kprobe"))
-		goto cleanup;
+	for (i = 0; i < prog_count; i++) {
+		kprobe_links[i] = bpf_program__attach(kprobe_progs[i]);
+		if (!ASSERT_OK_PTR(kprobe_links[i], "attach_kprobe"))
+			goto cleanup;
+	}
 
 	memset(&curr, 0, sizeof(curr));
 	in->sin_family = AF_INET;
@@ -69,6 +81,7 @@ void serial_test_probe_user(void)
 		  inet_ntoa(in->sin_addr), ntohs(in->sin_port)))
 		goto cleanup;
 cleanup:
-	bpf_link__destroy(kprobe_link);
+	for (i = 0; i < prog_count; i++)
+		bpf_link__destroy(kprobe_links[i]);
 	bpf_object__close(obj);
 }
diff --git a/tools/testing/selftests/bpf/progs/test_probe_user.c b/tools/testing/selftests/bpf/progs/test_probe_user.c
index 8e1495008e4d..476e310f8a0a 100644
--- a/tools/testing/selftests/bpf/progs/test_probe_user.c
+++ b/tools/testing/selftests/bpf/progs/test_probe_user.c
@@ -7,8 +7,7 @@
 
 static struct sockaddr_in old;
 
-SEC("ksyscall/connect")
-int BPF_KSYSCALL(handle_sys_connect, int fd, struct sockaddr_in *uservaddr, int addrlen)
+static int handle_sys_connect_common(struct sockaddr_in *uservaddr)
 {
 	struct sockaddr_in new;
 
@@ -19,4 +18,33 @@ int BPF_KSYSCALL(handle_sys_connect, int fd, struct sockaddr_in *uservaddr, int
 	return 0;
 }
 
+SEC("ksyscall/connect")
+int BPF_KSYSCALL(handle_sys_connect, int fd, struct sockaddr_in *uservaddr,
+		 int addrlen)
+{
+	return handle_sys_connect_common(uservaddr);
+}
+
+#if defined(bpf_target_s390)
+
+#ifndef SYS_CONNECT
+#define SYS_CONNECT 3
+#endif
+
+SEC("ksyscall/socketcall")
+int BPF_KSYSCALL(handle_sys_socketcall, int call, unsigned long *args)
+{
+	if (call == SYS_CONNECT) {
+		struct sockaddr_in *uservaddr;
+
+		bpf_probe_read_user(&uservaddr, sizeof(uservaddr), &args[1]);
+
+		return handle_sys_connect_common(uservaddr);
+	}
+
+	return 0;
+}
+
+#endif
+
 char _license[] SEC("license") = "GPL";
-- 
2.35.3

Re: [PATCH bpf-next v2 0/2] Fix test_probe_user on s390x

[Jiri Olsa]

On Tue, Jul 26, 2022 at 03:40:06PM +0200, Ilya Leoshkevich wrote:
> Hi,
> 
> This is a fix for [1]: test_probe_user fails on s390x, because it hooks
> only connect(), but not socketcall(SYS_CONNECT).
> 
> Patch 1 adds this quirk to BPF_KSYSCALL documentation.
> Patch 2 fixes the test by attaching a prog to socketcall().
> 
> Best regards,
> Ilya
> 
> [1] https://lore.kernel.org/bpf/06631b122b9bd6258139a36b971bba3e79543503.camel@linux.ibm.com/
> 
> v1: https://lore.kernel.org/bpf/20220723020344.21699-1-iii@linux.ibm.com/
> v1 -> v2: Add CONFIG_ prefix to CLONE_BACKWARDS* symbols (Jiri).
>           Change the type of prog_names to make checkpatch happy.
>           Use prog_count everywhere (Jiri).
>           #ifdef out handle_sys_socketcall() on non-s390x (Jiri).

LGTM 

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> 
> Ilya Leoshkevich (2):
>   libbpf: Extend BPF_KSYSCALL documentation
>   selftests/bpf: Attach to socketcall() in test_probe_user
> 
>  tools/lib/bpf/bpf_tracing.h                   | 15 +++++---
>  .../selftests/bpf/prog_tests/probe_user.c     | 35 +++++++++++++------
>  .../selftests/bpf/progs/test_probe_user.c     | 32 +++++++++++++++--
>  3 files changed, 65 insertions(+), 17 deletions(-)
> 
> -- 
> 2.35.3
> 

Re: [PATCH bpf-next v2 0/2] Fix test_probe_user on s390x

[Daniel Borkmann]

On 7/26/22 4:14 PM, Jiri Olsa wrote:
> On Tue, Jul 26, 2022 at 03:40:06PM +0200, Ilya Leoshkevich wrote:
>> Hi,
>>
>> This is a fix for [1]: test_probe_user fails on s390x, because it hooks
>> only connect(), but not socketcall(SYS_CONNECT).
>>
>> Patch 1 adds this quirk to BPF_KSYSCALL documentation.
>> Patch 2 fixes the test by attaching a prog to socketcall().
>>
>> Best regards,
>> Ilya
>>
>> [1] https://lore.kernel.org/bpf/06631b122b9bd6258139a36b971bba3e79543503.camel@linux.ibm.com/
>>
>> v1: https://lore.kernel.org/bpf/20220723020344.21699-1-iii@linux.ibm.com/
>> v1 -> v2: Add CONFIG_ prefix to CLONE_BACKWARDS* symbols (Jiri).
>>            Change the type of prog_names to make checkpatch happy.
>>            Use prog_count everywhere (Jiri).
>>            #ifdef out handle_sys_socketcall() on non-s390x (Jiri).
> 
> LGTM
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> 

Applied, thanks Ilya!