All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent
@ 2026-07-29  0:59 Andrey Grodzovsky
  2026-07-29  0:59 ` [RFC PATCH bpf-next 1/3] bpf: mark trampoline " Andrey Grodzovsky
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Andrey Grodzovsky @ 2026-07-29  0:59 UTC (permalink / raw)
  To: bpf, linux-trace-kernel, linux-kselftest
  Cc: ast, daniel, andrii, song, jolsa, rostedt, naveen, davem,
	mhiramat, stable, linux-open-source

This fixes a long-standing issue: kernel.ftrace_enabled=0 silently
disables BPF trampolines (fentry/fexit) and ftrace-based
kprobes/kretprobes. The write succeeds, the hook stops firing with no
error, and re-enabling silently restores it. Livepatch already solved
this for itself via FTRACE_OPS_FL_PERMANENT, which refuses to disable
ftrace while a permanent ops is registered and refuses to register
one while ftrace is disabled[1].

For trampolines this restores a historical property: from 2019-2022
they shared one global direct_ops, marked permanent the same
way[2].It was later lost as a side effect of the 2022
per-trampoline-ops split (patch 1's Fixes tag) and never
restored.[3][4] Kprobes never carried this protection at all, so
for them that is long-standing issue rather than a regression.

Patch 1: trampolines. Patch 2: classic kprobes/kretprobes. Patch 3:
a selftest covering both directions for all four hook types.

P.S
I initially implemented a per-record opt-in flag[5], but dropped it
as over-engineering once I saw the original blanket restriction.

P.P.S
Open question: kprobe.multi/kretprobe.multi/kprobe.session
(fprobe-backed) aren't covered -- return-capturing fprobes share the
function-graph tracer's subops manager with unrelated tracers, so
marking it permanent needs a different, per-record approach. Perhaps
something along the lines of [5].

[1] - https://lore.kernel.org/all/20191016113316.13415-1-mbenes@suse.cz/T/#u
[2] - https://lore.kernel.org/all/20191108213450.032003836@goodmis.org/
[3] - https://lore.kernel.org/all/20220602193706.2607681-1-song@kernel.org/
[4] - https://lore.kernel.org/bpf/20251230145010.103439-1-jolsa@kernel.org/
[5] - https://github.com/kernel-patches/bpf/compare/bpf-next_base...andrey-grodzovsky:bpf:ftrace-permanent-per-record

Andrey Grodzovsky (3):
  bpf: mark trampoline ftrace_ops permanent
  kprobes: mark ftrace-based kprobe ops permanent
  selftests/bpf: add ftrace_permanent test

 kernel/bpf/trampoline.c                       |   4 +
 kernel/kprobes.c                              |   5 +-
 .../bpf/prog_tests/ftrace_permanent.c         | 144 ++++++++++++++++++
 .../selftests/bpf/progs/ftrace_permanent.c    |  43 ++++++
 4 files changed, 194 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/ftrace_permanent.c
 create mode 100644 tools/testing/selftests/bpf/progs/ftrace_permanent.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH bpf-next 1/3] bpf: mark trampoline ftrace_ops permanent
  2026-07-29  0:59 [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent Andrey Grodzovsky
@ 2026-07-29  0:59 ` Andrey Grodzovsky
  2026-07-29 13:29   ` Jiri Olsa
  2026-07-29  0:59 ` [RFC PATCH bpf-next 2/3] kprobes: mark ftrace-based kprobe ops permanent Andrey Grodzovsky
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Andrey Grodzovsky @ 2026-07-29  0:59 UTC (permalink / raw)
  To: bpf, linux-trace-kernel, linux-kselftest
  Cc: ast, daniel, andrii, song, jolsa, rostedt, naveen, davem,
	mhiramat, stable, linux-open-source

kernel.ftrace_enabled=0 silently kills BPF trampolines (fentry/fexit):
attach still succeeds, the hook stops firing with no error, and
re-enabling silently restores it with no indication anything happened.

This is a regression, not a new gap: BPF trampolines were already
protected against this the same way livepatch protects itself, via
FTRACE_OPS_FL_PERMANENT on the shared trampoline ftrace_ops. That
protection was silently dropped during a later refactoring and never
restored, so this has been broken for years.

Both the shared direct_ops (CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS
arches) and the per-trampoline ops allocated on other arches (arm64,
s390) are BPF-trampoline-private, not shared with any unrelated
subsystem, so restoring the same unconditional flag is safe and needs
no kernel/trace/ftrace.c changes -- its existing generic sysctl gate
and attach-time refusal already scan the ops list and pick this up
for free.

Fixes: 00963a2e75a8 ("bpf: Support bpf_trampoline on functions with IPMODIFY (e.g. livepatch)")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
---
 kernel/bpf/trampoline.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 129d07db117e..3d5069091db7 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -224,6 +224,8 @@ void bpf_image_ksym_del(struct bpf_ksym *ksym)
  */
 struct ftrace_ops direct_ops = {
 	.ops_func = bpf_tramp_ftrace_ops_func,
+	/* Same protection livepatch gives its own ftrace_ops. */
+	.flags = FTRACE_OPS_FL_PERMANENT,
 };
 
 static int direct_ops_alloc(struct bpf_trampoline *tr)
@@ -303,6 +305,8 @@ static int direct_ops_alloc(struct bpf_trampoline *tr)
 		return -ENOMEM;
 	tr->fops->private = tr;
 	tr->fops->ops_func = bpf_tramp_ftrace_ops_func;
+	/* See the direct_ops initializer above for why. */
+	tr->fops->flags |= FTRACE_OPS_FL_PERMANENT;
 	return 0;
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH bpf-next 2/3] kprobes: mark ftrace-based kprobe ops permanent
  2026-07-29  0:59 [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent Andrey Grodzovsky
  2026-07-29  0:59 ` [RFC PATCH bpf-next 1/3] bpf: mark trampoline " Andrey Grodzovsky
@ 2026-07-29  0:59 ` Andrey Grodzovsky
  2026-07-29  1:39   ` sashiko-bot
  2026-07-29  0:59 ` [RFC PATCH bpf-next 3/3] selftests/bpf: add ftrace_permanent test Andrey Grodzovsky
  2026-07-29 14:41 ` [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent Steven Rostedt
  3 siblings, 1 reply; 13+ messages in thread
From: Andrey Grodzovsky @ 2026-07-29  0:59 UTC (permalink / raw)
  To: bpf, linux-trace-kernel, linux-kselftest
  Cc: ast, daniel, andrii, song, jolsa, rostedt, naveen, davem,
	mhiramat, stable, linux-open-source

kernel.ftrace_enabled=0 silently kills ftrace-based kprobes/kretprobes
the same way it does BPF trampolines: arm succeeds, the probe stops
firing with no error, and re-enabling silently restores it. Unlike
trampolines this is not a regression -- ftrace-based kprobes have
never carried this protection, so the bug is long-standing.

kprobe_ftrace_ops/kprobe_ipmodify_ops are shared only among
ftrace-based kprobe attachers, not with any other tracer, so the same
unconditional FTRACE_OPS_FL_PERMANENT livepatch already uses applies
cleanly here too, with no kernel/trace/ftrace.c changes needed.

Known limitation: kprobe.multi/kretprobe.multi/kprobe.session
(fprobe-backed, kernel/trace/fprobe.c) are not covered. Entry-only
fprobes could take the same fix, but return-capturing fprobes route
through the function-graph tracer's shared subops manager, which is
also used by unrelated tracers (function_graph, irqsoff, wakeup
latency, function profiler) -- marking it permanent would block
ftrace_enabled=0 for those too. Left for a follow-up.

Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
---
 kernel/kprobes.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index bfc89083daa9..5a54511eee79 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1122,14 +1122,15 @@ static struct kprobe *alloc_aggr_kprobe(struct kprobe *p)
 #endif /* CONFIG_OPTPROBES */
 
 #ifdef CONFIG_KPROBES_ON_FTRACE
+/* Same protection livepatch gives its own ftrace_ops. */
 static struct ftrace_ops kprobe_ftrace_ops __read_mostly = {
 	.func = kprobe_ftrace_handler,
-	.flags = FTRACE_OPS_FL_SAVE_REGS,
+	.flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_PERMANENT,
 };
 
 static struct ftrace_ops kprobe_ipmodify_ops __read_mostly = {
 	.func = kprobe_ftrace_handler,
-	.flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_IPMODIFY,
+	.flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_IPMODIFY | FTRACE_OPS_FL_PERMANENT,
 };
 
 static int kprobe_ipmodify_enabled;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH bpf-next 3/3] selftests/bpf: add ftrace_permanent test
  2026-07-29  0:59 [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent Andrey Grodzovsky
  2026-07-29  0:59 ` [RFC PATCH bpf-next 1/3] bpf: mark trampoline " Andrey Grodzovsky
  2026-07-29  0:59 ` [RFC PATCH bpf-next 2/3] kprobes: mark ftrace-based kprobe ops permanent Andrey Grodzovsky
@ 2026-07-29  0:59 ` Andrey Grodzovsky
  2026-07-29  1:31   ` sashiko-bot
  2026-07-29 13:23   ` Jiri Olsa
  2026-07-29 14:41 ` [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent Steven Rostedt
  3 siblings, 2 replies; 13+ messages in thread
From: Andrey Grodzovsky @ 2026-07-29  0:59 UTC (permalink / raw)
  To: bpf, linux-trace-kernel, linux-kselftest
  Cc: ast, daniel, andrii, song, jolsa, rostedt, naveen, davem,
	mhiramat, stable, linux-open-source

Cover the FTRACE_OPS_FL_PERMANENT fix on BPF trampolines and
ftrace-based kprobes/kretprobes: attach a fentry, fexit, kprobe, and
kretprobe program in turn and confirm kernel.ftrace_enabled=0 is
refused with EBUSY while attached, then succeeds once detached. Also
confirm a new fentry/kprobe attach is itself refused while
ftrace_enabled=0 is already in effect.

kprobe.multi/kretprobe.multi/kprobe.session are intentionally not
covered, matching the production change's scope.

Skip the kprobe/kretprobe subtests when CONFIG_KPROBES_ON_FTRACE is
off (e.g. arm64), read back via skel->kconfig->CONFIG_KPROBES_ON_FTRACE
(same __kconfig idiom as progs/test_fill_link_info.c).

Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
---
 .../bpf/prog_tests/ftrace_permanent.c         | 144 ++++++++++++++++++
 .../selftests/bpf/progs/ftrace_permanent.c    |  43 ++++++
 2 files changed, 187 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/ftrace_permanent.c
 create mode 100644 tools/testing/selftests/bpf/progs/ftrace_permanent.c

diff --git a/tools/testing/selftests/bpf/prog_tests/ftrace_permanent.c b/tools/testing/selftests/bpf/prog_tests/ftrace_permanent.c
new file mode 100644
index 000000000000..dbe78009a291
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/ftrace_permanent.c
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 CrowdStrike */
+#include <test_progs.h>
+#include "ftrace_permanent.skel.h"
+
+/*
+ * fentry/fexit/kprobe/kretprobe now carry FTRACE_OPS_FL_PERMANENT, so
+ * kernel.ftrace_enabled=0 must be refused with EBUSY while any of them
+ * is attached, and allowed again once detached.
+ *
+ * kprobe.multi/kretprobe.multi/kprobe.session are out of scope.
+ * kprobe/kretprobe subtests need CONFIG_KPROBES_ON_FTRACE (e.g. not on
+ * arm64) and are skipped otherwise.
+ */
+
+#define FTRACE_ENABLED_PATH "/proc/sys/kernel/ftrace_enabled"
+
+static int read_ftrace_enabled(int *val)
+{
+	char buf[16] = {};
+	int fd, n;
+
+	fd = open(FTRACE_ENABLED_PATH, O_RDONLY);
+	if (fd < 0)
+		return -errno;
+	n = read(fd, buf, sizeof(buf) - 1);
+	close(fd);
+	if (n <= 0)
+		return -EIO;
+	*val = atoi(buf);
+	return 0;
+}
+
+/* Returns 0 on success, or -errno on write failure. */
+static int write_ftrace_enabled(int val)
+{
+	char buf[4];
+	int fd, n, len, err = 0;
+
+	fd = open(FTRACE_ENABLED_PATH, O_WRONLY);
+	if (fd < 0)
+		return -errno;
+	len = snprintf(buf, sizeof(buf), "%d", val);
+	n = write(fd, buf, len);
+	if (n < 0)
+		err = -errno;
+	close(fd);
+	return err;
+}
+
+/*
+ * Attach @prog, assert kernel.ftrace_enabled=0 is refused while attached
+ * and stays at 1, then detach and assert the disable now succeeds.
+ */
+static void check_blocks_disable(struct bpf_program *prog, const char *name)
+{
+	struct bpf_link *link;
+	int val, err;
+
+	link = bpf_program__attach(prog);
+	if (!ASSERT_OK_PTR(link, name))
+		return;
+
+	err = write_ftrace_enabled(0);
+	ASSERT_EQ(err, -EBUSY, "disable_refused");
+	if (!ASSERT_OK(read_ftrace_enabled(&val), "read_back"))
+		goto detach;
+	ASSERT_EQ(val, 1, "still_enabled");
+
+detach:
+	bpf_link__destroy(link);
+
+	ASSERT_OK(write_ftrace_enabled(0), "disable_after_detach");
+	ASSERT_OK(write_ftrace_enabled(1), "reenable");
+}
+
+/* Attach @prog while ftrace_enabled=0 and assert it is refused. */
+static void check_attach_while_disabled_refused(struct bpf_program *prog, const char *name)
+{
+	struct bpf_link *link;
+
+	if (!ASSERT_OK(write_ftrace_enabled(0), "disable"))
+		return;
+
+	link = bpf_program__attach(prog);
+	if (!ASSERT_ERR_PTR(link, name))
+		bpf_link__destroy(link);
+
+	ASSERT_OK(write_ftrace_enabled(1), "reenable");
+}
+
+void test_ftrace_permanent(void)
+{
+	struct ftrace_permanent *skel;
+	bool kprobes_on_ftrace;
+	int orig = 1;
+
+	/* Save and always restore ftrace_enabled. */
+	if (read_ftrace_enabled(&orig)) {
+		test__skip();
+		return;
+	}
+
+	skel = ftrace_permanent__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))
+		goto restore;
+
+	kprobes_on_ftrace = skel->kconfig->CONFIG_KPROBES_ON_FTRACE;
+
+	if (test__start_subtest("fentry_blocks_disable"))
+		check_blocks_disable(skel->progs.test_fentry, "attach_fentry");
+
+	if (test__start_subtest("fexit_blocks_disable"))
+		check_blocks_disable(skel->progs.test_fexit, "attach_fexit");
+
+	if (test__start_subtest("kprobe_blocks_disable")) {
+		if (kprobes_on_ftrace)
+			check_blocks_disable(skel->progs.test_kprobe, "attach_kprobe");
+		else
+			test__skip();
+	}
+
+	if (test__start_subtest("kretprobe_blocks_disable")) {
+		if (kprobes_on_ftrace)
+			check_blocks_disable(skel->progs.test_kretprobe, "attach_kretprobe");
+		else
+			test__skip();
+	}
+
+	if (test__start_subtest("fentry_attach_while_disabled_refused"))
+		check_attach_while_disabled_refused(skel->progs.test_fentry, "attach_fentry");
+
+	if (test__start_subtest("kprobe_attach_while_disabled_refused")) {
+		if (kprobes_on_ftrace)
+			check_attach_while_disabled_refused(skel->progs.test_kprobe,
+							    "attach_kprobe");
+		else
+			test__skip();
+	}
+
+	ftrace_permanent__destroy(skel);
+restore:
+	write_ftrace_enabled(orig);
+}
diff --git a/tools/testing/selftests/bpf/progs/ftrace_permanent.c b/tools/testing/selftests/bpf/progs/ftrace_permanent.c
new file mode 100644
index 000000000000..ca706d8edc23
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/ftrace_permanent.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 CrowdStrike */
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <stdbool.h>
+
+char _license[] SEC("license") = "GPL";
+
+extern bool CONFIG_KPROBES_ON_FTRACE __kconfig __weak;
+
+/* This function is here to have CONFIG_KPROBES_ON_FTRACE used and
+ * added to object BTF, so the userspace side can read it back via
+ * skel->kconfig->CONFIG_KPROBES_ON_FTRACE.
+ */
+int unused(void)
+{
+	return CONFIG_KPROBES_ON_FTRACE ? 0 : 1;
+}
+
+SEC("fentry/bpf_fentry_test1")
+int BPF_PROG(test_fentry, int a)
+{
+	return 0;
+}
+
+SEC("fexit/bpf_fentry_test2")
+int BPF_PROG(test_fexit, int a, __u64 b)
+{
+	return 0;
+}
+
+SEC("kprobe/bpf_fentry_test3")
+int test_kprobe(struct pt_regs *ctx)
+{
+	return 0;
+}
+
+SEC("kretprobe/bpf_fentry_test4")
+int BPF_KRETPROBE(test_kretprobe)
+{
+	return 0;
+}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH bpf-next 3/3] selftests/bpf: add ftrace_permanent test
  2026-07-29  0:59 ` [RFC PATCH bpf-next 3/3] selftests/bpf: add ftrace_permanent test Andrey Grodzovsky
@ 2026-07-29  1:31   ` sashiko-bot
  2026-07-29 13:23   ` Jiri Olsa
  1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-29  1:31 UTC (permalink / raw)
  To: Andrey Grodzovsky; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The test dynamically alters a global system-wide setting (`kernel.ftrace_enabled`) but is not marked as a serial test, which will cause flaky test failures during default parallel execution.
- [Medium] The test does not explicitly ensure `kernel.ftrace_enabled = 1` after reading its original state, causing guaranteed test failures if the testing environment starts with ftrace disabled.
- [Low] A multi-line comment violates the BPF subsystem's mandatory comment style.
--

--- Patch [3]: [RFC PATCH bpf-next 3/3] selftests/bpf: add ftrace_permanent test ---
commit 16e4b9c3d2d63207f5f4817fc1dedd224a76749e
Author: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>

selftests/bpf: add ftrace_permanent test

This patch introduces a selftest to verify that kernel.ftrace_enabled=0 is 
refused with EBUSY when fentry, fexit, kprobe, or kretprobe BPF programs are 
attached. It confirms that the sysctl is allowed again once the programs are 
detached, and skips kprobes testing on architectures without 
CONFIG_KPROBES_ON_FTRACE.

> diff --git a/tools/testing/selftests/bpf/prog_tests/ftrace_permanent.c b/tools/testing/selftests/bpf/prog_tests/ftrace_permanent.c
> index 0000000000000..dbe78009a2913
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/ftrace_permanent.c

[ ... ]

> +void test_ftrace_permanent(void)
> +{

[Severity: Medium]
Since this test dynamically alters a global system-wide setting 
(kernel.ftrace_enabled), could this cause flaky failures for other concurrently 
running BPF selftests? 

Should this function be named serial_test_ftrace_permanent to ensure the test 
runner executes it exclusively?

> +	struct ftrace_permanent *skel;
> +	bool kprobes_on_ftrace;
> +	int orig = 1;
> +
> +	/* Save and always restore ftrace_enabled. */
> +	if (read_ftrace_enabled(&orig)) {
> +		test__skip();
> +		return;
> +	}
> +

[Severity: Medium]
If the testing environment happens to start with ftrace disabled (orig is 0), 
will check_blocks_disable() unconditionally fail when it tries to attach 
programs? 

Does the test need to explicitly set kernel.ftrace_enabled = 1 here before 
proceeding?

> +	skel = ftrace_permanent__open_and_load();

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/ftrace_permanent.c b/tools/testing/selftests/bpf/progs/ftrace_permanent.c
> index 0000000000000..ca706d8edc23b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/ftrace_permanent.c

[ ... ]

> +extern bool CONFIG_KPROBES_ON_FTRACE __kconfig __weak;
> +
> +/* This function is here to have CONFIG_KPROBES_ON_FTRACE used and
> + * added to object BTF, so the userspace side can read it back via
> + * skel->kconfig->CONFIG_KPROBES_ON_FTRACE.
> + */

[Severity: Low]
This isn't a bug, but this multi-line comment has text on the same line as the 
opening /*, which conflicts with the BPF subsystem's required comment style.

> +int unused(void)
> +{
> +	return CONFIG_KPROBES_ON_FTRACE ? 0 : 1;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729005959.3853865-1-andrey.grodzovsky@crowdstrike.com?part=3

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH bpf-next 2/3] kprobes: mark ftrace-based kprobe ops permanent
  2026-07-29  0:59 ` [RFC PATCH bpf-next 2/3] kprobes: mark ftrace-based kprobe ops permanent Andrey Grodzovsky
@ 2026-07-29  1:39   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-29  1:39 UTC (permalink / raw)
  To: Andrey Grodzovsky; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing error propagation in `__register_kprobe()` causes silent failures when `arm_kprobe()` fails, returning 0 (success) despite the kprobe failing to arm and being unhashed.
--

--- Patch [2]: [RFC PATCH bpf-next 2/3] kprobes: mark ftrace-based kprobe ops permanent ---
commit 97fc5419ead104f2c26b158adf549fba313680cd
Author: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
Subject: kprobes: mark ftrace-based kprobe ops permanent

This commit adds the FTRACE_OPS_FL_PERMANENT flag to ftrace-based kprobe
operations. It prevents kernel.ftrace_enabled=0 from silently killing
ftrace-based kprobes by allowing them to function independently of the
global ftrace state.

> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index bfc89083daa93..5a54511eee791 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1122,14 +1122,15 @@ static struct kprobe *alloc_aggr_kprobe(struct kprobe *p)
>  #endif /* CONFIG_OPTPROBES */
>  
>  #ifdef CONFIG_KPROBES_ON_FTRACE
> +/* Same protection livepatch gives its own ftrace_ops. */
>  static struct ftrace_ops kprobe_ftrace_ops __read_mostly = {
>  	.func = kprobe_ftrace_handler,
> -	.flags = FTRACE_OPS_FL_SAVE_REGS,
> +	.flags = FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_PERMANENT,
>  };

[Severity: High]
This is a pre-existing issue, but does adding this flag now cause a silent 
failure during kprobe registration if ftrace is disabled?

With FTRACE_OPS_FL_PERMANENT, register_ftrace_function() (and therefore
arm_kprobe()) will reliably fail with -EBUSY when ftrace_enabled=0.

However, in __register_kprobe(), the error code from arm_kprobe() is
discarded instead of being propagated to the caller:

kernel/kprobes.c:__register_kprobe() {
	if (!kprobes_all_disarmed && !kprobe_disabled(p)) {
		ret = arm_kprobe(p);
		if (ret) {
			hlist_del_rcu(&p->hlist);
			synchronize_rcu();
		}
	}

	/* Try to optimize kprobe */
	try_to_optimize_kprobe(p);
	return 0;
}

Could this cause the registration API to return 0 (success) even though the 
kprobe fails to arm and is unhashed? 

If the caller later calls unregister_kprobe(), it might fail internally with
-EINVAL because the kprobe is no longer in the hash table, skipping
__unregister_kprobe_bottom() and potentially leaking architecture-specific
resources.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729005959.3853865-1-andrey.grodzovsky@crowdstrike.com?part=2

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH bpf-next 3/3] selftests/bpf: add ftrace_permanent test
  2026-07-29  0:59 ` [RFC PATCH bpf-next 3/3] selftests/bpf: add ftrace_permanent test Andrey Grodzovsky
  2026-07-29  1:31   ` sashiko-bot
@ 2026-07-29 13:23   ` Jiri Olsa
  2026-07-29 17:16     ` Andrey Grodzovsky
  1 sibling, 1 reply; 13+ messages in thread
From: Jiri Olsa @ 2026-07-29 13:23 UTC (permalink / raw)
  To: Andrey Grodzovsky
  Cc: bpf, linux-trace-kernel, linux-kselftest, ast, daniel, andrii,
	song, rostedt, naveen, davem, mhiramat, stable, linux-open-source

On Tue, Jul 28, 2026 at 08:59:59PM -0400, Andrey Grodzovsky wrote:
> Cover the FTRACE_OPS_FL_PERMANENT fix on BPF trampolines and
> ftrace-based kprobes/kretprobes: attach a fentry, fexit, kprobe, and
> kretprobe program in turn and confirm kernel.ftrace_enabled=0 is
> refused with EBUSY while attached, then succeeds once detached. Also
> confirm a new fentry/kprobe attach is itself refused while
> ftrace_enabled=0 is already in effect.
> 
> kprobe.multi/kretprobe.multi/kprobe.session are intentionally not
> covered, matching the production change's scope.
> 
> Skip the kprobe/kretprobe subtests when CONFIG_KPROBES_ON_FTRACE is
> off (e.g. arm64), read back via skel->kconfig->CONFIG_KPROBES_ON_FTRACE
> (same __kconfig idiom as progs/test_fill_link_info.c).
> 
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
> ---

hi,
I have some links active already after boot which makes test to fail:

  [root@qemu-1 bpf]# echo 0 > /proc/sys/kernel/ftrace_enabled
  bash: echo: write error: Device or resource busy

  [root@qemu-1 bpf]# ./test_progs -t ftrace_permanent -v
  bpf_testmod.ko is already unloaded.
  Loading bpf_testmod.ko...
  Successfully loaded bpf_testmod.ko.
  test_ftrace_permanent:PASS:skel_open_and_load 0 nsec
  check_blocks_disable:PASS:attach_fentry 0 nsec
  check_blocks_disable:PASS:disable_refused 0 nsec
  check_blocks_disable:PASS:read_back 0 nsec
  check_blocks_disable:PASS:still_enabled 0 nsec
  check_blocks_disable:FAIL:disable_after_detach unexpected error: -16 (errno 16)
  check_blocks_disable:PASS:reenable 0 nsec
  #145/1   ftrace_permanent/fentry_blocks_disable:FAIL
  check_blocks_disable:PASS:attach_fexit 0 nsec
  check_blocks_disable:PASS:disable_refused 0 nsec
  check_blocks_disable:PASS:read_back 0 nsec
  check_blocks_disable:PASS:still_enabled 0 nsec
  check_blocks_disable:FAIL:disable_after_detach unexpected error: -16 (errno 16)
  check_blocks_disable:PASS:reenable 0 nsec
  #145/2   ftrace_permanent/fexit_blocks_disable:FAIL
  check_blocks_disable:PASS:attach_kprobe 0 nsec
  check_blocks_disable:PASS:disable_refused 0 nsec
  check_blocks_disable:PASS:read_back 0 nsec
  check_blocks_disable:PASS:still_enabled 0 nsec
  check_blocks_disable:FAIL:disable_after_detach unexpected error: -16 (errno 16)
  check_blocks_disable:PASS:reenable 0 nsec
  #145/3   ftrace_permanent/kprobe_blocks_disable:FAIL
  check_blocks_disable:PASS:attach_kretprobe 0 nsec
  check_blocks_disable:PASS:disable_refused 0 nsec
  check_blocks_disable:PASS:read_back 0 nsec
  check_blocks_disable:PASS:still_enabled 0 nsec
  check_blocks_disable:FAIL:disable_after_detach unexpected error: -16 (errno 16)
  check_blocks_disable:PASS:reenable 0 nsec
  #145/4   ftrace_permanent/kretprobe_blocks_disable:FAIL
  check_attach_while_disabled_refused:FAIL:disable unexpected error: -16 (errno 16)
  #145/5   ftrace_permanent/fentry_attach_while_disabled_refused:FAIL
  check_attach_while_disabled_refused:FAIL:disable unexpected error: -16 (errno 16)
  #145/6   ftrace_permanent/kprobe_attach_while_disabled_refused:FAIL
  #145     ftrace_permanent:FAIL
  Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED
  Successfully unloaded bpf_testmod.ko.

jirka


>  .../bpf/prog_tests/ftrace_permanent.c         | 144 ++++++++++++++++++
>  .../selftests/bpf/progs/ftrace_permanent.c    |  43 ++++++
>  2 files changed, 187 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/ftrace_permanent.c
>  create mode 100644 tools/testing/selftests/bpf/progs/ftrace_permanent.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/ftrace_permanent.c b/tools/testing/selftests/bpf/prog_tests/ftrace_permanent.c
> new file mode 100644
> index 000000000000..dbe78009a291
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/ftrace_permanent.c
> @@ -0,0 +1,144 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 CrowdStrike */
> +#include <test_progs.h>
> +#include "ftrace_permanent.skel.h"
> +
> +/*
> + * fentry/fexit/kprobe/kretprobe now carry FTRACE_OPS_FL_PERMANENT, so
> + * kernel.ftrace_enabled=0 must be refused with EBUSY while any of them
> + * is attached, and allowed again once detached.
> + *
> + * kprobe.multi/kretprobe.multi/kprobe.session are out of scope.
> + * kprobe/kretprobe subtests need CONFIG_KPROBES_ON_FTRACE (e.g. not on
> + * arm64) and are skipped otherwise.
> + */
> +
> +#define FTRACE_ENABLED_PATH "/proc/sys/kernel/ftrace_enabled"
> +
> +static int read_ftrace_enabled(int *val)
> +{
> +	char buf[16] = {};
> +	int fd, n;
> +
> +	fd = open(FTRACE_ENABLED_PATH, O_RDONLY);
> +	if (fd < 0)
> +		return -errno;
> +	n = read(fd, buf, sizeof(buf) - 1);
> +	close(fd);
> +	if (n <= 0)
> +		return -EIO;
> +	*val = atoi(buf);
> +	return 0;
> +}
> +
> +/* Returns 0 on success, or -errno on write failure. */
> +static int write_ftrace_enabled(int val)
> +{
> +	char buf[4];
> +	int fd, n, len, err = 0;
> +
> +	fd = open(FTRACE_ENABLED_PATH, O_WRONLY);
> +	if (fd < 0)
> +		return -errno;
> +	len = snprintf(buf, sizeof(buf), "%d", val);
> +	n = write(fd, buf, len);
> +	if (n < 0)
> +		err = -errno;
> +	close(fd);
> +	return err;
> +}
> +
> +/*
> + * Attach @prog, assert kernel.ftrace_enabled=0 is refused while attached
> + * and stays at 1, then detach and assert the disable now succeeds.
> + */
> +static void check_blocks_disable(struct bpf_program *prog, const char *name)
> +{
> +	struct bpf_link *link;
> +	int val, err;
> +
> +	link = bpf_program__attach(prog);
> +	if (!ASSERT_OK_PTR(link, name))
> +		return;
> +
> +	err = write_ftrace_enabled(0);
> +	ASSERT_EQ(err, -EBUSY, "disable_refused");
> +	if (!ASSERT_OK(read_ftrace_enabled(&val), "read_back"))
> +		goto detach;
> +	ASSERT_EQ(val, 1, "still_enabled");
> +
> +detach:
> +	bpf_link__destroy(link);
> +
> +	ASSERT_OK(write_ftrace_enabled(0), "disable_after_detach");
> +	ASSERT_OK(write_ftrace_enabled(1), "reenable");
> +}
> +
> +/* Attach @prog while ftrace_enabled=0 and assert it is refused. */
> +static void check_attach_while_disabled_refused(struct bpf_program *prog, const char *name)
> +{
> +	struct bpf_link *link;
> +
> +	if (!ASSERT_OK(write_ftrace_enabled(0), "disable"))
> +		return;
> +
> +	link = bpf_program__attach(prog);
> +	if (!ASSERT_ERR_PTR(link, name))
> +		bpf_link__destroy(link);
> +
> +	ASSERT_OK(write_ftrace_enabled(1), "reenable");
> +}
> +
> +void test_ftrace_permanent(void)
> +{
> +	struct ftrace_permanent *skel;
> +	bool kprobes_on_ftrace;
> +	int orig = 1;
> +
> +	/* Save and always restore ftrace_enabled. */
> +	if (read_ftrace_enabled(&orig)) {
> +		test__skip();
> +		return;
> +	}
> +
> +	skel = ftrace_permanent__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))
> +		goto restore;
> +
> +	kprobes_on_ftrace = skel->kconfig->CONFIG_KPROBES_ON_FTRACE;
> +
> +	if (test__start_subtest("fentry_blocks_disable"))
> +		check_blocks_disable(skel->progs.test_fentry, "attach_fentry");
> +
> +	if (test__start_subtest("fexit_blocks_disable"))
> +		check_blocks_disable(skel->progs.test_fexit, "attach_fexit");
> +
> +	if (test__start_subtest("kprobe_blocks_disable")) {
> +		if (kprobes_on_ftrace)
> +			check_blocks_disable(skel->progs.test_kprobe, "attach_kprobe");
> +		else
> +			test__skip();
> +	}
> +
> +	if (test__start_subtest("kretprobe_blocks_disable")) {
> +		if (kprobes_on_ftrace)
> +			check_blocks_disable(skel->progs.test_kretprobe, "attach_kretprobe");
> +		else
> +			test__skip();
> +	}
> +
> +	if (test__start_subtest("fentry_attach_while_disabled_refused"))
> +		check_attach_while_disabled_refused(skel->progs.test_fentry, "attach_fentry");
> +
> +	if (test__start_subtest("kprobe_attach_while_disabled_refused")) {
> +		if (kprobes_on_ftrace)
> +			check_attach_while_disabled_refused(skel->progs.test_kprobe,
> +							    "attach_kprobe");
> +		else
> +			test__skip();
> +	}
> +
> +	ftrace_permanent__destroy(skel);
> +restore:
> +	write_ftrace_enabled(orig);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/ftrace_permanent.c b/tools/testing/selftests/bpf/progs/ftrace_permanent.c
> new file mode 100644
> index 000000000000..ca706d8edc23
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/ftrace_permanent.c
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 CrowdStrike */
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include <stdbool.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +extern bool CONFIG_KPROBES_ON_FTRACE __kconfig __weak;
> +
> +/* This function is here to have CONFIG_KPROBES_ON_FTRACE used and
> + * added to object BTF, so the userspace side can read it back via
> + * skel->kconfig->CONFIG_KPROBES_ON_FTRACE.
> + */
> +int unused(void)
> +{
> +	return CONFIG_KPROBES_ON_FTRACE ? 0 : 1;
> +}
> +
> +SEC("fentry/bpf_fentry_test1")
> +int BPF_PROG(test_fentry, int a)
> +{
> +	return 0;
> +}
> +
> +SEC("fexit/bpf_fentry_test2")
> +int BPF_PROG(test_fexit, int a, __u64 b)
> +{
> +	return 0;
> +}
> +
> +SEC("kprobe/bpf_fentry_test3")
> +int test_kprobe(struct pt_regs *ctx)
> +{
> +	return 0;
> +}
> +
> +SEC("kretprobe/bpf_fentry_test4")
> +int BPF_KRETPROBE(test_kretprobe)
> +{
> +	return 0;
> +}
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH bpf-next 1/3] bpf: mark trampoline ftrace_ops permanent
  2026-07-29  0:59 ` [RFC PATCH bpf-next 1/3] bpf: mark trampoline " Andrey Grodzovsky
@ 2026-07-29 13:29   ` Jiri Olsa
  0 siblings, 0 replies; 13+ messages in thread
From: Jiri Olsa @ 2026-07-29 13:29 UTC (permalink / raw)
  To: Andrey Grodzovsky
  Cc: bpf, linux-trace-kernel, linux-kselftest, ast, daniel, andrii,
	song, rostedt, naveen, davem, mhiramat, stable, linux-open-source

On Tue, Jul 28, 2026 at 08:59:57PM -0400, Andrey Grodzovsky wrote:
> kernel.ftrace_enabled=0 silently kills BPF trampolines (fentry/fexit):
> attach still succeeds, the hook stops firing with no error, and
> re-enabling silently restores it with no indication anything happened.
> 
> This is a regression, not a new gap: BPF trampolines were already
> protected against this the same way livepatch protects itself, via
> FTRACE_OPS_FL_PERMANENT on the shared trampoline ftrace_ops. That
> protection was silently dropped during a later refactoring and never
> restored, so this has been broken for years.
> 
> Both the shared direct_ops (CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS
> arches) and the per-trampoline ops allocated on other arches (arm64,
> s390) are BPF-trampoline-private, not shared with any unrelated
> subsystem, so restoring the same unconditional flag is safe and needs
> no kernel/trace/ftrace.c changes -- its existing generic sysctl gate
> and attach-time refusal already scan the ops list and pick this up
> for free.
> 
> Fixes: 00963a2e75a8 ("bpf: Support bpf_trampoline on functions with IPMODIFY (e.g. livepatch)")

hum, we did use it before this commit, I completely missed that, thanks

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

jirka


> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>

> ---
>  kernel/bpf/trampoline.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 129d07db117e..3d5069091db7 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -224,6 +224,8 @@ void bpf_image_ksym_del(struct bpf_ksym *ksym)
>   */
>  struct ftrace_ops direct_ops = {
>  	.ops_func = bpf_tramp_ftrace_ops_func,
> +	/* Same protection livepatch gives its own ftrace_ops. */
> +	.flags = FTRACE_OPS_FL_PERMANENT,
>  };
>  
>  static int direct_ops_alloc(struct bpf_trampoline *tr)
> @@ -303,6 +305,8 @@ static int direct_ops_alloc(struct bpf_trampoline *tr)
>  		return -ENOMEM;
>  	tr->fops->private = tr;
>  	tr->fops->ops_func = bpf_tramp_ftrace_ops_func;
> +	/* See the direct_ops initializer above for why. */
> +	tr->fops->flags |= FTRACE_OPS_FL_PERMANENT;
>  	return 0;
>  }
>  
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent
  2026-07-29  0:59 [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent Andrey Grodzovsky
                   ` (2 preceding siblings ...)
  2026-07-29  0:59 ` [RFC PATCH bpf-next 3/3] selftests/bpf: add ftrace_permanent test Andrey Grodzovsky
@ 2026-07-29 14:41 ` Steven Rostedt
  2026-07-29 15:00   ` Andrey Grodzovsky
  3 siblings, 1 reply; 13+ messages in thread
From: Steven Rostedt @ 2026-07-29 14:41 UTC (permalink / raw)
  To: Andrey Grodzovsky
  Cc: bpf, linux-trace-kernel, linux-kselftest, ast, daniel, andrii,
	song, jolsa, naveen, davem, mhiramat, stable, linux-open-source

On Tue, 28 Jul 2026 20:59:56 -0400
Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com> wrote:

> This fixes a long-standing issue: kernel.ftrace_enabled=0 silently
> disables BPF trampolines (fentry/fexit) and ftrace-based
> kprobes/kretprobes. The write succeeds, the hook stops firing with no
> error, and re-enabling silently restores it. Livepatch already solved
> this for itself via FTRACE_OPS_FL_PERMANENT, which refuses to disable
> ftrace while a permanent ops is registered and refuses to register
> one while ftrace is disabled[1].

The /proc/sys/kernel/ftrace_enabled was added as a "safety kill switch"
back when ftrace was first added to the kernel. It's addition was namely
there because live runtime modification of kernel code was new and we were
worried about how stable it could be.

Honestly, I would love to get rid of it as today ftrace has proven to be
rather stable. But as it is a user space ABI, I'm not sure what will break
if we do. I wonder if we just make it a nop, and print a message to dmesg
saying:

  "ftrace_enabled no longer does anything. Please report if you need it to."

?

> 
> For trampolines this restores a historical property: from 2019-2022
> they shared one global direct_ops, marked permanent the same
> way[2].It was later lost as a side effect of the 2022
> per-trampoline-ops split (patch 1's Fixes tag) and never
> restored.[3][4] Kprobes never carried this protection at all, so
> for them that is long-standing issue rather than a regression.
> 
> Patch 1: trampolines. Patch 2: classic kprobes/kretprobes. Patch 3:
> a selftest covering both directions for all four hook types.
> 
> P.S
> I initially implemented a per-record opt-in flag[5], but dropped it
> as over-engineering once I saw the original blanket restriction.
> 
> P.P.S
> Open question: kprobe.multi/kretprobe.multi/kprobe.session
> (fprobe-backed) aren't covered -- return-capturing fprobes share the
> function-graph tracer's subops manager with unrelated tracers, so
> marking it permanent needs a different, per-record approach. Perhaps
> something along the lines of [5].

Honestly, I would like to start deprecating that kill switch.

For the series:

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent
  2026-07-29 14:41 ` [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent Steven Rostedt
@ 2026-07-29 15:00   ` Andrey Grodzovsky
  2026-07-29 15:31     ` Steven Rostedt
  0 siblings, 1 reply; 13+ messages in thread
From: Andrey Grodzovsky @ 2026-07-29 15:00 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: bpf, linux-trace-kernel, linux-kselftest, ast, daniel, andrii,
	song, jolsa, naveen, davem, mhiramat, stable, linux-open-source,
	mbenes

,

On Wed, Jul 29, 2026 at 10:40 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 28 Jul 2026 20:59:56 -0400
> Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com> wrote:
>
> > This fixes a long-standing issue: kernel.ftrace_enabled=0 silently
> > disables BPF trampolines (fentry/fexit) and ftrace-based
> > kprobes/kretprobes. The write succeeds, the hook stops firing with no
> > error, and re-enabling silently restores it. Livepatch already solved
> > this for itself via FTRACE_OPS_FL_PERMANENT, which refuses to disable
> > ftrace while a permanent ops is registered and refuses to register
> > one while ftrace is disabled[1].
>
> The /proc/sys/kernel/ftrace_enabled was added as a "safety kill switch"
> back when ftrace was first added to the kernel. It's addition was namely
> there because live runtime modification of kernel code was new and we were
> worried about how stable it could be.
>
> Honestly, I would love to get rid of it as today ftrace has proven to be
> rather stable. But as it is a user space ABI, I'm not sure what will break
> if we do. I wonder if we just make it a nop, and print a message to dmesg
> saying:
>
>   "ftrace_enabled no longer does anything. Please report if you need it to."
>
> ?

Thanks Steven, if you and/or other community members would approve such
approach I would happily do this instead for a few reasons -

1) Seems like the more correct course of action if idneed this switch
effectively became obsolete in usage instead of piling extra logic to work
around it.

2) As I mentioned in the end of the cover letter, I believe for some cases
such as fsessions/ksessions and multi opts retprobes, this patchset is
not effective and the issue will persist, requiring a more complicated solution
for them anyway if we want watertight resilience.

If you approve this - I can try what you suggested, this would include
making the
knob a NOP with a message, dropping the FTRACE_OPS_FL_PERMANENT
flag and reverting/cleaning what's possible from the original livepatch patchset
that dealt with this issue [1]

One question is what the correct set of tests to run in such a case
would be to verify
we didn't break anything. For this patchset I ran BPF and livepatch
selftest suites, but
I wonder what else would need to be run.

CC Miroslav for Livepatch.

Thanks,
Andrey

[1] - https://lore.kernel.org/all/20191016113316.13415-1-mbenes@suse.cz/T/#u

>
> >
> > For trampolines this restores a historical property: from 2019-2022
> > they shared one global direct_ops, marked permanent the same
> > way[2].It was later lost as a side effect of the 2022
> > per-trampoline-ops split (patch 1's Fixes tag) and never
> > restored.[3][4] Kprobes never carried this protection at all, so
> > for them that is long-standing issue rather than a regression.
> >
> > Patch 1: trampolines. Patch 2: classic kprobes/kretprobes. Patch 3:
> > a selftest covering both directions for all four hook types.
> >
> > P.S
> > I initially implemented a per-record opt-in flag[5], but dropped it
> > as over-engineering once I saw the original blanket restriction.
> >
> > P.P.S
> > Open question: kprobe.multi/kretprobe.multi/kprobe.session
> > (fprobe-backed) aren't covered -- return-capturing fprobes share the
> > function-graph tracer's subops manager with unrelated tracers, so
> > marking it permanent needs a different, per-record approach. Perhaps
> > something along the lines of [5].
>
> Honestly, I would like to start deprecating that kill switch.
>
> For the series:
>
> Acked-by: Steven Rostedt <rostedt@goodmis.org>
>
> -- Steve

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent
  2026-07-29 15:00   ` Andrey Grodzovsky
@ 2026-07-29 15:31     ` Steven Rostedt
  2026-07-29 15:47       ` Linus Torvalds
  0 siblings, 1 reply; 13+ messages in thread
From: Steven Rostedt @ 2026-07-29 15:31 UTC (permalink / raw)
  To: Andrey Grodzovsky
  Cc: bpf, linux-trace-kernel, linux-kselftest, ast, daniel, andrii,
	song, jolsa, naveen, davem, mhiramat, stable, linux-open-source,
	mbenes, Linus Torvalds


[ Adding Linus in case he cares ]

On Wed, 29 Jul 2026 11:00:31 -0400
Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com> wrote:

> ,
> 
> On Wed, Jul 29, 2026 at 10:40 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Tue, 28 Jul 2026 20:59:56 -0400
> > Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com> wrote:
> >  
> > > This fixes a long-standing issue: kernel.ftrace_enabled=0 silently
> > > disables BPF trampolines (fentry/fexit) and ftrace-based
> > > kprobes/kretprobes. The write succeeds, the hook stops firing with no
> > > error, and re-enabling silently restores it. Livepatch already solved
> > > this for itself via FTRACE_OPS_FL_PERMANENT, which refuses to disable
> > > ftrace while a permanent ops is registered and refuses to register
> > > one while ftrace is disabled[1].  
> >
> > The /proc/sys/kernel/ftrace_enabled was added as a "safety kill switch"
> > back when ftrace was first added to the kernel. It's addition was namely
> > there because live runtime modification of kernel code was new and we were
> > worried about how stable it could be.
> >
> > Honestly, I would love to get rid of it as today ftrace has proven to be
> > rather stable. But as it is a user space ABI, I'm not sure what will break
> > if we do. I wonder if we just make it a nop, and print a message to dmesg
> > saying:
> >
> >   "ftrace_enabled no longer does anything. Please report if you need it to."
> >
> > ?  
> 
> Thanks Steven, if you and/or other community members would approve such
> approach I would happily do this instead for a few reasons -

It's actually if Linus is OK with this approach.

> 
> 1) Seems like the more correct course of action if idneed this switch
> effectively became obsolete in usage instead of piling extra logic to work
> around it.
> 
> 2) As I mentioned in the end of the cover letter, I believe for some cases
> such as fsessions/ksessions and multi opts retprobes, this patchset is
> not effective and the issue will persist, requiring a more complicated solution
> for them anyway if we want watertight resilience.
> 
> If you approve this - I can try what you suggested, this would include
> making the
> knob a NOP with a message, dropping the FTRACE_OPS_FL_PERMANENT
> flag and reverting/cleaning what's possible from the original livepatch patchset
> that dealt with this issue [1]
> 
> One question is what the correct set of tests to run in such a case
> would be to verify
> we didn't break anything. For this patchset I ran BPF and livepatch

Hah! That's the key question isn't it? It's a user space visible knob. I
don't have anything that uses it, but I have no idea of there's a tool out
there that does. Linus's rule is to not break user space. If nothing uses
it, then it's OK to get rid of (as getting rid of it will not break user
space). But, if something uses it for a good reason, then we are stuck with
it. We really will not know until we disable it and somebody complains.

-- Steve



> selftest suites, but
> I wonder what else would need to be run.
> 
> CC Miroslav for Livepatch.
> 
> Thanks,
> Andrey
> 
> [1] - https://lore.kernel.org/all/20191016113316.13415-1-mbenes@suse.cz/T/#u
> 
> >  
> > >
> > > For trampolines this restores a historical property: from 2019-2022
> > > they shared one global direct_ops, marked permanent the same
> > > way[2].It was later lost as a side effect of the 2022
> > > per-trampoline-ops split (patch 1's Fixes tag) and never
> > > restored.[3][4] Kprobes never carried this protection at all, so
> > > for them that is long-standing issue rather than a regression.
> > >
> > > Patch 1: trampolines. Patch 2: classic kprobes/kretprobes. Patch 3:
> > > a selftest covering both directions for all four hook types.
> > >
> > > P.S
> > > I initially implemented a per-record opt-in flag[5], but dropped it
> > > as over-engineering once I saw the original blanket restriction.
> > >
> > > P.P.S
> > > Open question: kprobe.multi/kretprobe.multi/kprobe.session
> > > (fprobe-backed) aren't covered -- return-capturing fprobes share the
> > > function-graph tracer's subops manager with unrelated tracers, so
> > > marking it permanent needs a different, per-record approach. Perhaps
> > > something along the lines of [5].  
> >
> > Honestly, I would like to start deprecating that kill switch.
> >
> > For the series:
> >
> > Acked-by: Steven Rostedt <rostedt@goodmis.org>
> >
> > -- Steve  


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent
  2026-07-29 15:31     ` Steven Rostedt
@ 2026-07-29 15:47       ` Linus Torvalds
  0 siblings, 0 replies; 13+ messages in thread
From: Linus Torvalds @ 2026-07-29 15:47 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Andrey Grodzovsky, bpf, linux-trace-kernel, linux-kselftest, ast,
	daniel, andrii, song, jolsa, naveen, davem, mhiramat, stable,
	linux-open-source, mbenes

On Wed, 29 Jul 2026 at 08:31, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > > Honestly, I would love to get rid of it as today ftrace has proven to be
> > > rather stable. But as it is a user space ABI, I'm not sure what will break
> > > if we do. I wonder if we just make it a nop, and print a message to dmesg
> > > saying:
> > >
> > >   "ftrace_enabled no longer does anything. Please report if you need it to."
> > >
> > > ?
> >
> > Thanks Steven, if you and/or other community members would approve such
> > approach I would happily do this instead for a few reasons -
>
> It's actually if Linus is OK with this approach.

The rule is always the same, and I don't understand why there's any
discussion about it because it's been stated a million times: we can
try to make any ABI changes at all, and what matters is whether it
breaks somebodys setup.

If people rely on old behavior, it can't change. And if people don't,
nobody cares. It really is that simple. That is literally the
DEFINITION of "don't break user space".

Whether something is user-*VISIBLE* in theory is entirely immaterial
except in the sense of "be careful, this needs testing and may have to
be reverted". So user-visibility isn't a "don't do it", it's a "do it
carefully, and knowing that you may have to revert immediately if
somebody complains".

So changing user-visible things is potentially a lot of wasted effort,
and needs loving care and follow-up.

So everybody should typically avoid changing user-visible things
because it's potentially painful and developer-intensive, but not
because it can't be done.

All that matters is whether real user workloads regress.

And please make sure that any changed behaviour is really obvious, so
that people don't spend lots of time chasing down *why* something
broke. A kernel printk is generally way too subtle - and no, the
tracing "make the printk extra ugly with crazy ASCII barfics" model is
not an improvement.

It might be better to actually return an error to user space so that
users can see that setting ftrace_enabled to zero failed and it would
show up in user logs. kernel developers may think that kernel logs are
important, but users seldom even think about them. Of course, that can
then cause even worse regressions, so there's a balance here.

End result: don't do user visible changes unless you really really
care. And then you spend the extra effort.

               Linus

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH bpf-next 3/3] selftests/bpf: add ftrace_permanent test
  2026-07-29 13:23   ` Jiri Olsa
@ 2026-07-29 17:16     ` Andrey Grodzovsky
  0 siblings, 0 replies; 13+ messages in thread
From: Andrey Grodzovsky @ 2026-07-29 17:16 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: bpf, linux-trace-kernel, linux-kselftest, ast, daniel, andrii,
	song, rostedt, naveen, davem, mhiramat, stable, linux-open-source

On Wed, Jul 29, 2026 at 9:23 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Tue, Jul 28, 2026 at 08:59:59PM -0400, Andrey Grodzovsky wrote:
> > Cover the FTRACE_OPS_FL_PERMANENT fix on BPF trampolines and
> > ftrace-based kprobes/kretprobes: attach a fentry, fexit, kprobe, and
> > kretprobe program in turn and confirm kernel.ftrace_enabled=0 is
> > refused with EBUSY while attached, then succeeds once detached. Also
> > confirm a new fentry/kprobe attach is itself refused while
> > ftrace_enabled=0 is already in effect.
> >
> > kprobe.multi/kretprobe.multi/kprobe.session are intentionally not
> > covered, matching the production change's scope.
> >
> > Skip the kprobe/kretprobe subtests when CONFIG_KPROBES_ON_FTRACE is
> > off (e.g. arm64), read back via skel->kconfig->CONFIG_KPROBES_ON_FTRACE
> > (same __kconfig idiom as progs/test_fill_link_info.c).
> >
> > Assisted-by: Claude:claude-sonnet-5
> > Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
> > ---
>
> hi,
> I have some links active already after boot which makes test to fail:
>
>   [root@qemu-1 bpf]# echo 0 > /proc/sys/kernel/ftrace_enabled
>   bash: echo: write error: Device or resource busy
>
>   [root@qemu-1 bpf]# ./test_progs -t ftrace_permanent -v
>   bpf_testmod.ko is already unloaded.
>   Loading bpf_testmod.ko...
>   Successfully loaded bpf_testmod.ko.
>   test_ftrace_permanent:PASS:skel_open_and_load 0 nsec
>   check_blocks_disable:PASS:attach_fentry 0 nsec
>   check_blocks_disable:PASS:disable_refused 0 nsec
>   check_blocks_disable:PASS:read_back 0 nsec
>   check_blocks_disable:PASS:still_enabled 0 nsec
>   check_blocks_disable:FAIL:disable_after_detach unexpected error: -16 (errno 16)
>   check_blocks_disable:PASS:reenable 0 nsec
>   #145/1   ftrace_permanent/fentry_blocks_disable:FAIL
>   check_blocks_disable:PASS:attach_fexit 0 nsec
>   check_blocks_disable:PASS:disable_refused 0 nsec
>   check_blocks_disable:PASS:read_back 0 nsec
>   check_blocks_disable:PASS:still_enabled 0 nsec
>   check_blocks_disable:FAIL:disable_after_detach unexpected error: -16 (errno 16)
>   check_blocks_disable:PASS:reenable 0 nsec
>   #145/2   ftrace_permanent/fexit_blocks_disable:FAIL
>   check_blocks_disable:PASS:attach_kprobe 0 nsec
>   check_blocks_disable:PASS:disable_refused 0 nsec
>   check_blocks_disable:PASS:read_back 0 nsec
>   check_blocks_disable:PASS:still_enabled 0 nsec
>   check_blocks_disable:FAIL:disable_after_detach unexpected error: -16 (errno 16)
>   check_blocks_disable:PASS:reenable 0 nsec
>   #145/3   ftrace_permanent/kprobe_blocks_disable:FAIL
>   check_blocks_disable:PASS:attach_kretprobe 0 nsec
>   check_blocks_disable:PASS:disable_refused 0 nsec
>   check_blocks_disable:PASS:read_back 0 nsec
>   check_blocks_disable:PASS:still_enabled 0 nsec
>   check_blocks_disable:FAIL:disable_after_detach unexpected error: -16 (errno 16)
>   check_blocks_disable:PASS:reenable 0 nsec
>   #145/4   ftrace_permanent/kretprobe_blocks_disable:FAIL
>   check_attach_while_disabled_refused:FAIL:disable unexpected error: -16 (errno 16)
>   #145/5   ftrace_permanent/fentry_attach_while_disabled_refused:FAIL
>   check_attach_while_disabled_refused:FAIL:disable unexpected error: -16 (errno 16)
>   #145/6   ftrace_permanent/kprobe_attach_while_disabled_refused:FAIL
>   #145     ftrace_permanent:FAIL
>   Summary: 0/0 PASSED, 0 SKIPPED, 1 FAILED
>   Successfully unloaded bpf_testmod.ko.
>
> jirka
>
>

Thank you, Jiri. I will address this once the decision on whether to
proceed with this
route or, just make ftrace_enabled obsolete, is made.

Andrey

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-07-29 17:16 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29  0:59 [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent Andrey Grodzovsky
2026-07-29  0:59 ` [RFC PATCH bpf-next 1/3] bpf: mark trampoline " Andrey Grodzovsky
2026-07-29 13:29   ` Jiri Olsa
2026-07-29  0:59 ` [RFC PATCH bpf-next 2/3] kprobes: mark ftrace-based kprobe ops permanent Andrey Grodzovsky
2026-07-29  1:39   ` sashiko-bot
2026-07-29  0:59 ` [RFC PATCH bpf-next 3/3] selftests/bpf: add ftrace_permanent test Andrey Grodzovsky
2026-07-29  1:31   ` sashiko-bot
2026-07-29 13:23   ` Jiri Olsa
2026-07-29 17:16     ` Andrey Grodzovsky
2026-07-29 14:41 ` [RFC PATCH bpf-next 0/3] ftrace, kprobes, bpf: mark trampoline/kprobe ftrace_ops permanent Steven Rostedt
2026-07-29 15:00   ` Andrey Grodzovsky
2026-07-29 15:31     ` Steven Rostedt
2026-07-29 15:47       ` Linus Torvalds

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.