[PATCH bpf-next v3 0/3] Separate tests that need error injection

[PATCH bpf-next v3 0/3] Separate tests that need error injection

[Viktor Malik]

Some enterprise kernels (such as RHEL) do not enable error injection via
BPF (CONFIG_FUNCTION_ERROR_INJECTION and CONFIG_BPF_KPROBE_OVERRIDE).
When running test_progs on such kernels, a lot of test cases fail since
they use sleepable fentry or fmod_ret program types which require error
injection to be enabled. While it is possible to skip these via custom
DENYLIST, some test_progs are not properly split into subtests and
therefore must be entirely skipped.

This patch series split such tests into subtests, namely module_attach
and read_vsyscall. In addition, the last patch separates a sleepable
fentry test out of the LSM test suite into a separate test so that it
can be skipped without skipping other LSM tests.

Changes in v3:
- Replace `return` by `goto cleanup` when skeleton is already open
  (found by AI review)

Changes in v2:
- Fix indices in read_vsyscall/copy_from_user subtest (reported by CI)

Viktor Malik (3):
  selftests/bpf: Split module_attach into subtests
  selftests/bpf: Split read_vsyscall into subtests
  selftests/bpf: Split sleepable fentry from LSM test

 .../bpf/prog_tests/fentry_sleepable.c         |  28 +++
 .../selftests/bpf/prog_tests/module_attach.c  | 168 +++++++++++++-----
 .../selftests/bpf/prog_tests/read_vsyscall.c  |  41 ++++-
 .../selftests/bpf/prog_tests/test_lsm.c       |   8 -
 .../selftests/bpf/progs/fentry_sleepable.c    |  28 +++
 tools/testing/selftests/bpf/progs/lsm.c       |  21 ---
 .../selftests/bpf/progs/read_vsyscall.c       |  18 +-
 .../selftests/bpf/progs/test_module_attach.c  |  63 +++----
 8 files changed, 249 insertions(+), 126 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/fentry_sleepable.c
 create mode 100644 tools/testing/selftests/bpf/progs/fentry_sleepable.c

-- 
2.53.0

[PATCH bpf-next v3 1/3] selftests/bpf: Split module_attach into subtests

[Viktor Malik]

The test verifies attachment to various hooks in a kernel module,
however, everything is flattened into a single test. When running the
test on a kernel which doesn't support some of the hooks, it is
impossible to skip them selectively.

Isolate each BPF program into a separate subtest. This is done by
disabling auto-loading of programs and loading and testing each program
separately.

Signed-off-by: Viktor Malik <vmalik@redhat.com>
---
 .../selftests/bpf/prog_tests/module_attach.c  | 168 +++++++++++++-----
 .../selftests/bpf/progs/test_module_attach.c  |  63 +++----
 2 files changed, 149 insertions(+), 82 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c b/tools/testing/selftests/bpf/prog_tests/module_attach.c
index 70fa7ae93173..a932e03cb1c9 100644
--- a/tools/testing/selftests/bpf/prog_tests/module_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c
@@ -6,6 +6,23 @@
 #include "test_module_attach.skel.h"
 #include "testing_helpers.h"
 
+static const char * const read_tests[] = {
+	"handle_raw_tp",
+	"handle_tp_btf",
+	"handle_fentry",
+	"handle_fentry_explicit",
+	"handle_fmod_ret",
+};
+
+static const char * const detach_tests[] = {
+	"handle_fentry",
+	"handle_fexit",
+	"kprobe_multi",
+};
+
+static const int READ_SZ = 456;
+static const int WRITE_SZ = 457;
+
 static int duration;
 
 static int trigger_module_test_writable(int *val)
@@ -33,53 +50,77 @@ static int trigger_module_test_writable(int *val)
 	return 0;
 }
 
-void test_module_attach(void)
+static void test_module_attach_prog(const char *prog_name, int sz,
+				    const char *attach_target, int ret)
 {
-	const int READ_SZ = 456;
-	const int WRITE_SZ = 457;
-	struct test_module_attach* skel;
-	struct test_module_attach__bss *bss;
-	struct bpf_link *link;
+	struct test_module_attach *skel;
+	struct bpf_program *prog;
 	int err;
-	int writable_val = 0;
 
 	skel = test_module_attach__open();
 	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
 		return;
 
-	err = bpf_program__set_attach_target(skel->progs.handle_fentry_manual,
-					     0, "bpf_testmod_test_read");
-	ASSERT_OK(err, "set_attach_target");
+	prog = bpf_object__find_program_by_name(skel->obj, prog_name);
+	if (!ASSERT_OK_PTR(prog, "find program"))
+		goto cleanup;
+	bpf_program__set_autoload(prog, true);
 
-	err = bpf_program__set_attach_target(skel->progs.handle_fentry_explicit_manual,
-					     0, "bpf_testmod:bpf_testmod_test_read");
-	ASSERT_OK(err, "set_attach_target_explicit");
+	if (attach_target) {
+		err = bpf_program__set_attach_target(prog, 0, attach_target);
+		ASSERT_OK(err, attach_target);
+	}
 
 	err = test_module_attach__load(skel);
 	if (CHECK(err, "skel_load", "failed to load skeleton\n"))
+		goto cleanup;
+
+	err = test_module_attach__attach(skel);
+	if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
+		goto cleanup;
+
+	if (sz) {
+		/* trigger both read and write though each test uses only one */
+		ASSERT_OK(trigger_module_test_read(sz), "trigger_read");
+		ASSERT_OK(trigger_module_test_write(sz), "trigger_write");
+
+		ASSERT_EQ(skel->bss->sz, sz, prog_name);
+	}
+
+	if (ret)
+		ASSERT_EQ(skel->bss->retval, ret, "ret");
+cleanup:
+	test_module_attach__destroy(skel);
+}
+
+static void test_module_attach_writable(void)
+{
+	struct test_module_attach__bss *bss;
+	struct test_module_attach *skel;
+	struct bpf_program *prog;
+	int writable_val = 0;
+	int err;
+
+	skel = test_module_attach__open();
+	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
 		return;
 
+	prog = bpf_object__find_program_by_name(skel->obj,
+						"handle_raw_tp_writable_bare");
+	if (!ASSERT_OK_PTR(prog, "find program"))
+		goto cleanup;
+	bpf_program__set_autoload(prog, true);
+
+	err = test_module_attach__load(skel);
+	if (CHECK(err, "skel_load", "failed to load skeleton\n"))
+		goto cleanup;
+
 	bss = skel->bss;
 
 	err = test_module_attach__attach(skel);
 	if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
 		goto cleanup;
 
-	/* trigger tracepoint */
-	ASSERT_OK(trigger_module_test_read(READ_SZ), "trigger_read");
-	ASSERT_OK(trigger_module_test_write(WRITE_SZ), "trigger_write");
-
-	ASSERT_EQ(bss->raw_tp_read_sz, READ_SZ, "raw_tp");
-	ASSERT_EQ(bss->raw_tp_bare_write_sz, WRITE_SZ, "raw_tp_bare");
-	ASSERT_EQ(bss->tp_btf_read_sz, READ_SZ, "tp_btf");
-	ASSERT_EQ(bss->fentry_read_sz, READ_SZ, "fentry");
-	ASSERT_EQ(bss->fentry_manual_read_sz, READ_SZ, "fentry_manual");
-	ASSERT_EQ(bss->fentry_explicit_read_sz, READ_SZ, "fentry_explicit");
-	ASSERT_EQ(bss->fentry_explicit_manual_read_sz, READ_SZ, "fentry_explicit_manual");
-	ASSERT_EQ(bss->fexit_read_sz, READ_SZ, "fexit");
-	ASSERT_EQ(bss->fexit_ret, -EIO, "fexit_tet");
-	ASSERT_EQ(bss->fmod_ret_read_sz, READ_SZ, "fmod_ret");
-
 	bss->raw_tp_writable_bare_early_ret = true;
 	bss->raw_tp_writable_bare_out_val = 0xf1f2f3f4;
 	ASSERT_OK(trigger_module_test_writable(&writable_val),
@@ -87,31 +128,72 @@ void test_module_attach(void)
 	ASSERT_EQ(bss->raw_tp_writable_bare_in_val, 1024, "writable_test_in");
 	ASSERT_EQ(bss->raw_tp_writable_bare_out_val, writable_val,
 		  "writable_test_out");
+cleanup:
+	test_module_attach__destroy(skel);
+}
 
-	test_module_attach__detach(skel);
-
-	/* attach fentry/fexit and make sure it gets module reference */
-	link = bpf_program__attach(skel->progs.handle_fentry);
-	if (!ASSERT_OK_PTR(link, "attach_fentry"))
-		goto cleanup;
+static void test_module_attach_detach(const char *prog_name)
+{
+	struct test_module_attach *skel;
+	struct bpf_program *prog;
+	struct bpf_link *link;
+	int err;
 
-	ASSERT_ERR(unload_bpf_testmod(false), "unload_bpf_testmod");
-	bpf_link__destroy(link);
+	skel = test_module_attach__open();
+	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
+		return;
 
-	link = bpf_program__attach(skel->progs.handle_fexit);
-	if (!ASSERT_OK_PTR(link, "attach_fexit"))
+	prog = bpf_object__find_program_by_name(skel->obj, prog_name);
+	if (!ASSERT_OK_PTR(prog, "find program"))
 		goto cleanup;
+	bpf_program__set_autoload(prog, true);
 
-	ASSERT_ERR(unload_bpf_testmod(false), "unload_bpf_testmod");
-	bpf_link__destroy(link);
+	err = test_module_attach__load(skel);
+	if (CHECK(err, "skel_load", "failed to load skeleton\n"))
+		goto cleanup;
 
-	link = bpf_program__attach(skel->progs.kprobe_multi);
-	if (!ASSERT_OK_PTR(link, "attach_kprobe_multi"))
+	/* attach and make sure it gets module reference */
+	link = bpf_program__attach(prog);
+	if (!ASSERT_OK_PTR(link, "attach"))
 		goto cleanup;
 
 	ASSERT_ERR(unload_bpf_testmod(false), "unload_bpf_testmod");
 	bpf_link__destroy(link);
-
 cleanup:
 	test_module_attach__destroy(skel);
 }
+
+void test_module_attach(void)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(read_tests); i++) {
+		if (!test__start_subtest(read_tests[i]))
+			continue;
+		test_module_attach_prog(read_tests[i], READ_SZ, NULL, 0);
+	}
+	if (test__start_subtest("handle_raw_tp_bare")) {
+		test_module_attach_prog("handle_raw_tp_bare", WRITE_SZ, NULL,
+					0);
+	}
+	if (test__start_subtest("handle_raw_tp_writable_bare"))
+		test_module_attach_writable();
+	if (test__start_subtest("handle_fentry_manual")) {
+		test_module_attach_prog("handle_fentry_manual", READ_SZ,
+					"bpf_testmod_test_read", 0);
+	}
+	if (test__start_subtest("handle_fentry_explicit_manual")) {
+		test_module_attach_prog("handle_fentry_explicit_manual",
+					READ_SZ,
+					"bpf_testmod:bpf_testmod_test_read", 0);
+	}
+	if (test__start_subtest("handle_fexit"))
+		test_module_attach_prog("handle_fexit", READ_SZ, NULL, -EIO);
+	if (test__start_subtest("handle_fexit_ret"))
+		test_module_attach_prog("handle_fexit_ret", 0, NULL, 0);
+	for (i = 0; i < ARRAY_SIZE(detach_tests); i++) {
+		if (!test__start_subtest(detach_tests[i]))
+			continue;
+		test_module_attach_detach(detach_tests[i]);
+	}
+}
diff --git a/tools/testing/selftests/bpf/progs/test_module_attach.c b/tools/testing/selftests/bpf/progs/test_module_attach.c
index 03d7f89787a1..5609e388fb58 100644
--- a/tools/testing/selftests/bpf/progs/test_module_attach.c
+++ b/tools/testing/selftests/bpf/progs/test_module_attach.c
@@ -7,23 +7,21 @@
 #include <bpf/bpf_core_read.h>
 #include "../test_kmods/bpf_testmod.h"
 
-__u32 raw_tp_read_sz = 0;
+__u32 sz = 0;
 
-SEC("raw_tp/bpf_testmod_test_read")
+SEC("?raw_tp/bpf_testmod_test_read")
 int BPF_PROG(handle_raw_tp,
 	     struct task_struct *task, struct bpf_testmod_test_read_ctx *read_ctx)
 {
-	raw_tp_read_sz = BPF_CORE_READ(read_ctx, len);
+	sz = BPF_CORE_READ(read_ctx, len);
 	return 0;
 }
 
-__u32 raw_tp_bare_write_sz = 0;
-
-SEC("raw_tp/bpf_testmod_test_write_bare_tp")
+SEC("?raw_tp/bpf_testmod_test_write_bare_tp")
 int BPF_PROG(handle_raw_tp_bare,
 	     struct task_struct *task, struct bpf_testmod_test_write_ctx *write_ctx)
 {
-	raw_tp_bare_write_sz = BPF_CORE_READ(write_ctx, len);
+	sz = BPF_CORE_READ(write_ctx, len);
 	return 0;
 }
 
@@ -31,7 +29,7 @@ int raw_tp_writable_bare_in_val = 0;
 int raw_tp_writable_bare_early_ret = 0;
 int raw_tp_writable_bare_out_val = 0;
 
-SEC("raw_tp.w/bpf_testmod_test_writable_bare_tp")
+SEC("?raw_tp.w/bpf_testmod_test_writable_bare_tp")
 int BPF_PROG(handle_raw_tp_writable_bare,
 	     struct bpf_testmod_test_writable_ctx *writable)
 {
@@ -41,76 +39,65 @@ int BPF_PROG(handle_raw_tp_writable_bare,
 	return 0;
 }
 
-__u32 tp_btf_read_sz = 0;
-
-SEC("tp_btf/bpf_testmod_test_read")
+SEC("?tp_btf/bpf_testmod_test_read")
 int BPF_PROG(handle_tp_btf,
 	     struct task_struct *task, struct bpf_testmod_test_read_ctx *read_ctx)
 {
-	tp_btf_read_sz = read_ctx->len;
+	sz = read_ctx->len;
 	return 0;
 }
 
-__u32 fentry_read_sz = 0;
-
-SEC("fentry/bpf_testmod_test_read")
+SEC("?fentry/bpf_testmod_test_read")
 int BPF_PROG(handle_fentry,
 	     struct file *file, struct kobject *kobj,
 	     struct bin_attribute *bin_attr, char *buf, loff_t off, size_t len)
 {
-	fentry_read_sz = len;
+	sz = len;
 	return 0;
 }
 
-__u32 fentry_manual_read_sz = 0;
-
-SEC("fentry")
+SEC("?fentry")
 int BPF_PROG(handle_fentry_manual,
 	     struct file *file, struct kobject *kobj,
 	     struct bin_attribute *bin_attr, char *buf, loff_t off, size_t len)
 {
-	fentry_manual_read_sz = len;
+	sz = len;
 	return 0;
 }
 
-__u32 fentry_explicit_read_sz = 0;
-
-SEC("fentry/bpf_testmod:bpf_testmod_test_read")
+SEC("?fentry/bpf_testmod:bpf_testmod_test_read")
 int BPF_PROG(handle_fentry_explicit,
 	     struct file *file, struct kobject *kobj,
 	     struct bin_attribute *bin_attr, char *buf, loff_t off, size_t len)
 {
-	fentry_explicit_read_sz = len;
+	sz = len;
 	return 0;
 }
 
 
-__u32 fentry_explicit_manual_read_sz = 0;
-
-SEC("fentry")
+SEC("?fentry")
 int BPF_PROG(handle_fentry_explicit_manual,
 	     struct file *file, struct kobject *kobj,
 	     struct bin_attribute *bin_attr, char *buf, loff_t off, size_t len)
 {
-	fentry_explicit_manual_read_sz = len;
+	sz = len;
 	return 0;
 }
 
-__u32 fexit_read_sz = 0;
-int fexit_ret = 0;
+int retval = 0;
 
-SEC("fexit/bpf_testmod_test_read")
+SEC("?fexit/bpf_testmod_test_read")
 int BPF_PROG(handle_fexit,
 	     struct file *file, struct kobject *kobj,
 	     struct bin_attribute *bin_attr, char *buf, loff_t off, size_t len,
 	     int ret)
 {
-	fexit_read_sz = len;
-	fexit_ret = ret;
+	sz = len;
+	retval = ret;
 	return 0;
 }
 
-SEC("fexit/bpf_testmod_return_ptr")
+SEC("?fexit/bpf_testmod_return_ptr")
 int BPF_PROG(handle_fexit_ret, int arg, struct file *ret)
 {
 	long buf = 0;
@@ -122,18 +109,16 @@ int BPF_PROG(handle_fexit_ret, int arg, struct file *ret)
 	return 0;
 }
 
-__u32 fmod_ret_read_sz = 0;
-
-SEC("fmod_ret/bpf_testmod_test_read")
+SEC("?fmod_ret/bpf_testmod_test_read")
 int BPF_PROG(handle_fmod_ret,
 	     struct file *file, struct kobject *kobj,
 	     struct bin_attribute *bin_attr, char *buf, loff_t off, size_t len)
 {
-	fmod_ret_read_sz = len;
+	sz = len;
 	return 0; /* don't override the exit code */
 }
 
-SEC("kprobe.multi/bpf_testmod_test_read")
+SEC("?kprobe.multi/bpf_testmod_test_read")
 int BPF_PROG(kprobe_multi)
 {
 	return 0;
-- 
2.53.0

[PATCH bpf-next v3 2/3] selftests/bpf: Split read_vsyscall into subtests

[Viktor Malik]

Split the test into two subtests: one for checking bpf_probe_read* of a
vsyscall page from a non-sleepable probe and one for checking
bpf_copy_from_user* of a vsyscall page from a sleepable probe.

The split is useful for running the test on kernels which do not support
sleepable fentry programs to allow skipping just a part of the test.

Signed-off-by: Viktor Malik <vmalik@redhat.com>
---
 .../selftests/bpf/prog_tests/read_vsyscall.c  | 41 ++++++++++++++++---
 .../selftests/bpf/progs/read_vsyscall.c       | 18 ++++----
 2 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c b/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c
index a8d1eaa67020..bbacd1309046 100644
--- a/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/read_vsyscall.c
@@ -14,22 +14,30 @@
 struct read_ret_desc {
 	const char *name;
 	int ret;
-} all_read[] = {
+};
+
+struct read_ret_desc all_probe_read[] = {
 	{ .name = "probe_read_kernel", .ret = -ERANGE },
 	{ .name = "probe_read_kernel_str", .ret = -ERANGE },
 	{ .name = "probe_read", .ret = -ERANGE },
 	{ .name = "probe_read_str", .ret = -ERANGE },
 	{ .name = "probe_read_user", .ret = -EFAULT },
 	{ .name = "probe_read_user_str", .ret = -EFAULT },
+};
+
+struct read_ret_desc all_copy_from_user[] = {
 	{ .name = "copy_from_user", .ret = -EFAULT },
 	{ .name = "copy_from_user_task", .ret = -EFAULT },
 	{ .name = "copy_from_user_str", .ret = -EFAULT },
 	{ .name = "copy_from_user_task_str", .ret = -EFAULT },
 };
 
-void test_read_vsyscall(void)
+static void test_read_vsyscall_subtest(const char *prog_name,
+				       const struct read_ret_desc *descs,
+				       unsigned int cnt)
 {
 	struct read_vsyscall *skel;
+	struct bpf_program *prog;
 	unsigned int i;
 	int err;
 
@@ -37,10 +45,19 @@ void test_read_vsyscall(void)
 	test__skip();
 	return;
 #endif
-	skel = read_vsyscall__open_and_load();
-	if (!ASSERT_OK_PTR(skel, "read_vsyscall open_load"))
+	skel = read_vsyscall__open();
+	if (!ASSERT_OK_PTR(skel, "read_vsyscall open"))
 		return;
 
+	prog = bpf_object__find_program_by_name(skel->obj, prog_name);
+	if (!ASSERT_OK_PTR(prog, "read_vsyscall find program"))
+		goto out;
+	bpf_program__set_autoload(prog, true);
+
+	err = read_vsyscall__load(skel);
+	if (!ASSERT_EQ(err, 0, "read_vsyscall load"))
+		goto out;
+
 	skel->bss->target_pid = getpid();
 	err = read_vsyscall__attach(skel);
 	if (!ASSERT_EQ(err, 0, "read_vsyscall attach"))
@@ -52,8 +69,20 @@ void test_read_vsyscall(void)
 	skel->bss->user_ptr = (void *)VSYSCALL_ADDR;
 	usleep(1);
 
-	for (i = 0; i < ARRAY_SIZE(all_read); i++)
-		ASSERT_EQ(skel->bss->read_ret[i], all_read[i].ret, all_read[i].name);
+	for (i = 0; i < cnt; i++)
+		ASSERT_EQ(skel->bss->read_ret[i], descs[i].ret, descs[i].name);
 out:
 	read_vsyscall__destroy(skel);
 }
+
+void test_read_vsyscall(void)
+{
+	if (test__start_subtest("probe_read")) {
+		test_read_vsyscall_subtest("probe_read", all_probe_read,
+					   ARRAY_SIZE(all_probe_read));
+	}
+	if (test__start_subtest("copy_from_user")) {
+		test_read_vsyscall_subtest("copy_from_user", all_copy_from_user,
+					   ARRAY_SIZE(all_copy_from_user));
+	}
+}
diff --git a/tools/testing/selftests/bpf/progs/read_vsyscall.c b/tools/testing/selftests/bpf/progs/read_vsyscall.c
index 395591374d4f..45bcf923ec70 100644
--- a/tools/testing/selftests/bpf/progs/read_vsyscall.c
+++ b/tools/testing/selftests/bpf/progs/read_vsyscall.c
@@ -8,7 +8,7 @@
 
 int target_pid = 0;
 void *user_ptr = 0;
-int read_ret[10];
+int read_ret[6];
 
 char _license[] SEC("license") = "GPL";
 
@@ -19,8 +19,8 @@ int bpf_copy_from_user_str(void *dst, u32, const void *, u64) __weak __ksym;
 int bpf_copy_from_user_task_str(void *dst, u32, const void *,
 				struct task_struct *, u64) __weak __ksym;
 
-SEC("fentry/" SYS_PREFIX "sys_nanosleep")
-int do_probe_read(void *ctx)
+SEC("?fentry/" SYS_PREFIX "sys_nanosleep")
+int probe_read(void *ctx)
 {
 	char buf[8];
 
@@ -37,19 +37,19 @@ int do_probe_read(void *ctx)
 	return 0;
 }
 
-SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
-int do_copy_from_user(void *ctx)
+SEC("?fentry.s/" SYS_PREFIX "sys_nanosleep")
+int copy_from_user(void *ctx)
 {
 	char buf[8];
 
 	if ((bpf_get_current_pid_tgid() >> 32) != target_pid)
 		return 0;
 
-	read_ret[6] = bpf_copy_from_user(buf, sizeof(buf), user_ptr);
-	read_ret[7] = bpf_copy_from_user_task(buf, sizeof(buf), user_ptr,
+	read_ret[0] = bpf_copy_from_user(buf, sizeof(buf), user_ptr);
+	read_ret[1] = bpf_copy_from_user_task(buf, sizeof(buf), user_ptr,
 					      bpf_get_current_task_btf(), 0);
-	read_ret[8] = bpf_copy_from_user_str((char *)buf, sizeof(buf), user_ptr, 0);
-	read_ret[9] = bpf_copy_from_user_task_str((char *)buf,
+	read_ret[2] = bpf_copy_from_user_str((char *)buf, sizeof(buf), user_ptr, 0);
+	read_ret[3] = bpf_copy_from_user_task_str((char *)buf,
 						  sizeof(buf),
 						  user_ptr,
 						  bpf_get_current_task_btf(),
-- 
2.53.0

[PATCH bpf-next v3 3/3] selftests/bpf: Split sleepable fentry from LSM test

[Viktor Malik]

The test_lsm/lsm_basic test contains a test attaching to a sleepable
fentry, which doesn't logically fit into the LSM test suite. In
addition, it makes the entire test fail on kernels which do not support
sleepable fentry programs.

Factor out the sleepable fentry part into a new test fentry_sleepable.

Signed-off-by: Viktor Malik <vmalik@redhat.com>
---
 .../bpf/prog_tests/fentry_sleepable.c         | 28 +++++++++++++++++++
 .../selftests/bpf/prog_tests/test_lsm.c       |  8 ------
 .../selftests/bpf/progs/fentry_sleepable.c    | 28 +++++++++++++++++++
 tools/testing/selftests/bpf/progs/lsm.c       | 21 --------------
 4 files changed, 56 insertions(+), 29 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/fentry_sleepable.c
 create mode 100644 tools/testing/selftests/bpf/progs/fentry_sleepable.c

diff --git a/tools/testing/selftests/bpf/prog_tests/fentry_sleepable.c b/tools/testing/selftests/bpf/prog_tests/fentry_sleepable.c
new file mode 100644
index 000000000000..efe3ea616575
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/fentry_sleepable.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <unistd.h>
+#include "fentry_sleepable.skel.h"
+
+void test_fentry_sleepable(void)
+{
+	struct fentry_sleepable *skel;
+	int buf = 1234;
+	int err;
+
+	skel = fentry_sleepable__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "fentry_sleepable__open_and_load"))
+		return;
+
+	err = fentry_sleepable__attach(skel);
+	if (!ASSERT_OK(err, "fentry_sleepable__attach"))
+		goto cleanup;
+
+	syscall(__NR_setdomainname, &buf, -2L);
+	syscall(__NR_setdomainname, 0, -3L);
+	syscall(__NR_setdomainname, ~0L, -4L);
+
+	ASSERT_EQ(skel->bss->copy_test, 3, "copy_test");
+
+cleanup:
+	fentry_sleepable__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/prog_tests/test_lsm.c b/tools/testing/selftests/bpf/prog_tests/test_lsm.c
index bdc4fc06bc5a..67c7c437574a 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_lsm.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_lsm.c
@@ -55,7 +55,6 @@ int exec_cmd(int *monitored_pid)
 static int test_lsm(struct lsm *skel)
 {
 	struct bpf_link *link;
-	int buf = 1234;
 	int err;
 
 	err = lsm__attach(skel);
@@ -82,15 +81,8 @@ static int test_lsm(struct lsm *skel)
 
 	ASSERT_EQ(skel->bss->mprotect_count, 1, "mprotect_count");
 
-	syscall(__NR_setdomainname, &buf, -2L);
-	syscall(__NR_setdomainname, 0, -3L);
-	syscall(__NR_setdomainname, ~0L, -4L);
-
-	ASSERT_EQ(skel->bss->copy_test, 3, "copy_test");
-
 	lsm__detach(skel);
 
-	skel->bss->copy_test = 0;
 	skel->bss->bprm_count = 0;
 	skel->bss->mprotect_count = 0;
 	return 0;
diff --git a/tools/testing/selftests/bpf/progs/fentry_sleepable.c b/tools/testing/selftests/bpf/progs/fentry_sleepable.c
new file mode 100644
index 000000000000..621548b97564
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/fentry_sleepable.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <errno.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+int copy_test = 0;
+
+SEC("fentry.s/" SYS_PREFIX "sys_setdomainname")
+int BPF_PROG(test_sys_setdomainname, struct pt_regs *regs)
+{
+	void *ptr = (void *)PT_REGS_PARM1_SYSCALL(regs);
+	int len = PT_REGS_PARM2_SYSCALL(regs);
+	int buf = 0;
+	long ret;
+
+	ret = bpf_copy_from_user(&buf, sizeof(buf), ptr);
+	if (len == -2 && ret == 0 && buf == 1234)
+		copy_test++;
+	if (len == -3 && ret == -EFAULT)
+		copy_test++;
+	if (len == -4 && ret == -EFAULT)
+		copy_test++;
+	return 0;
+}
diff --git a/tools/testing/selftests/bpf/progs/lsm.c b/tools/testing/selftests/bpf/progs/lsm.c
index 7de173daf27b..dd5d41903a26 100644
--- a/tools/testing/selftests/bpf/progs/lsm.c
+++ b/tools/testing/selftests/bpf/progs/lsm.c
@@ -9,7 +9,6 @@
 #include <bpf/bpf_core_read.h>
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
-#include "bpf_misc.h"
 
 struct {
 	__uint(type, BPF_MAP_TYPE_ARRAY);
@@ -161,23 +160,3 @@ int BPF_PROG(test_task_free, struct task_struct *task)
 {
 	return 0;
 }
-
-int copy_test = 0;
-
-SEC("fentry.s/" SYS_PREFIX "sys_setdomainname")
-int BPF_PROG(test_sys_setdomainname, struct pt_regs *regs)
-{
-	void *ptr = (void *)PT_REGS_PARM1_SYSCALL(regs);
-	int len = PT_REGS_PARM2_SYSCALL(regs);
-	int buf = 0;
-	long ret;
-
-	ret = bpf_copy_from_user(&buf, sizeof(buf), ptr);
-	if (len == -2 && ret == 0 && buf == 1234)
-		copy_test++;
-	if (len == -3 && ret == -EFAULT)
-		copy_test++;
-	if (len == -4 && ret == -EFAULT)
-		copy_test++;
-	return 0;
-}
-- 
2.53.0

Re: [PATCH bpf-next v3 0/3] Separate tests that need error injection

[Alexei Starovoitov]

On Tue, Feb 10, 2026 at 7:14 AM Viktor Malik <vmalik@redhat.com> wrote:
>
> Some enterprise kernels (such as RHEL) do not enable error injection via
> BPF (CONFIG_FUNCTION_ERROR_INJECTION and CONFIG_BPF_KPROBE_OVERRIDE).
> When running test_progs on such kernels, a lot of test cases fail since
> they use sleepable fentry or fmod_ret program types which require error
> injection to be enabled. While it is possible to skip these via custom
> DENYLIST, some test_progs are not properly split into subtests and
> therefore must be entirely skipped.

Something is wrong.
-SEC("fentry/" SYS_PREFIX "sys_nanosleep")
-int do_probe_read(void *ctx)
+SEC("?fentry/" SYS_PREFIX "sys_nanosleep")
+int probe_read(void *ctx)
 {
        char buf[8];

@@ -37,19 +37,19 @@ int do_probe_read(void *ctx)
        return 0;
 }

-SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
-int do_copy_from_user(void *ctx)
+SEC("?fentry.s/" SYS_PREFIX "sys_nanosleep")

sleepable should have nothing to do with CONFIG_FUNCTION_ERROR_INJECTION.
While CONFIG_BPF_KPROBE_OVERRIDE affects a single helper.

Both fentry and fentry.s should work fine on sys_nanosleep.
fmod_ret is also completely independent of these two flags.

Please root cause it instead of hacking the tests.

pw-bot: cr

Re: [PATCH bpf-next v3 0/3] Separate tests that need error injection

[Viktor Malik]

On 2/11/26 03:42, Alexei Starovoitov wrote:
> On Tue, Feb 10, 2026 at 7:14 AM Viktor Malik <vmalik@redhat.com> wrote:
>>
>> Some enterprise kernels (such as RHEL) do not enable error injection via
>> BPF (CONFIG_FUNCTION_ERROR_INJECTION and CONFIG_BPF_KPROBE_OVERRIDE).
>> When running test_progs on such kernels, a lot of test cases fail since
>> they use sleepable fentry or fmod_ret program types which require error
>> injection to be enabled. While it is possible to skip these via custom
>> DENYLIST, some test_progs are not properly split into subtests and
>> therefore must be entirely skipped.
> 
> Something is wrong.
> -SEC("fentry/" SYS_PREFIX "sys_nanosleep")
> -int do_probe_read(void *ctx)
> +SEC("?fentry/" SYS_PREFIX "sys_nanosleep")
> +int probe_read(void *ctx)
>  {
>         char buf[8];
> 
> @@ -37,19 +37,19 @@ int do_probe_read(void *ctx)
>         return 0;
>  }
> 
> -SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
> -int do_copy_from_user(void *ctx)
> +SEC("?fentry.s/" SYS_PREFIX "sys_nanosleep")
> 
> sleepable should have nothing to do with CONFIG_FUNCTION_ERROR_INJECTION.

I may be missing something but I think it is directly dependent, at
least for fentry.

In bpf_check_attach_target (kernel/bpf/verifier.c), we have:

    if (prog->sleepable) {
            ret = -EINVAL;
            switch (prog->type) {
            case BPF_PROG_TYPE_TRACING:

                /* fentry/fexit/fmod_ret progs can be sleepable if they are
                 * attached to ALLOW_ERROR_INJECTION and are not in denylist.
                 */
                if (!check_non_sleepable_error_inject(btf_id) &&
                    within_error_injection_list(addr))
                    ret = 0;
                /* fentry/fexit/fmod_ret progs can also be sleepable if they are
                 * in the fmodret id set with the KF_SLEEPABLE flag.
                 */
                else {
                    u32 *flags = btf_kfunc_is_modify_return(btf, btf_id,
                                        prog);

                    if (flags && (*flags & KF_SLEEPABLE))
                        ret = 0;
                }
                break;
            [...]
            }
            if (ret) {
                module_put(mod);
                bpf_log(log, "%s is not sleepable\n", tname);
                return ret;
            }
    }

If I read that correctly, fentry must be on the error injection list
(which is populated by functions marked with ALLOW_ERROR_INJECTION when
CONFIG_FUNCTION_ERROR_INJECTION=y) to be allowed as sleepable.

The other branch for fmodret id set seems to be only relevant for
kernel modules which explicily register their functions into
BTF_KFUNC_HOOK_FMODRET set.

> While CONFIG_BPF_KPROBE_OVERRIDE affects a single helper.
> 
> Both fentry and fentry.s should work fine on sys_nanosleep.
> fmod_ret is also completely independent of these two flags.

For fmod_ret, we have a very similar code:

    } else if (prog->expected_attach_type == BPF_MODIFY_RETURN) {
            [...]
            ret = -EINVAL;
            if (btf_kfunc_is_modify_return(btf, btf_id, prog) ||
                !check_attach_modify_return(addr, tname))
                ret = 0;
            if (ret) {
                module_put(mod);
                bpf_log(log, "%s() is not modifiable\n", tname);
                return ret;
            }
    }

with

    static int check_attach_modify_return(unsigned long addr, const char *func_name)
    {
        if (within_error_injection_list(addr) ||
            !strncmp(SECURITY_PREFIX, func_name, sizeof(SECURITY_PREFIX) - 1))
            return 0;

        return -EINVAL;
    }

So, again, the function needs to be on the error injection list to be
allowed as modifiable.

> 
> Please root cause it instead of hacking the tests.

My question is: are sleepable and fmod_ret functions supposed to work
without error injection turned on? I'm happy to discuss that instead of
hacking the tests but it seems to me that it doesn't work at the moment
(also confirmed experimentally - turning CONFIG_FUNCTION_ERROR_INJECTION
off will make many tests fail with "... is not modifiable" or "... is
not sleepable").

Viktor

> 
> pw-bot: cr
> 

Re: [PATCH bpf-next v3 0/3] Separate tests that need error injection

[Alexei Starovoitov]

On Wed, Feb 11, 2026 at 5:55 AM Viktor Malik <vmalik@redhat.com> wrote:
>
> On 2/11/26 03:42, Alexei Starovoitov wrote:
> > On Tue, Feb 10, 2026 at 7:14 AM Viktor Malik <vmalik@redhat.com> wrote:
> >>
> >> Some enterprise kernels (such as RHEL) do not enable error injection via
> >> BPF (CONFIG_FUNCTION_ERROR_INJECTION and CONFIG_BPF_KPROBE_OVERRIDE).
> >> When running test_progs on such kernels, a lot of test cases fail since
> >> they use sleepable fentry or fmod_ret program types which require error
> >> injection to be enabled. While it is possible to skip these via custom
> >> DENYLIST, some test_progs are not properly split into subtests and
> >> therefore must be entirely skipped.
> >
> > Something is wrong.
> > -SEC("fentry/" SYS_PREFIX "sys_nanosleep")
> > -int do_probe_read(void *ctx)
> > +SEC("?fentry/" SYS_PREFIX "sys_nanosleep")
> > +int probe_read(void *ctx)
> >  {
> >         char buf[8];
> >
> > @@ -37,19 +37,19 @@ int do_probe_read(void *ctx)
> >         return 0;
> >  }
> >
> > -SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
> > -int do_copy_from_user(void *ctx)
> > +SEC("?fentry.s/" SYS_PREFIX "sys_nanosleep")
> >
> > sleepable should have nothing to do with CONFIG_FUNCTION_ERROR_INJECTION.
>
> I may be missing something but I think it is directly dependent, at
> least for fentry.
>
> In bpf_check_attach_target (kernel/bpf/verifier.c), we have:
>
>     if (prog->sleepable) {
>             ret = -EINVAL;
>             switch (prog->type) {
>             case BPF_PROG_TYPE_TRACING:
>
>                 /* fentry/fexit/fmod_ret progs can be sleepable if they are
>                  * attached to ALLOW_ERROR_INJECTION and are not in denylist.
>                  */
>                 if (!check_non_sleepable_error_inject(btf_id) &&
>                     within_error_injection_list(addr))

Oh. That's an annoying side effect of disabling error inject.
fentry progs are not injecting any errors. They just use that
list as allowlist of good attach points.
Let's explicitly allow all syscalls here.

>                     ret = 0;
>                 /* fentry/fexit/fmod_ret progs can also be sleepable if they are
>                  * in the fmodret id set with the KF_SLEEPABLE flag.
>                  */
>                 else {
>                     u32 *flags = btf_kfunc_is_modify_return(btf, btf_id,
>                                         prog);
>
>                     if (flags && (*flags & KF_SLEEPABLE))
>                         ret = 0;
>                 }
>                 break;
>             [...]
>             }
>             if (ret) {
>                 module_put(mod);
>                 bpf_log(log, "%s is not sleepable\n", tname);
>                 return ret;
>             }
>     }
>
> If I read that correctly, fentry must be on the error injection list
> (which is populated by functions marked with ALLOW_ERROR_INJECTION when
> CONFIG_FUNCTION_ERROR_INJECTION=y) to be allowed as sleepable.
>
> The other branch for fmodret id set seems to be only relevant for
> kernel modules which explicily register their functions into
> BTF_KFUNC_HOOK_FMODRET set.
>
> > While CONFIG_BPF_KPROBE_OVERRIDE affects a single helper.
> >
> > Both fentry and fentry.s should work fine on sys_nanosleep.
> > fmod_ret is also completely independent of these two flags.
>
> For fmod_ret, we have a very similar code:
>
>     } else if (prog->expected_attach_type == BPF_MODIFY_RETURN) {
>             [...]
>             ret = -EINVAL;
>             if (btf_kfunc_is_modify_return(btf, btf_id, prog) ||
>                 !check_attach_modify_return(addr, tname))
>                 ret = 0;
>             if (ret) {
>                 module_put(mod);
>                 bpf_log(log, "%s() is not modifiable\n", tname);
>                 return ret;
>             }
>     }
>
> with
>
>     static int check_attach_modify_return(unsigned long addr, const char *func_name)
>     {
>         if (within_error_injection_list(addr) ||
>             !strncmp(SECURITY_PREFIX, func_name, sizeof(SECURITY_PREFIX) - 1))

Same thing here.
Let's wrap within_error_injection_list() with bpf specific helper
that in case of !CONFIG_FUNCTION_ERROR_INJECTION
will allow syscalls anyway.

> My question is: are sleepable and fmod_ret functions supposed to work
> without error injection turned on?

Yes. since they're not injecting errors.

Re: [PATCH bpf-next v3 0/3] Separate tests that need error injection

[Viktor Malik]

On 2/12/26 02:42, Alexei Starovoitov wrote:
> On Wed, Feb 11, 2026 at 5:55 AM Viktor Malik <vmalik@redhat.com> wrote:
>>
>> On 2/11/26 03:42, Alexei Starovoitov wrote:
>>> On Tue, Feb 10, 2026 at 7:14 AM Viktor Malik <vmalik@redhat.com> wrote:
>>>>
>>>> Some enterprise kernels (such as RHEL) do not enable error injection via
>>>> BPF (CONFIG_FUNCTION_ERROR_INJECTION and CONFIG_BPF_KPROBE_OVERRIDE).
>>>> When running test_progs on such kernels, a lot of test cases fail since
>>>> they use sleepable fentry or fmod_ret program types which require error
>>>> injection to be enabled. While it is possible to skip these via custom
>>>> DENYLIST, some test_progs are not properly split into subtests and
>>>> therefore must be entirely skipped.
>>>
>>> Something is wrong.
>>> -SEC("fentry/" SYS_PREFIX "sys_nanosleep")
>>> -int do_probe_read(void *ctx)
>>> +SEC("?fentry/" SYS_PREFIX "sys_nanosleep")
>>> +int probe_read(void *ctx)
>>>  {
>>>         char buf[8];
>>>
>>> @@ -37,19 +37,19 @@ int do_probe_read(void *ctx)
>>>         return 0;
>>>  }
>>>
>>> -SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
>>> -int do_copy_from_user(void *ctx)
>>> +SEC("?fentry.s/" SYS_PREFIX "sys_nanosleep")
>>>
>>> sleepable should have nothing to do with CONFIG_FUNCTION_ERROR_INJECTION.
>>
>> I may be missing something but I think it is directly dependent, at
>> least for fentry.
>>
>> In bpf_check_attach_target (kernel/bpf/verifier.c), we have:
>>
>>     if (prog->sleepable) {
>>             ret = -EINVAL;
>>             switch (prog->type) {
>>             case BPF_PROG_TYPE_TRACING:
>>
>>                 /* fentry/fexit/fmod_ret progs can be sleepable if they are
>>                  * attached to ALLOW_ERROR_INJECTION and are not in denylist.
>>                  */
>>                 if (!check_non_sleepable_error_inject(btf_id) &&
>>                     within_error_injection_list(addr))
> 
> Oh. That's an annoying side effect of disabling error inject.
> fentry progs are not injecting any errors. They just use that
> list as allowlist of good attach points.
> Let's explicitly allow all syscalls here.

Ok, I'll post a patch.

> 
>>                     ret = 0;
>>                 /* fentry/fexit/fmod_ret progs can also be sleepable if they are
>>                  * in the fmodret id set with the KF_SLEEPABLE flag.
>>                  */
>>                 else {
>>                     u32 *flags = btf_kfunc_is_modify_return(btf, btf_id,
>>                                         prog);
>>
>>                     if (flags && (*flags & KF_SLEEPABLE))
>>                         ret = 0;
>>                 }
>>                 break;
>>             [...]
>>             }
>>             if (ret) {
>>                 module_put(mod);
>>                 bpf_log(log, "%s is not sleepable\n", tname);
>>                 return ret;
>>             }
>>     }
>>
>> If I read that correctly, fentry must be on the error injection list
>> (which is populated by functions marked with ALLOW_ERROR_INJECTION when
>> CONFIG_FUNCTION_ERROR_INJECTION=y) to be allowed as sleepable.
>>
>> The other branch for fmodret id set seems to be only relevant for
>> kernel modules which explicily register their functions into
>> BTF_KFUNC_HOOK_FMODRET set.
>>
>>> While CONFIG_BPF_KPROBE_OVERRIDE affects a single helper.
>>>
>>> Both fentry and fentry.s should work fine on sys_nanosleep.
>>> fmod_ret is also completely independent of these two flags.
>>
>> For fmod_ret, we have a very similar code:
>>
>>     } else if (prog->expected_attach_type == BPF_MODIFY_RETURN) {
>>             [...]
>>             ret = -EINVAL;
>>             if (btf_kfunc_is_modify_return(btf, btf_id, prog) ||
>>                 !check_attach_modify_return(addr, tname))
>>                 ret = 0;
>>             if (ret) {
>>                 module_put(mod);
>>                 bpf_log(log, "%s() is not modifiable\n", tname);
>>                 return ret;
>>             }
>>     }
>>
>> with
>>
>>     static int check_attach_modify_return(unsigned long addr, const char *func_name)
>>     {
>>         if (within_error_injection_list(addr) ||
>>             !strncmp(SECURITY_PREFIX, func_name, sizeof(SECURITY_PREFIX) - 1))
> 
> Same thing here.
> Let's wrap within_error_injection_list() with bpf specific helper
> that in case of !CONFIG_FUNCTION_ERROR_INJECTION
> will allow syscalls anyway.

I would argue that fmod_ret should remain gated behind
CONFIG_ERROR_INJECTION. It's a similar concept to kprobe override - it
allows to override the execution of the original syscall and return a
custom error code.

> 
>> My question is: are sleepable and fmod_ret functions supposed to work
>> without error injection turned on?
> 
> Yes. since they're not injecting errors.
> 

Re: [PATCH bpf-next v3 0/3] Separate tests that need error injection

[Alexei Starovoitov]

On Wed, Feb 11, 2026 at 11:22 PM Viktor Malik <vmalik@redhat.com> wrote:
>
> On 2/12/26 02:42, Alexei Starovoitov wrote:
> > On Wed, Feb 11, 2026 at 5:55 AM Viktor Malik <vmalik@redhat.com> wrote:
> >>
> >> On 2/11/26 03:42, Alexei Starovoitov wrote:
> >>> On Tue, Feb 10, 2026 at 7:14 AM Viktor Malik <vmalik@redhat.com> wrote:
> >>>>
> >>>> Some enterprise kernels (such as RHEL) do not enable error injection via
> >>>> BPF (CONFIG_FUNCTION_ERROR_INJECTION and CONFIG_BPF_KPROBE_OVERRIDE).
> >>>> When running test_progs on such kernels, a lot of test cases fail since
> >>>> they use sleepable fentry or fmod_ret program types which require error
> >>>> injection to be enabled. While it is possible to skip these via custom
> >>>> DENYLIST, some test_progs are not properly split into subtests and
> >>>> therefore must be entirely skipped.
> >>>
> >>> Something is wrong.
> >>> -SEC("fentry/" SYS_PREFIX "sys_nanosleep")
> >>> -int do_probe_read(void *ctx)
> >>> +SEC("?fentry/" SYS_PREFIX "sys_nanosleep")
> >>> +int probe_read(void *ctx)
> >>>  {
> >>>         char buf[8];
> >>>
> >>> @@ -37,19 +37,19 @@ int do_probe_read(void *ctx)
> >>>         return 0;
> >>>  }
> >>>
> >>> -SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
> >>> -int do_copy_from_user(void *ctx)
> >>> +SEC("?fentry.s/" SYS_PREFIX "sys_nanosleep")
> >>>
> >>> sleepable should have nothing to do with CONFIG_FUNCTION_ERROR_INJECTION.
> >>
> >> I may be missing something but I think it is directly dependent, at
> >> least for fentry.
> >>
> >> In bpf_check_attach_target (kernel/bpf/verifier.c), we have:
> >>
> >>     if (prog->sleepable) {
> >>             ret = -EINVAL;
> >>             switch (prog->type) {
> >>             case BPF_PROG_TYPE_TRACING:
> >>
> >>                 /* fentry/fexit/fmod_ret progs can be sleepable if they are
> >>                  * attached to ALLOW_ERROR_INJECTION and are not in denylist.
> >>                  */
> >>                 if (!check_non_sleepable_error_inject(btf_id) &&
> >>                     within_error_injection_list(addr))
> >
> > Oh. That's an annoying side effect of disabling error inject.
> > fentry progs are not injecting any errors. They just use that
> > list as allowlist of good attach points.
> > Let's explicitly allow all syscalls here.
>
> Ok, I'll post a patch.
>
> >
> >>                     ret = 0;
> >>                 /* fentry/fexit/fmod_ret progs can also be sleepable if they are
> >>                  * in the fmodret id set with the KF_SLEEPABLE flag.
> >>                  */
> >>                 else {
> >>                     u32 *flags = btf_kfunc_is_modify_return(btf, btf_id,
> >>                                         prog);
> >>
> >>                     if (flags && (*flags & KF_SLEEPABLE))
> >>                         ret = 0;
> >>                 }
> >>                 break;
> >>             [...]
> >>             }
> >>             if (ret) {
> >>                 module_put(mod);
> >>                 bpf_log(log, "%s is not sleepable\n", tname);
> >>                 return ret;
> >>             }
> >>     }
> >>
> >> If I read that correctly, fentry must be on the error injection list
> >> (which is populated by functions marked with ALLOW_ERROR_INJECTION when
> >> CONFIG_FUNCTION_ERROR_INJECTION=y) to be allowed as sleepable.
> >>
> >> The other branch for fmodret id set seems to be only relevant for
> >> kernel modules which explicily register their functions into
> >> BTF_KFUNC_HOOK_FMODRET set.
> >>
> >>> While CONFIG_BPF_KPROBE_OVERRIDE affects a single helper.
> >>>
> >>> Both fentry and fentry.s should work fine on sys_nanosleep.
> >>> fmod_ret is also completely independent of these two flags.
> >>
> >> For fmod_ret, we have a very similar code:
> >>
> >>     } else if (prog->expected_attach_type == BPF_MODIFY_RETURN) {
> >>             [...]
> >>             ret = -EINVAL;
> >>             if (btf_kfunc_is_modify_return(btf, btf_id, prog) ||
> >>                 !check_attach_modify_return(addr, tname))
> >>                 ret = 0;
> >>             if (ret) {
> >>                 module_put(mod);
> >>                 bpf_log(log, "%s() is not modifiable\n", tname);
> >>                 return ret;
> >>             }
> >>     }
> >>
> >> with
> >>
> >>     static int check_attach_modify_return(unsigned long addr, const char *func_name)
> >>     {
> >>         if (within_error_injection_list(addr) ||
> >>             !strncmp(SECURITY_PREFIX, func_name, sizeof(SECURITY_PREFIX) - 1))
> >
> > Same thing here.
> > Let's wrap within_error_injection_list() with bpf specific helper
> > that in case of !CONFIG_FUNCTION_ERROR_INJECTION
> > will allow syscalls anyway.
>
> I would argue that fmod_ret should remain gated behind
> CONFIG_ERROR_INJECTION. It's a similar concept to kprobe override - it
> allows to override the execution of the original syscall and return a
> custom error code.

Definitely not. fmod_ret and error injection are two orthogonal concepts.
fmod_ret was introduced to be independent of error injection in the
first place. fmod_ret is a deliberate spot to allow bpf to change
return code
in production.
While error injection is a debug feature to stress test the kernel.
syscalls are in fmod_ret territory, since this is not debugging
and not stressing the kernel. ptrace can change syscall returns without
bpf, without fmod_ret, and without error injection.

Re: [PATCH bpf-next v3 0/3] Separate tests that need error injection

[Viktor Malik]

On 2/12/26 17:35, Alexei Starovoitov wrote:
> On Wed, Feb 11, 2026 at 11:22 PM Viktor Malik <vmalik@redhat.com> wrote:
>>
>> On 2/12/26 02:42, Alexei Starovoitov wrote:
>>> On Wed, Feb 11, 2026 at 5:55 AM Viktor Malik <vmalik@redhat.com> wrote:
>>>>
>>>> On 2/11/26 03:42, Alexei Starovoitov wrote:
>>>>> On Tue, Feb 10, 2026 at 7:14 AM Viktor Malik <vmalik@redhat.com> wrote:
>>>>>>
>>>>>> Some enterprise kernels (such as RHEL) do not enable error injection via
>>>>>> BPF (CONFIG_FUNCTION_ERROR_INJECTION and CONFIG_BPF_KPROBE_OVERRIDE).
>>>>>> When running test_progs on such kernels, a lot of test cases fail since
>>>>>> they use sleepable fentry or fmod_ret program types which require error
>>>>>> injection to be enabled. While it is possible to skip these via custom
>>>>>> DENYLIST, some test_progs are not properly split into subtests and
>>>>>> therefore must be entirely skipped.
>>>>>
>>>>> Something is wrong.
>>>>> -SEC("fentry/" SYS_PREFIX "sys_nanosleep")
>>>>> -int do_probe_read(void *ctx)
>>>>> +SEC("?fentry/" SYS_PREFIX "sys_nanosleep")
>>>>> +int probe_read(void *ctx)
>>>>>  {
>>>>>         char buf[8];
>>>>>
>>>>> @@ -37,19 +37,19 @@ int do_probe_read(void *ctx)
>>>>>         return 0;
>>>>>  }
>>>>>
>>>>> -SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
>>>>> -int do_copy_from_user(void *ctx)
>>>>> +SEC("?fentry.s/" SYS_PREFIX "sys_nanosleep")
>>>>>
>>>>> sleepable should have nothing to do with CONFIG_FUNCTION_ERROR_INJECTION.
>>>>
>>>> I may be missing something but I think it is directly dependent, at
>>>> least for fentry.
>>>>
>>>> In bpf_check_attach_target (kernel/bpf/verifier.c), we have:
>>>>
>>>>     if (prog->sleepable) {
>>>>             ret = -EINVAL;
>>>>             switch (prog->type) {
>>>>             case BPF_PROG_TYPE_TRACING:
>>>>
>>>>                 /* fentry/fexit/fmod_ret progs can be sleepable if they are
>>>>                  * attached to ALLOW_ERROR_INJECTION and are not in denylist.
>>>>                  */
>>>>                 if (!check_non_sleepable_error_inject(btf_id) &&
>>>>                     within_error_injection_list(addr))
>>>
>>> Oh. That's an annoying side effect of disabling error inject.
>>> fentry progs are not injecting any errors. They just use that
>>> list as allowlist of good attach points.
>>> Let's explicitly allow all syscalls here.
>>
>> Ok, I'll post a patch.
>>
>>>
>>>>                     ret = 0;
>>>>                 /* fentry/fexit/fmod_ret progs can also be sleepable if they are
>>>>                  * in the fmodret id set with the KF_SLEEPABLE flag.
>>>>                  */
>>>>                 else {
>>>>                     u32 *flags = btf_kfunc_is_modify_return(btf, btf_id,
>>>>                                         prog);
>>>>
>>>>                     if (flags && (*flags & KF_SLEEPABLE))
>>>>                         ret = 0;
>>>>                 }
>>>>                 break;
>>>>             [...]
>>>>             }
>>>>             if (ret) {
>>>>                 module_put(mod);
>>>>                 bpf_log(log, "%s is not sleepable\n", tname);
>>>>                 return ret;
>>>>             }
>>>>     }
>>>>
>>>> If I read that correctly, fentry must be on the error injection list
>>>> (which is populated by functions marked with ALLOW_ERROR_INJECTION when
>>>> CONFIG_FUNCTION_ERROR_INJECTION=y) to be allowed as sleepable.
>>>>
>>>> The other branch for fmodret id set seems to be only relevant for
>>>> kernel modules which explicily register their functions into
>>>> BTF_KFUNC_HOOK_FMODRET set.
>>>>
>>>>> While CONFIG_BPF_KPROBE_OVERRIDE affects a single helper.
>>>>>
>>>>> Both fentry and fentry.s should work fine on sys_nanosleep.
>>>>> fmod_ret is also completely independent of these two flags.
>>>>
>>>> For fmod_ret, we have a very similar code:
>>>>
>>>>     } else if (prog->expected_attach_type == BPF_MODIFY_RETURN) {
>>>>             [...]
>>>>             ret = -EINVAL;
>>>>             if (btf_kfunc_is_modify_return(btf, btf_id, prog) ||
>>>>                 !check_attach_modify_return(addr, tname))
>>>>                 ret = 0;
>>>>             if (ret) {
>>>>                 module_put(mod);
>>>>                 bpf_log(log, "%s() is not modifiable\n", tname);
>>>>                 return ret;
>>>>             }
>>>>     }
>>>>
>>>> with
>>>>
>>>>     static int check_attach_modify_return(unsigned long addr, const char *func_name)
>>>>     {
>>>>         if (within_error_injection_list(addr) ||
>>>>             !strncmp(SECURITY_PREFIX, func_name, sizeof(SECURITY_PREFIX) - 1))
>>>
>>> Same thing here.
>>> Let's wrap within_error_injection_list() with bpf specific helper
>>> that in case of !CONFIG_FUNCTION_ERROR_INJECTION
>>> will allow syscalls anyway.
>>
>> I would argue that fmod_ret should remain gated behind
>> CONFIG_ERROR_INJECTION. It's a similar concept to kprobe override - it
>> allows to override the execution of the original syscall and return a
>> custom error code.
> 
> Definitely not. fmod_ret and error injection are two orthogonal concepts.
> fmod_ret was introduced to be independent of error injection in the
> first place. fmod_ret is a deliberate spot to allow bpf to change
> return code
> in production.

I'm not sure that was the original intention of fmod_ret. Even the
original patch set mentions that error injection needs to be explicitly
enabled [1,2]. Changing that is IMHO a significant shift which needs
discussion.

[1] https://lore.kernel.org/bpf/20200304191853.1529-1-kpsingh@chromium.org/
[2] https://lore.kernel.org/bpf/20200304191853.1529-4-kpsingh@chromium.org/

> While error injection is a debug feature to stress test the kernel.
> syscalls are in fmod_ret territory, since this is not debugging
> and not stressing the kernel. ptrace can change syscall returns without
> bpf, without fmod_ret, and without error injection.

The thing I'm worried about here is that allowing BPF programs override
syscalls will esentially allow to implement a BPF-based rootkit. I'm
well aware that BPF is root-only (although there are ways to allow
unprivileged users run BPF, such as tokens) but if a potential attacker
gains root access even for a short time, they can install the rootkit
which may live unnoticed from that moment. Which is not something we
really want to enable in production. For ptrace, my understanding is
that it is significantly easier to detect if it is used.

If you don't mind, I'd start with only allowing sleepable syscalls (when
error injection is disabled) for now.

Viktor

Re: [PATCH bpf-next v3 0/3] Separate tests that need error injection

[Alexei Starovoitov]

On Fri, Feb 13, 2026 at 1:22 AM Viktor Malik <vmalik@redhat.com> wrote:
>
> On 2/12/26 17:35, Alexei Starovoitov wrote:
> > On Wed, Feb 11, 2026 at 11:22 PM Viktor Malik <vmalik@redhat.com> wrote:
> >>
> >> On 2/12/26 02:42, Alexei Starovoitov wrote:
> >>> On Wed, Feb 11, 2026 at 5:55 AM Viktor Malik <vmalik@redhat.com> wrote:
> >>>>
> >>>> On 2/11/26 03:42, Alexei Starovoitov wrote:
> >>>>> On Tue, Feb 10, 2026 at 7:14 AM Viktor Malik <vmalik@redhat.com> wrote:
> >>>>>>
> >>>>>> Some enterprise kernels (such as RHEL) do not enable error injection via
> >>>>>> BPF (CONFIG_FUNCTION_ERROR_INJECTION and CONFIG_BPF_KPROBE_OVERRIDE).
> >>>>>> When running test_progs on such kernels, a lot of test cases fail since
> >>>>>> they use sleepable fentry or fmod_ret program types which require error
> >>>>>> injection to be enabled. While it is possible to skip these via custom
> >>>>>> DENYLIST, some test_progs are not properly split into subtests and
> >>>>>> therefore must be entirely skipped.
> >>>>>
> >>>>> Something is wrong.
> >>>>> -SEC("fentry/" SYS_PREFIX "sys_nanosleep")
> >>>>> -int do_probe_read(void *ctx)
> >>>>> +SEC("?fentry/" SYS_PREFIX "sys_nanosleep")
> >>>>> +int probe_read(void *ctx)
> >>>>>  {
> >>>>>         char buf[8];
> >>>>>
> >>>>> @@ -37,19 +37,19 @@ int do_probe_read(void *ctx)
> >>>>>         return 0;
> >>>>>  }
> >>>>>
> >>>>> -SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
> >>>>> -int do_copy_from_user(void *ctx)
> >>>>> +SEC("?fentry.s/" SYS_PREFIX "sys_nanosleep")
> >>>>>
> >>>>> sleepable should have nothing to do with CONFIG_FUNCTION_ERROR_INJECTION.
> >>>>
> >>>> I may be missing something but I think it is directly dependent, at
> >>>> least for fentry.
> >>>>
> >>>> In bpf_check_attach_target (kernel/bpf/verifier.c), we have:
> >>>>
> >>>>     if (prog->sleepable) {
> >>>>             ret = -EINVAL;
> >>>>             switch (prog->type) {
> >>>>             case BPF_PROG_TYPE_TRACING:
> >>>>
> >>>>                 /* fentry/fexit/fmod_ret progs can be sleepable if they are
> >>>>                  * attached to ALLOW_ERROR_INJECTION and are not in denylist.
> >>>>                  */
> >>>>                 if (!check_non_sleepable_error_inject(btf_id) &&
> >>>>                     within_error_injection_list(addr))
> >>>
> >>> Oh. That's an annoying side effect of disabling error inject.
> >>> fentry progs are not injecting any errors. They just use that
> >>> list as allowlist of good attach points.
> >>> Let's explicitly allow all syscalls here.
> >>
> >> Ok, I'll post a patch.
> >>
> >>>
> >>>>                     ret = 0;
> >>>>                 /* fentry/fexit/fmod_ret progs can also be sleepable if they are
> >>>>                  * in the fmodret id set with the KF_SLEEPABLE flag.
> >>>>                  */
> >>>>                 else {
> >>>>                     u32 *flags = btf_kfunc_is_modify_return(btf, btf_id,
> >>>>                                         prog);
> >>>>
> >>>>                     if (flags && (*flags & KF_SLEEPABLE))
> >>>>                         ret = 0;
> >>>>                 }
> >>>>                 break;
> >>>>             [...]
> >>>>             }
> >>>>             if (ret) {
> >>>>                 module_put(mod);
> >>>>                 bpf_log(log, "%s is not sleepable\n", tname);
> >>>>                 return ret;
> >>>>             }
> >>>>     }
> >>>>
> >>>> If I read that correctly, fentry must be on the error injection list
> >>>> (which is populated by functions marked with ALLOW_ERROR_INJECTION when
> >>>> CONFIG_FUNCTION_ERROR_INJECTION=y) to be allowed as sleepable.
> >>>>
> >>>> The other branch for fmodret id set seems to be only relevant for
> >>>> kernel modules which explicily register their functions into
> >>>> BTF_KFUNC_HOOK_FMODRET set.
> >>>>
> >>>>> While CONFIG_BPF_KPROBE_OVERRIDE affects a single helper.
> >>>>>
> >>>>> Both fentry and fentry.s should work fine on sys_nanosleep.
> >>>>> fmod_ret is also completely independent of these two flags.
> >>>>
> >>>> For fmod_ret, we have a very similar code:
> >>>>
> >>>>     } else if (prog->expected_attach_type == BPF_MODIFY_RETURN) {
> >>>>             [...]
> >>>>             ret = -EINVAL;
> >>>>             if (btf_kfunc_is_modify_return(btf, btf_id, prog) ||
> >>>>                 !check_attach_modify_return(addr, tname))
> >>>>                 ret = 0;
> >>>>             if (ret) {
> >>>>                 module_put(mod);
> >>>>                 bpf_log(log, "%s() is not modifiable\n", tname);
> >>>>                 return ret;
> >>>>             }
> >>>>     }
> >>>>
> >>>> with
> >>>>
> >>>>     static int check_attach_modify_return(unsigned long addr, const char *func_name)
> >>>>     {
> >>>>         if (within_error_injection_list(addr) ||
> >>>>             !strncmp(SECURITY_PREFIX, func_name, sizeof(SECURITY_PREFIX) - 1))
> >>>
> >>> Same thing here.
> >>> Let's wrap within_error_injection_list() with bpf specific helper
> >>> that in case of !CONFIG_FUNCTION_ERROR_INJECTION
> >>> will allow syscalls anyway.
> >>
> >> I would argue that fmod_ret should remain gated behind
> >> CONFIG_ERROR_INJECTION. It's a similar concept to kprobe override - it
> >> allows to override the execution of the original syscall and return a
> >> custom error code.
> >
> > Definitely not. fmod_ret and error injection are two orthogonal concepts.
> > fmod_ret was introduced to be independent of error injection in the
> > first place. fmod_ret is a deliberate spot to allow bpf to change
> > return code
> > in production.
>
> I'm not sure that was the original intention of fmod_ret. Even the
> original patch set mentions that error injection needs to be explicitly
> enabled [1,2]. Changing that is IMHO a significant shift which needs
> discussion.

That discussion already happened when error_injection was forked
as a config flag and went to default N.

>
> > While error injection is a debug feature to stress test the kernel.
> > syscalls are in fmod_ret territory, since this is not debugging
> > and not stressing the kernel. ptrace can change syscall returns without
> > bpf, without fmod_ret, and without error injection.
>
> The thing I'm worried about here is that allowing BPF programs override
> syscalls will esentially allow to implement a BPF-based rootkit. I'm
> well aware that BPF is root-only (although there are ways to allow
> unprivileged users run BPF, such as tokens) but if a potential attacker
> gains root access even for a short time, they can install the rootkit
> which may live unnoticed from that moment. Which is not something we
> really want to enable in production. For ptrace, my understanding is
> that it is significantly easier to detect if it is used.
>
> If you don't mind, I'd start with only allowing sleepable syscalls (when
> error injection is disabled) for now.

I mind and I hate the reasoning that in kernels with error_injection=y
bpf is prone to be a rootkit. This is just a security circus
detached from reality.

Re: [PATCH bpf-next v3 0/3] Separate tests that need error injection

[Viktor Malik]

On 2/14/26 00:13, Alexei Starovoitov wrote:
> On Fri, Feb 13, 2026 at 1:22 AM Viktor Malik <vmalik@redhat.com> wrote:
>>
>> On 2/12/26 17:35, Alexei Starovoitov wrote:
>>> On Wed, Feb 11, 2026 at 11:22 PM Viktor Malik <vmalik@redhat.com> wrote:
>>>>
>>>> On 2/12/26 02:42, Alexei Starovoitov wrote:
>>>>> On Wed, Feb 11, 2026 at 5:55 AM Viktor Malik <vmalik@redhat.com> wrote:
>>>>>>
>>>>>> On 2/11/26 03:42, Alexei Starovoitov wrote:
>>>>>>> On Tue, Feb 10, 2026 at 7:14 AM Viktor Malik <vmalik@redhat.com> wrote:
>>>>>>>>
>>>>>>>> Some enterprise kernels (such as RHEL) do not enable error injection via
>>>>>>>> BPF (CONFIG_FUNCTION_ERROR_INJECTION and CONFIG_BPF_KPROBE_OVERRIDE).
>>>>>>>> When running test_progs on such kernels, a lot of test cases fail since
>>>>>>>> they use sleepable fentry or fmod_ret program types which require error
>>>>>>>> injection to be enabled. While it is possible to skip these via custom
>>>>>>>> DENYLIST, some test_progs are not properly split into subtests and
>>>>>>>> therefore must be entirely skipped.
>>>>>>>
>>>>>>> Something is wrong.
>>>>>>> -SEC("fentry/" SYS_PREFIX "sys_nanosleep")
>>>>>>> -int do_probe_read(void *ctx)
>>>>>>> +SEC("?fentry/" SYS_PREFIX "sys_nanosleep")
>>>>>>> +int probe_read(void *ctx)
>>>>>>>  {
>>>>>>>         char buf[8];
>>>>>>>
>>>>>>> @@ -37,19 +37,19 @@ int do_probe_read(void *ctx)
>>>>>>>         return 0;
>>>>>>>  }
>>>>>>>
>>>>>>> -SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
>>>>>>> -int do_copy_from_user(void *ctx)
>>>>>>> +SEC("?fentry.s/" SYS_PREFIX "sys_nanosleep")
>>>>>>>
>>>>>>> sleepable should have nothing to do with CONFIG_FUNCTION_ERROR_INJECTION.
>>>>>>
>>>>>> I may be missing something but I think it is directly dependent, at
>>>>>> least for fentry.
>>>>>>
>>>>>> In bpf_check_attach_target (kernel/bpf/verifier.c), we have:
>>>>>>
>>>>>>     if (prog->sleepable) {
>>>>>>             ret = -EINVAL;
>>>>>>             switch (prog->type) {
>>>>>>             case BPF_PROG_TYPE_TRACING:
>>>>>>
>>>>>>                 /* fentry/fexit/fmod_ret progs can be sleepable if they are
>>>>>>                  * attached to ALLOW_ERROR_INJECTION and are not in denylist.
>>>>>>                  */
>>>>>>                 if (!check_non_sleepable_error_inject(btf_id) &&
>>>>>>                     within_error_injection_list(addr))
>>>>>
>>>>> Oh. That's an annoying side effect of disabling error inject.
>>>>> fentry progs are not injecting any errors. They just use that
>>>>> list as allowlist of good attach points.
>>>>> Let's explicitly allow all syscalls here.
>>>>
>>>> Ok, I'll post a patch.
>>>>
>>>>>
>>>>>>                     ret = 0;
>>>>>>                 /* fentry/fexit/fmod_ret progs can also be sleepable if they are
>>>>>>                  * in the fmodret id set with the KF_SLEEPABLE flag.
>>>>>>                  */
>>>>>>                 else {
>>>>>>                     u32 *flags = btf_kfunc_is_modify_return(btf, btf_id,
>>>>>>                                         prog);
>>>>>>
>>>>>>                     if (flags && (*flags & KF_SLEEPABLE))
>>>>>>                         ret = 0;
>>>>>>                 }
>>>>>>                 break;
>>>>>>             [...]
>>>>>>             }
>>>>>>             if (ret) {
>>>>>>                 module_put(mod);
>>>>>>                 bpf_log(log, "%s is not sleepable\n", tname);
>>>>>>                 return ret;
>>>>>>             }
>>>>>>     }
>>>>>>
>>>>>> If I read that correctly, fentry must be on the error injection list
>>>>>> (which is populated by functions marked with ALLOW_ERROR_INJECTION when
>>>>>> CONFIG_FUNCTION_ERROR_INJECTION=y) to be allowed as sleepable.
>>>>>>
>>>>>> The other branch for fmodret id set seems to be only relevant for
>>>>>> kernel modules which explicily register their functions into
>>>>>> BTF_KFUNC_HOOK_FMODRET set.
>>>>>>
>>>>>>> While CONFIG_BPF_KPROBE_OVERRIDE affects a single helper.
>>>>>>>
>>>>>>> Both fentry and fentry.s should work fine on sys_nanosleep.
>>>>>>> fmod_ret is also completely independent of these two flags.
>>>>>>
>>>>>> For fmod_ret, we have a very similar code:
>>>>>>
>>>>>>     } else if (prog->expected_attach_type == BPF_MODIFY_RETURN) {
>>>>>>             [...]
>>>>>>             ret = -EINVAL;
>>>>>>             if (btf_kfunc_is_modify_return(btf, btf_id, prog) ||
>>>>>>                 !check_attach_modify_return(addr, tname))
>>>>>>                 ret = 0;
>>>>>>             if (ret) {
>>>>>>                 module_put(mod);
>>>>>>                 bpf_log(log, "%s() is not modifiable\n", tname);
>>>>>>                 return ret;
>>>>>>             }
>>>>>>     }
>>>>>>
>>>>>> with
>>>>>>
>>>>>>     static int check_attach_modify_return(unsigned long addr, const char *func_name)
>>>>>>     {
>>>>>>         if (within_error_injection_list(addr) ||
>>>>>>             !strncmp(SECURITY_PREFIX, func_name, sizeof(SECURITY_PREFIX) - 1))
>>>>>
>>>>> Same thing here.
>>>>> Let's wrap within_error_injection_list() with bpf specific helper
>>>>> that in case of !CONFIG_FUNCTION_ERROR_INJECTION
>>>>> will allow syscalls anyway.
>>>>
>>>> I would argue that fmod_ret should remain gated behind
>>>> CONFIG_ERROR_INJECTION. It's a similar concept to kprobe override - it
>>>> allows to override the execution of the original syscall and return a
>>>> custom error code.
>>>
>>> Definitely not. fmod_ret and error injection are two orthogonal concepts.
>>> fmod_ret was introduced to be independent of error injection in the
>>> first place. fmod_ret is a deliberate spot to allow bpf to change
>>> return code
>>> in production.
>>
>> I'm not sure that was the original intention of fmod_ret. Even the
>> original patch set mentions that error injection needs to be explicitly
>> enabled [1,2]. Changing that is IMHO a significant shift which needs
>> discussion.
> 
> That discussion already happened when error_injection was forked
> as a config flag and went to default N.

Then I'm a bit confused about the purpose of error_injection. The
description of the config option says:

    Add fault injections into various functions that are annotated with
    ALLOW_ERROR_INJECTION() in the kernel. BPF may also modify the return
    value of these functions. This is useful to test error paths of code.

    If unsure, say N

But if you say that fmod_ret and error_injection are orthogonal
concepts, then what is error_injection good for without fmod_ret?

>>
>>> While error injection is a debug feature to stress test the kernel.
>>> syscalls are in fmod_ret territory, since this is not debugging
>>> and not stressing the kernel. ptrace can change syscall returns without
>>> bpf, without fmod_ret, and without error injection.
>>
>> The thing I'm worried about here is that allowing BPF programs override
>> syscalls will esentially allow to implement a BPF-based rootkit. I'm
>> well aware that BPF is root-only (although there are ways to allow
>> unprivileged users run BPF, such as tokens) but if a potential attacker
>> gains root access even for a short time, they can install the rootkit
>> which may live unnoticed from that moment. Which is not something we
>> really want to enable in production. For ptrace, my understanding is
>> that it is significantly easier to detect if it is used.
>>
>> If you don't mind, I'd start with only allowing sleepable syscalls (when
>> error injection is disabled) for now.
> 
> I mind and I hate the reasoning that in kernels with error_injection=y
> bpf is prone to be a rootkit. This is just a security circus
> detached from reality.

Could you share a bit more details why? I'm not a security expert myself
but it seems to me that intercepting syscalls could be used to implement
a rootkit. For instance [1] does something similar, although they use
bpf_override_return in syscall kprobes. So, I'd love to understand why
this is detached from reality.

Thanks!
Viktor

[1] https://github.com/Gui774ume/ebpfkit

Re: [PATCH bpf-next v3 0/3] Separate tests that need error injection

[Viktor Malik]

On 2/16/26 08:46, Viktor Malik wrote:
> On 2/14/26 00:13, Alexei Starovoitov wrote:
>> On Fri, Feb 13, 2026 at 1:22 AM Viktor Malik <vmalik@redhat.com> wrote:
>>>
>>> On 2/12/26 17:35, Alexei Starovoitov wrote:
>>>> On Wed, Feb 11, 2026 at 11:22 PM Viktor Malik <vmalik@redhat.com> wrote:
>>>>>
>>>>> On 2/12/26 02:42, Alexei Starovoitov wrote:
>>>>>> On Wed, Feb 11, 2026 at 5:55 AM Viktor Malik <vmalik@redhat.com> wrote:
>>>>>>>
>>>>>>> On 2/11/26 03:42, Alexei Starovoitov wrote:
>>>>>>>> On Tue, Feb 10, 2026 at 7:14 AM Viktor Malik <vmalik@redhat.com> wrote:
>>>>>>>>>
>>>>>>>>> Some enterprise kernels (such as RHEL) do not enable error injection via
>>>>>>>>> BPF (CONFIG_FUNCTION_ERROR_INJECTION and CONFIG_BPF_KPROBE_OVERRIDE).
>>>>>>>>> When running test_progs on such kernels, a lot of test cases fail since
>>>>>>>>> they use sleepable fentry or fmod_ret program types which require error
>>>>>>>>> injection to be enabled. While it is possible to skip these via custom
>>>>>>>>> DENYLIST, some test_progs are not properly split into subtests and
>>>>>>>>> therefore must be entirely skipped.
>>>>>>>>
>>>>>>>> Something is wrong.
>>>>>>>> -SEC("fentry/" SYS_PREFIX "sys_nanosleep")
>>>>>>>> -int do_probe_read(void *ctx)
>>>>>>>> +SEC("?fentry/" SYS_PREFIX "sys_nanosleep")
>>>>>>>> +int probe_read(void *ctx)
>>>>>>>>  {
>>>>>>>>         char buf[8];
>>>>>>>>
>>>>>>>> @@ -37,19 +37,19 @@ int do_probe_read(void *ctx)
>>>>>>>>         return 0;
>>>>>>>>  }
>>>>>>>>
>>>>>>>> -SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
>>>>>>>> -int do_copy_from_user(void *ctx)
>>>>>>>> +SEC("?fentry.s/" SYS_PREFIX "sys_nanosleep")
>>>>>>>>
>>>>>>>> sleepable should have nothing to do with CONFIG_FUNCTION_ERROR_INJECTION.
>>>>>>>
>>>>>>> I may be missing something but I think it is directly dependent, at
>>>>>>> least for fentry.
>>>>>>>
>>>>>>> In bpf_check_attach_target (kernel/bpf/verifier.c), we have:
>>>>>>>
>>>>>>>     if (prog->sleepable) {
>>>>>>>             ret = -EINVAL;
>>>>>>>             switch (prog->type) {
>>>>>>>             case BPF_PROG_TYPE_TRACING:
>>>>>>>
>>>>>>>                 /* fentry/fexit/fmod_ret progs can be sleepable if they are
>>>>>>>                  * attached to ALLOW_ERROR_INJECTION and are not in denylist.
>>>>>>>                  */
>>>>>>>                 if (!check_non_sleepable_error_inject(btf_id) &&
>>>>>>>                     within_error_injection_list(addr))
>>>>>>
>>>>>> Oh. That's an annoying side effect of disabling error inject.
>>>>>> fentry progs are not injecting any errors. They just use that
>>>>>> list as allowlist of good attach points.
>>>>>> Let's explicitly allow all syscalls here.
>>>>>
>>>>> Ok, I'll post a patch.
>>>>>
>>>>>>
>>>>>>>                     ret = 0;
>>>>>>>                 /* fentry/fexit/fmod_ret progs can also be sleepable if they are
>>>>>>>                  * in the fmodret id set with the KF_SLEEPABLE flag.
>>>>>>>                  */
>>>>>>>                 else {
>>>>>>>                     u32 *flags = btf_kfunc_is_modify_return(btf, btf_id,
>>>>>>>                                         prog);
>>>>>>>
>>>>>>>                     if (flags && (*flags & KF_SLEEPABLE))
>>>>>>>                         ret = 0;
>>>>>>>                 }
>>>>>>>                 break;
>>>>>>>             [...]
>>>>>>>             }
>>>>>>>             if (ret) {
>>>>>>>                 module_put(mod);
>>>>>>>                 bpf_log(log, "%s is not sleepable\n", tname);
>>>>>>>                 return ret;
>>>>>>>             }
>>>>>>>     }
>>>>>>>
>>>>>>> If I read that correctly, fentry must be on the error injection list
>>>>>>> (which is populated by functions marked with ALLOW_ERROR_INJECTION when
>>>>>>> CONFIG_FUNCTION_ERROR_INJECTION=y) to be allowed as sleepable.
>>>>>>>
>>>>>>> The other branch for fmodret id set seems to be only relevant for
>>>>>>> kernel modules which explicily register their functions into
>>>>>>> BTF_KFUNC_HOOK_FMODRET set.
>>>>>>>
>>>>>>>> While CONFIG_BPF_KPROBE_OVERRIDE affects a single helper.
>>>>>>>>
>>>>>>>> Both fentry and fentry.s should work fine on sys_nanosleep.
>>>>>>>> fmod_ret is also completely independent of these two flags.
>>>>>>>
>>>>>>> For fmod_ret, we have a very similar code:
>>>>>>>
>>>>>>>     } else if (prog->expected_attach_type == BPF_MODIFY_RETURN) {
>>>>>>>             [...]
>>>>>>>             ret = -EINVAL;
>>>>>>>             if (btf_kfunc_is_modify_return(btf, btf_id, prog) ||
>>>>>>>                 !check_attach_modify_return(addr, tname))
>>>>>>>                 ret = 0;
>>>>>>>             if (ret) {
>>>>>>>                 module_put(mod);
>>>>>>>                 bpf_log(log, "%s() is not modifiable\n", tname);
>>>>>>>                 return ret;
>>>>>>>             }
>>>>>>>     }
>>>>>>>
>>>>>>> with
>>>>>>>
>>>>>>>     static int check_attach_modify_return(unsigned long addr, const char *func_name)
>>>>>>>     {
>>>>>>>         if (within_error_injection_list(addr) ||
>>>>>>>             !strncmp(SECURITY_PREFIX, func_name, sizeof(SECURITY_PREFIX) - 1))
>>>>>>
>>>>>> Same thing here.
>>>>>> Let's wrap within_error_injection_list() with bpf specific helper
>>>>>> that in case of !CONFIG_FUNCTION_ERROR_INJECTION
>>>>>> will allow syscalls anyway.
>>>>>
>>>>> I would argue that fmod_ret should remain gated behind
>>>>> CONFIG_ERROR_INJECTION. It's a similar concept to kprobe override - it
>>>>> allows to override the execution of the original syscall and return a
>>>>> custom error code.
>>>>
>>>> Definitely not. fmod_ret and error injection are two orthogonal concepts.
>>>> fmod_ret was introduced to be independent of error injection in the
>>>> first place. fmod_ret is a deliberate spot to allow bpf to change
>>>> return code
>>>> in production.
>>>
>>> I'm not sure that was the original intention of fmod_ret. Even the
>>> original patch set mentions that error injection needs to be explicitly
>>> enabled [1,2]. Changing that is IMHO a significant shift which needs
>>> discussion.
>>
>> That discussion already happened when error_injection was forked
>> as a config flag and went to default N.
> 
> Then I'm a bit confused about the purpose of error_injection. The
> description of the config option says:
> 
>     Add fault injections into various functions that are annotated with
>     ALLOW_ERROR_INJECTION() in the kernel. BPF may also modify the return
>     value of these functions. This is useful to test error paths of code.
> 
>     If unsure, say N
> 
> But if you say that fmod_ret and error_injection are orthogonal
> concepts, then what is error_injection good for without fmod_ret?
> 
>>>
>>>> While error injection is a debug feature to stress test the kernel.
>>>> syscalls are in fmod_ret territory, since this is not debugging
>>>> and not stressing the kernel. ptrace can change syscall returns without
>>>> bpf, without fmod_ret, and without error injection.
>>>
>>> The thing I'm worried about here is that allowing BPF programs override
>>> syscalls will esentially allow to implement a BPF-based rootkit. I'm
>>> well aware that BPF is root-only (although there are ways to allow
>>> unprivileged users run BPF, such as tokens) but if a potential attacker
>>> gains root access even for a short time, they can install the rootkit
>>> which may live unnoticed from that moment. Which is not something we
>>> really want to enable in production. For ptrace, my understanding is
>>> that it is significantly easier to detect if it is used.
>>>
>>> If you don't mind, I'd start with only allowing sleepable syscalls (when
>>> error injection is disabled) for now.
>>
>> I mind and I hate the reasoning that in kernels with error_injection=y
>> bpf is prone to be a rootkit. This is just a security circus
>> detached from reality.
> 
> Could you share a bit more details why? I'm not a security expert myself
> but it seems to me that intercepting syscalls could be used to implement
> a rootkit. For instance [1] does something similar, although they use
> bpf_override_return in syscall kprobes. So, I'd love to understand why
> this is detached from reality.

Looking into this more, I assume that the premise here is that injecting
errors into syscalls is not sufficient to implement a rootkit, more
mechanisms are necessary for that (even [1] uses bpf_probe_write_user
which is by default enabled on any kernel). On top of that, there are
tons of other ways to compromise a system using eBPF so enabling error
injection doesn't change much in that direction.

With that, I'm ok with enabling fmod_ret for syscalls even with
CONFIG_ERROR_INJECTION=n, I'll send a patch.

> 
> Thanks!
> Viktor
> 
> [1] https://github.com/Gui774ume/ebpfkit

Re: [PATCH bpf-next v3 0/3] Separate tests that need error injection

[Alexei Starovoitov]

On Mon, Feb 23, 2026 at 3:52 AM Viktor Malik <vmalik@redhat.com> wrote:
>
>
> Looking into this more, I assume that the premise here is that injecting
> errors into syscalls is not sufficient to implement a rootkit, more
> mechanisms are necessary for that (even [1] uses bpf_probe_write_user
> which is by default enabled on any kernel). On top of that, there are
> tons of other ways to compromise a system using eBPF so enabling error
> injection doesn't change much in that direction.
>
> With that, I'm ok with enabling fmod_ret for syscalls even with
> CONFIG_ERROR_INJECTION=n, I'll send a patch.

Now we're on the same page. Thanks

Re: [PATCH bpf-next v3 0/3] Separate tests that need error injection

[Viktor Malik]

On 2/24/26 03:58, Alexei Starovoitov wrote:
> On Mon, Feb 23, 2026 at 3:52 AM Viktor Malik <vmalik@redhat.com> wrote:
>>
>>
>> Looking into this more, I assume that the premise here is that injecting
>> errors into syscalls is not sufficient to implement a rootkit, more
>> mechanisms are necessary for that (even [1] uses bpf_probe_write_user
>> which is by default enabled on any kernel). On top of that, there are
>> tons of other ways to compromise a system using eBPF so enabling error
>> injection doesn't change much in that direction.
>>
>> With that, I'm ok with enabling fmod_ret for syscalls even with
>> CONFIG_ERROR_INJECTION=n, I'll send a patch.
> 
> Now we're on the same page. Thanks

Thanks for the confirmation.

The first patch of this series still looks reasonable to me since the
module_attach test flattens different test cases together. I'll send a
new version with just that first patch.