[PATCH bpf-next 0/2] selftests/bpf: port test_ftrace.sh into test_progs

[PATCH bpf-next 0/2] selftests/bpf: port test_ftrace.sh into test_progs

[Alexis Lothoré (eBPF Foundation)]

Hello,
this small series aims to port the test_ftrace.sh script (testing that
trampolines and function graph behave correctly when both are enabled)
into the test_progs framework

- commit 1 brings a few new helpers to manipulate ftrace from test_progs
- commit 2 performs the actual porting

Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
Alexis Lothoré (eBPF Foundation) (2):
      selftests/bpf: add helpers to manipulate ftrace
      selftests/bpf: port test_ftrace.sh to test_progs

 .../selftests/bpf/prog_tests/fgraph_trampoline.c   | 129 +++++++++++
 .../selftests/bpf/progs/fgraph_trampoline.c        |  29 +++
 tools/testing/selftests/bpf/test_ftrace.sh         |  44 ----
 tools/testing/selftests/bpf/trace_helpers.c        | 247 ++++++++++++++++++++-
 tools/testing/selftests/bpf/trace_helpers.h        |   9 +
 5 files changed, 412 insertions(+), 46 deletions(-)
---
base-commit: 960cc441448b063bbc1f283d7e801af6d872e45c
change-id: 20261027-ftrace_in_test_progs-d5f70a2330ac

Best regards,
--  
Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>

[PATCH bpf-next 1/2] selftests/bpf: add helpers to manipulate ftrace

[Alexis Lothoré (eBPF Foundation)]

Add helpers to allow a test_progs test to tune ftrace configuration.
This new set of helpers work with an opaque structure: this one is used
to record any change performed on ftrace, so that we can restore those
automatically once the test is done.

The new helpers distinguish between two entries types:
- basic controls, that are "knobs"
- filters, for which writes are "additive"

Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
 tools/testing/selftests/bpf/trace_helpers.c | 247 +++++++++++++++++++++++++++-
 tools/testing/selftests/bpf/trace_helpers.h |   9 +
 2 files changed, 254 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
index 6d7a4b8c4ab8..1f26a057a810 100644
--- a/tools/testing/selftests/bpf/trace_helpers.c
+++ b/tools/testing/selftests/bpf/trace_helpers.c
@@ -21,8 +21,12 @@
 #include "bpf/libbpf_internal.h"
 #include "bpf_util.h"
 
-#define TRACEFS_PIPE	"/sys/kernel/tracing/trace_pipe"
-#define DEBUGFS_PIPE	"/sys/kernel/debug/tracing/trace_pipe"
+#define TRACEFS		"/sys/kernel/tracing"
+#define TRACEFS_PIPE	TRACEFS "/trace_pipe"
+#define DEBUGFS		"/sys/kernel/debug/tracing"
+#define DEBUGFS_PIPE	DEBUGFS "/trace_pipe"
+
+#define CHUNK_SIZE	128
 
 static struct ksyms *ksyms;
 static pthread_mutex_t ksyms_mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -557,6 +561,245 @@ void read_trace_pipe(void)
 	read_trace_pipe_iter(trace_pipe_cb, NULL, 0);
 }
 
+static const char *tracefs_mountpoint(void)
+{
+	return access(TRACEFS_PIPE, F_OK) == 0 ? TRACEFS : DEBUGFS;
+}
+
+static int tracefs_path(char *buf, size_t size, const char *file)
+{
+	int ret = snprintf(buf, size, "%s/%s", tracefs_mountpoint(), file);
+
+	return ret < 0 ? ret : 0;
+}
+
+static int tracefs_read(int fd, char **content)
+{
+	size_t len = 0;
+	char tmp[CHUNK_SIZE];
+	char *buf = NULL;
+	ssize_t n;
+	int ret;
+
+	ret = lseek(fd, 0, SEEK_SET);
+	if (ret < 0)
+		return ret;
+
+	do {
+		n = read(fd, tmp, CHUNK_SIZE);
+		if (n < 0) {
+			if (buf)
+				free(buf);
+			return n;
+		}
+		if (!n)
+			break;
+		buf = realloc(buf, len + n + 1);
+		memcpy(buf + len, tmp, n);
+		len += n;
+	} while (1);
+
+	/* No data read at all, file was empty */
+	if (!buf) {
+		buf = malloc(1);
+		if (!buf)
+			return -ENOMEM;
+	}
+	buf[len] = '\0';
+	*content = buf;
+	return 0;
+}
+
+static int tracefs_write(int fd, const char *str)
+{
+	ssize_t ret;
+
+	ret = lseek(fd, 0, SEEK_SET);
+	if (ret < 0)
+		return ret;
+
+	ret = write(fd, str, strlen(str));
+	if (ret < 0)
+		return ret;
+	return 0;
+}
+
+static int tracefs_write_file(const char *path, const char *str)
+{
+	int fd, err;
+
+	fd = open(path, O_CLOEXEC | O_WRONLY | (*str ? 0 : O_TRUNC));
+	if (fd < 0)
+		return fd;
+
+	err = tracefs_write(fd, str);
+	close(fd);
+	return err;
+}
+
+enum tracefs_ctl_kind {
+	TRACEFS_CTL_FILTER,
+	TRACEFS_CTL_VALUE,
+};
+
+struct tracefs_ctl_entry {
+	char path[PATH_MAX];
+	char *old;
+	enum tracefs_ctl_kind kind;
+};
+
+struct tracefs_ctl {
+	struct tracefs_ctl_entry *entries;
+	size_t cnt;
+};
+
+struct tracefs_ctl *tracefs_ctl_init(void)
+{
+	return calloc(1, sizeof(struct tracefs_ctl));
+}
+
+static int tracefs_ctl_update(struct tracefs_ctl *ctl, const char *file,
+			      const char *str, enum tracefs_ctl_kind kind)
+{
+	struct tracefs_ctl_entry *entry, *tmp;
+	char path[PATH_MAX];
+	char *old = NULL;
+	bool recorded = false;
+	size_t i;
+	int fd, err;
+
+	if (!ctl)
+		return -EINVAL;
+
+	err = tracefs_path(path, sizeof(path), file);
+	if (err)
+		return err;
+
+	for (i = 0; i < ctl->cnt; i++) {
+		if (strcmp(ctl->entries[i].path, path))
+			continue;
+		if (ctl->entries[i].kind != kind)
+			return -EINVAL;
+		/* Already altered, keep the value read on the first write. */
+		return tracefs_write_file(ctl->entries[i].path, str);
+	}
+
+	fd = open(path, O_CLOEXEC | O_RDWR);
+	if (fd < 0)
+		return fd;
+
+	err = tracefs_read(fd, &old);
+	if (!err) {
+		tmp = realloc(ctl->entries, (ctl->cnt + 1) * sizeof(*tmp));
+		if (!tmp) {
+			free(old);
+			close(fd);
+			return -ENOMEM;
+		}
+		ctl->entries = tmp;
+		entry = &ctl->entries[ctl->cnt];
+		strcpy(entry->path, path);
+		entry->old = old;
+		entry->kind = kind;
+		recorded = true;
+	}
+
+	err = tracefs_write(fd, str);
+	close(fd);
+	if (err) {
+		free(old);
+		return err;
+	}
+
+	if (recorded)
+		ctl->cnt++;
+	return 0;
+}
+
+int tracefs_ctl_filter_write(struct tracefs_ctl *ctl, const char *file,
+			     const char *pattern)
+{
+	return tracefs_ctl_update(ctl, file, pattern, TRACEFS_CTL_FILTER);
+}
+
+int tracefs_ctl_set(struct tracefs_ctl *ctl, const char *file, const char *value)
+{
+	return tracefs_ctl_update(ctl, file, value, TRACEFS_CTL_VALUE);
+}
+
+static const char *tracefs_next_line(const char *content, char *buf, size_t size,
+				     int *err)
+{
+	const char *end;
+	size_t len;
+
+	*err = 0;
+	while (*content) {
+		end = strchrnul(content, '\n');
+		len = end - content;
+		content = *end ? end + 1 : end;
+
+		if (!len || end[-len] == '#')
+			continue;
+
+		if (len + 2 > size) {
+			*err = -ENAMETOOLONG;
+			return NULL;
+		}
+		memcpy(buf, end - len, len);
+		buf[len++] = '\n';
+		buf[len] = '\0';
+		return content;
+	}
+
+	return NULL;
+}
+
+static int tracefs_restore_file(const struct tracefs_ctl_entry *entry)
+{
+	char buf[CHUNK_SIZE];
+	const char *content = entry->old;
+	int err = 0;
+
+	if (!content)
+		return 0;
+
+	if (entry->kind == TRACEFS_CTL_FILTER) {
+		err = tracefs_write_file(entry->path, "");
+		if (err)
+			return err;
+	}
+
+	while ((content = tracefs_next_line(content, buf, sizeof(buf), &err))) {
+		err = tracefs_write_file(entry->path, buf);
+		if (err)
+			return err;
+	}
+
+	return err;
+}
+
+void tracefs_ctl_restore(struct tracefs_ctl *ctl)
+{
+	size_t i;
+	int err;
+
+	if (!ctl)
+		return;
+
+	for (i = ctl->cnt; i > 0; i--) {
+		struct tracefs_ctl_entry *entry = &ctl->entries[i - 1];
+
+		err = tracefs_restore_file(entry);
+		if (err)
+			fprintf(stderr, "Failed to restore %s: %s\n",
+				entry->path, strerror(-err));
+		free(entry->old);
+	}
+	free(ctl->entries);
+	free(ctl);
+}
+
 static size_t symbol_hash(long key, void *ctx __maybe_unused)
 {
 	return str_hash((const char *) key);
diff --git a/tools/testing/selftests/bpf/trace_helpers.h b/tools/testing/selftests/bpf/trace_helpers.h
index 0f8d99746084..432a95016b69 100644
--- a/tools/testing/selftests/bpf/trace_helpers.h
+++ b/tools/testing/selftests/bpf/trace_helpers.h
@@ -57,6 +57,15 @@ void read_trace_pipe(void);
 int read_trace_pipe_iter(void (*cb)(const char *str, void *data),
 			 void *data, int iter);
 
+struct tracefs_ctl;
+
+struct tracefs_ctl *tracefs_ctl_init(void);
+int tracefs_ctl_filter_write(struct tracefs_ctl *ctl, const char *file,
+			     const char *pattern);
+int tracefs_ctl_set(struct tracefs_ctl *ctl, const char *file,
+		    const char *value);
+void tracefs_ctl_restore(struct tracefs_ctl *ctl);
+
 ssize_t get_uprobe_offset(const void *addr);
 ssize_t get_rel_offset(uintptr_t addr);
 

-- 
2.56.0

[PATCH bpf-next 2/2] selftests/bpf: port test_ftrace.sh to test_progs

[Alexis Lothoré (eBPF Foundation)]

Commit ff205766dbbe ("ftrace: Fix function_graph tracer interaction with
BPF trampoline") made sure that trampolines generated for tracing
programs did not collide with the function graph when we are returning
from the hooked function. The series bringing this commit also came with
a new test ensuring that the kernel does not crash when executing BPF
programs handled by trampolines while the function graph trace is
enabled, but this test is a standalone shell script, hence not executed
in CI.

Convert the script to the test_progs runner so it can be executed
automatically by the CI infrastructure. There are two main options to
port this test:
  - just enable ftrace during some existing trampoline-related tests
    (eg: fentry/fexit tests)
  - create a dedicated, minimal test with function graph

The new test adopts the latter approach, with a smaller scope to keep
execution time reasonable. On top of some simple fentry/fexit program,
it also adds a small fmod_ret program to ensure that return values are
correctly handled while the function graph enabled. Despite the few
asserts in the new test, the actual goal  is to make sure that the
kernel does not crash because of some incorrect stack frame skipping
when both the function graph and tracing programs are running.

ftrace is configured through the new ftrace helpers, and restored to its
original state afterward. Despite being restored afterward, it is
configured for the whole system, so the test is marked as serial_ to
avoid conflicts with other tests needing traces (eg trace_vprintk)

With the new test integrated into test_progs, drop test_ftrace.sh.

Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
 .../selftests/bpf/prog_tests/fgraph_trampoline.c   | 129 +++++++++++++++++++++
 .../selftests/bpf/progs/fgraph_trampoline.c        |  29 +++++
 tools/testing/selftests/bpf/test_ftrace.sh         |  44 -------
 3 files changed, 158 insertions(+), 44 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/fgraph_trampoline.c b/tools/testing/selftests/bpf/prog_tests/fgraph_trampoline.c
new file mode 100644
index 000000000000..98edd77cf214
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/fgraph_trampoline.c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+#include "trace_helpers.h"
+#include "fgraph_trampoline.skel.h"
+
+#define TRACER_GRAPH	"function_graph"
+
+#define FTRACE_NOTRACE	"*printk* *console* *wake* *serial* *lock*"
+#define GRAPH_FUNCS	"bpf_prog_test*"
+
+/* See test_run.c: we are overriding bpf_modify_return_test with a program
+ * returning 5, but not bpf_modify_return_test2
+ */
+#define FMOD_EXPECTED_RETVAL ((1 << 16) | (5 + 1 + 3 + 3 + 4 + 5 + 6 + 7))
+
+static int configure_tracer(struct tracefs_ctl *ftrace)
+{
+	int err;
+
+	err = tracefs_ctl_filter_write(ftrace, "set_ftrace_filter", "");
+	if (err)
+		return err;
+	err = tracefs_ctl_filter_write(ftrace, "set_ftrace_notrace",
+				       FTRACE_NOTRACE);
+	if (err)
+		return err;
+	err = tracefs_ctl_filter_write(ftrace, "set_graph_function", GRAPH_FUNCS);
+	if (err)
+		return err;
+	err = tracefs_ctl_filter_write(ftrace, "set_graph_notrace", "");
+	if (err)
+		return err;
+	err = tracefs_ctl_set(ftrace, "current_tracer", TRACER_GRAPH);
+	if (err)
+		return err;
+
+	return err;
+}
+
+static int run_test_under_ftrace_graph(struct tracefs_ctl *ftrace,
+				       struct bpf_program *prog, int *retval)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, opts);
+	int err, stop_err;
+
+	err = tracefs_ctl_set(ftrace, "tracing_on", "1");
+	if (err)
+		return err;
+
+	err = bpf_prog_test_run_opts(bpf_program__fd(prog), &opts);
+	if (retval)
+		*retval = opts.retval;
+
+	stop_err = tracefs_ctl_set(ftrace, "tracing_on", "0");
+	if (!err)
+		err = stop_err;
+
+	return err;
+}
+
+void serial_test_fgraph_trampoline(void)
+{
+	struct fgraph_trampoline *skel = NULL;
+	struct tracefs_ctl *ftrace;
+	int retval;
+
+	ftrace = tracefs_ctl_init();
+	if (!ASSERT_OK_PTR(ftrace, "tracefs_ctl_init"))
+		return;
+
+	if (configure_tracer(ftrace)) {
+		test__skip();
+		tracefs_ctl_restore(ftrace);
+		goto cleanup;
+	}
+
+	skel = fgraph_trampoline__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "fgraph_trampoline__open_and_load"))
+		goto cleanup;
+
+	if (test__start_subtest("no_trampoline")) {
+		ASSERT_OK(run_test_under_ftrace_graph(ftrace,
+						      skel->progs.fentry_test1,
+						      NULL),
+			  "run test no trampoline");
+	}
+
+	if (test__start_subtest("attach_fentry_fexit")) {
+		skel->links.fentry_test1 =
+			bpf_program__attach(skel->progs.fentry_test1);
+		if (!ASSERT_OK_PTR(skel->links.fentry_test1, "attach_fentry"))
+			goto destroy_cleanup;
+		skel->links.fexit_test1 =
+			bpf_program__attach(skel->progs.fexit_test1);
+		if (!ASSERT_OK_PTR(skel->links.fexit_test1, "attach_fexit"))
+			goto destroy_cleanup;
+
+		memset(skel->bss, 0, sizeof(*skel->bss));
+		ASSERT_OK(run_test_under_ftrace_graph(ftrace,
+						      skel->progs.fentry_test1,
+						      NULL),
+			  "run test with fentry and fexit");
+
+		ASSERT_EQ(skel->bss->fexit_result, 1, "fexit_result");
+
+		fgraph_trampoline__detach(skel);
+	}
+
+	if (test__start_subtest("attach_fmod_ret")) {
+		skel->links.fmod_ret_test =
+			bpf_program__attach(skel->progs.fmod_ret_test);
+		if (!ASSERT_OK_PTR(skel->links.fmod_ret_test, "attach_fmod_ret"))
+			goto destroy_cleanup;
+
+		ASSERT_OK(run_test_under_ftrace_graph(ftrace,
+						      skel->progs.fmod_ret_test,
+						      &retval),
+			  "run test with fmod_ret");
+		ASSERT_EQ(retval, FMOD_EXPECTED_RETVAL, "fmod return value");
+
+		fgraph_trampoline__detach(skel);
+	}
+
+destroy_cleanup:
+	fgraph_trampoline__destroy(skel);
+cleanup:
+	tracefs_ctl_restore(ftrace);
+}
diff --git a/tools/testing/selftests/bpf/progs/fgraph_trampoline.c b/tools/testing/selftests/bpf/progs/fgraph_trampoline.c
new file mode 100644
index 000000000000..1cc2dadc817e
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/fgraph_trampoline.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+__u64 fexit_result;
+
+SEC("fentry/bpf_fentry_test1")
+int BPF_PROG(fentry_test1, int a)
+{
+	return 0;
+}
+
+SEC("fexit/bpf_fentry_test1")
+int BPF_PROG(fexit_test1, int a, int ret)
+{
+	fexit_result = (a == 1 && ret == 2);
+	return 0;
+}
+
+SEC("fmod_ret/bpf_modify_return_test")
+int BPF_PROG(fmod_ret_test, int a, int *b, int ret)
+{
+	return 5;
+}
+
+char _license[] SEC("license") = "GPL";
+
diff --git a/tools/testing/selftests/bpf/test_ftrace.sh b/tools/testing/selftests/bpf/test_ftrace.sh
deleted file mode 100755
index f5109eb0e951..000000000000
--- a/tools/testing/selftests/bpf/test_ftrace.sh
+++ /dev/null
@@ -1,44 +0,0 @@
-#!/bin/bash
-
-if [[ -e /sys/kernel/tracing/trace ]]; then
-    TR=/sys/kernel/tracing/
-else
-    TR=/sys/kernel/debug/tracing/
-fi
-
-clear_trace() { # reset trace output
-    echo > $TR/trace
-}
-
-disable_tracing() { # stop trace recording
-    echo 0 > $TR/tracing_on
-}
-
-enable_tracing() { # start trace recording
-    echo 1 > $TR/tracing_on
-}
-
-reset_tracer() { # reset the current tracer
-    echo nop > $TR/current_tracer
-}
-
-disable_tracing
-clear_trace
-
-echo "" > $TR/set_ftrace_filter
-echo '*printk* *console* *wake* *serial* *lock*' > $TR/set_ftrace_notrace
-
-echo "bpf_prog_test*" > $TR/set_graph_function
-echo "" > $TR/set_graph_notrace
-
-echo function_graph > $TR/current_tracer
-
-enable_tracing
-./test_progs -t fentry
-./test_progs -t fexit
-disable_tracing
-clear_trace
-
-reset_tracer
-
-exit 0

-- 
2.56.0

Re: [PATCH bpf-next 1/2] selftests/bpf: add helpers to manipulate ftrace

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3178 bytes --]

> selftests/bpf: add helpers to manipulate ftrace

TL;DR: Restoring ftrace filters in trace_helpers.c may abort on a saved line
over 126 bytes (128-byte buffer), leaving the filter empty, so all functions
get traced and global ftrace state isn't restored.

> diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
> index 6d7a4b8c4ab8..1f26a057a810 100644
> --- a/tools/testing/selftests/bpf/trace_helpers.c
> +++ b/tools/testing/selftests/bpf/trace_helpers.c

[ ... ]

> +#define CHUNK_SIZE	128

[ ... ]

> +static const char *tracefs_next_line(const char *content, char *buf, size_t size,
> +				     int *err)
> +{
> +	const char *end;
> +	size_t len;
> +
> +	*err = 0;
> +	while (*content) {
> +		end = strchrnul(content, '\n');
> +		len = end - content;
> +		content = *end ? end + 1 : end;
> +
> +		if (!len || end[-len] == '#')
> +			continue;
> +
> +		if (len + 2 > size) {
> +			*err = -ENAMETOOLONG;
> +			return NULL;
> +		}
> +		memcpy(buf, end - len, len);
> +		buf[len++] = '\n';
> +		buf[len] = '\0';
> +		return content;
> +	}
> +
> +	return NULL;
> +}
> +
> +static int tracefs_restore_file(const struct tracefs_ctl_entry *entry)
> +{
> +	char buf[CHUNK_SIZE];
> +	const char *content = entry->old;
> +	int err = 0;
> +
> +	if (!content)
> +		return 0;
> +
> +	if (entry->kind == TRACEFS_CTL_FILTER) {
> +		err = tracefs_write_file(entry->path, "");
> +		if (err)
> +			return err;
> +	}
> +
> +	while ((content = tracefs_next_line(content, buf, sizeof(buf), &err))) {
> +		err = tracefs_write_file(entry->path, buf);
> +		if (err)
> +			return err;
> +	}
> +
> +	return err;
> +}

Can a saved line longer than 126 bytes abort the restore here?

tracefs_restore_file() splits the saved content into a buffer sized by the
read chunk constant:

	char buf[CHUNK_SIZE];     /* CHUNK_SIZE is 128 */

and tracefs_next_line() fails the whole restore with -ENAMETOOLONG when
len + 2 > size.  The kernel can print much longer entries than that.
print_rec() emits the full symbol (KSYM_NAME_LEN is 512) plus the
" [module]" suffix, so mangled Rust symbols or long C names with a module
suffix can exceed 126 bytes.

For TRACEFS_CTL_FILTER entries the file has already been emptied by
tracefs_write_file(entry->path, "") before the long line is reached:

	if (entry->kind == TRACEFS_CTL_FILTER) {
		err = tracefs_write_file(entry->path, "");
		...
	}
	while ((content = tracefs_next_line(content, buf, sizeof(buf), &err))) {

so the restore stops part-way, every entry after the long line is lost, and
if none were restored yet the filter is left empty.  For set_ftrace_filter
an empty filter means all functions are traced, which is the opposite of
the original restriction, and the global ftrace state is not restored.

Should the line buffer be sized to the actual line length (or should the
content be written straight from the saved string) instead of being tied to
CHUNK_SIZE?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/41113297251