[PATCH] kprobes: use DEFINE_DEBUGFS_ATTRIBUTE for the enabled knob

[PATCH] kprobes: use DEFINE_DEBUGFS_ATTRIBUTE for the enabled knob

[Bradley Morgan]

This enabled knob is a disgusting terrible hack. It has rolled its
own read/write pair since 2007, writing '1' or '0' into a three byte
buffer by hand just to print a single character, with an XXX comment
begging debugfs for write callbacks on bool files.
DEFINE_DEBUGFS_ATTRIBUTE showed up in 2016 and does exactly that, so
the disgusting terrible hack has outlived its excuse for nine years.
Kill it, and the stale comment with it.

The behavior does not change, except the write only accepts 0/1 now
instead of y/n/on/off, and nothing uses anything else.
---
 kernel/kprobes.c | 43 +++++++++----------------------------------
 1 file changed, 9 insertions(+), 34 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index bfc89083daa9..044c6b5fd2aa 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -3013,47 +3013,22 @@ static int disarm_all_kprobes(void)
 	return ret;
 }
 
-/*
- * XXX: The debugfs bool file interface doesn't allow for callbacks
- * when the bool state is switched. We can reuse that facility when
- * available
- */
-static ssize_t read_enabled_file_bool(struct file *file,
-	       char __user *user_buf, size_t count, loff_t *ppos)
+static int kprobes_enabled_set(void *data, u64 val)
 {
-	char buf[3];
+	if (val)
+		return arm_all_kprobes();
 
-	if (!kprobes_all_disarmed)
-		buf[0] = '1';
-	else
-		buf[0] = '0';
-	buf[1] = '\n';
-	buf[2] = 0x00;
-	return simple_read_from_buffer(user_buf, count, ppos, buf, 2);
+	return disarm_all_kprobes();
 }
 
-static ssize_t write_enabled_file_bool(struct file *file,
-	       const char __user *user_buf, size_t count, loff_t *ppos)
+static int kprobes_enabled_get(void *data, u64 *val)
 {
-	bool enable;
-	int ret;
-
-	ret = kstrtobool_from_user(user_buf, count, &enable);
-	if (ret)
-		return ret;
-
-	ret = enable ? arm_all_kprobes() : disarm_all_kprobes();
-	if (ret)
-		return ret;
-
-	return count;
+	*val = !kprobes_all_disarmed;
+	return 0;
 }
 
-static const struct file_operations fops_kp = {
-	.read =         read_enabled_file_bool,
-	.write =        write_enabled_file_bool,
-	.llseek =	default_llseek,
-};
+DEFINE_DEBUGFS_ATTRIBUTE(fops_kp, kprobes_enabled_get,
+			 kprobes_enabled_set, "%llu\n");
 
 static int __init debugfs_kprobe_init(void)
 {
-- 
2.47.3

Re: [PATCH] kprobes: use DEFINE_DEBUGFS_ATTRIBUTE for the enabled knob

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The 'enabled' debugfs file is created using `debugfs_create_file()` instead of `debugfs_create_file_unsafe()` with a `DEFINE_DEBUGFS_ATTRIBUTE` struct, leading to redundant proxying and overhead.
--

--- Patch [1]: [PATCH] kprobes: use DEFINE_DEBUGFS_ATTRIBUTE for the enabled knob ---
commit f5babbd146ccf4194f28506d04f5cbfb2c40150a
Author: Bradley Morgan <include@grrlz.net>

    kprobes: use DEFINE_DEBUGFS_ATTRIBUTE for the enabled knob

This commit replaces the custom read/write handlers for the debugfs enabled
file with the standard DEFINE_DEBUGFS_ATTRIBUTE macro. It simplifies the
implementation and accepts integer inputs rather than characters.

> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index ca9d834d0b843..10bcc2a0926a6 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -2962,47 +2962,22 @@ static int disarm_all_kprobes(void)

[ ... ]

> -static const struct file_operations fops_kp = {
> -	.read =         read_enabled_file_bool,
> -	.write =        write_enabled_file_bool,
> -	.llseek =	default_llseek,
> -};
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_kp, kprobes_enabled_get,
> +			 kprobes_enabled_set, "%llu\n");
>  
>  static int __init debugfs_kprobe_init(void)
>  {

[Severity: Low]
Will this lead to double-proxying for the debugfs file?

Since DEFINE_DEBUGFS_ATTRIBUTE natively provides debugfs_attr_read and
debugfs_attr_write operations that already include debugfs_file_get() and
debugfs_file_put() protection, passing fops_kp to debugfs_create_file()
in debugfs_kprobe_init() will cause the debugfs core to wrap it again:

kernel/kprobes.c:debugfs_kprobe_init() {
    ...
	debugfs_create_file("enabled", 0600, dir, NULL, &fops_kp);
    ...
}

Could this use debugfs_create_file_unsafe() instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818014920.23870-1-include@grrlz.net?part=1

Re: [PATCH] kprobes: use DEFINE_DEBUGFS_ATTRIBUTE for the enabled knob

[Bradley Morgan]

On 18 August 2026 03:01:18 BST, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 1 potential
>issue(s) to consider:
>- [Low] The 'enabled' debugfs file is created using `debugfs_create_file()` instead of `debugfs_create_file_unsafe()` with a `DEFINE_DEBUGFS_ATTRIBUTE` struct, leading to redundant proxying and overhead.

+akpm@linux-foundation.org 

I hate sashiko so damn much, best to wait for review!

>--
>
>--- Patch [1]: [PATCH] kprobes: use DEFINE_DEBUGFS_ATTRIBUTE for the enabled knob ---
>commit f5babbd146ccf4194f28506d04f5cbfb2c40150a
>Author: Bradley Morgan <include@grrlz.net>
>
>    kprobes: use DEFINE_DEBUGFS_ATTRIBUTE for the enabled knob
>
>This commit replaces the custom read/write handlers for the debugfs
>enabled
>file with the standard DEFINE_DEBUGFS_ATTRIBUTE macro. It simplifies the
>implementation and accepts integer inputs rather than characters.
>
>> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
>> index ca9d834d0b843..10bcc2a0926a6 100644
>> --- a/kernel/kprobes.c
>> +++ b/kernel/kprobes.c
>> @@ -2962,47 +2962,22 @@ static int disarm_all_kprobes(void)
>
>[ ... ]
>
>> -static const struct file_operations fops_kp = {
>> -	.read =         read_enabled_file_bool,
>> -	.write =        write_enabled_file_bool,
>> -	.llseek =	default_llseek,
>> -};
>> +DEFINE_DEBUGFS_ATTRIBUTE(fops_kp, kprobes_enabled_get,
>> +			 kprobes_enabled_set, "%llu\n");
>>  
>>  static int __init debugfs_kprobe_init(void)
>>  {
>
>[Severity: Low]
>Will this lead to double-proxying for the debugfs file?
>
>Since DEFINE_DEBUGFS_ATTRIBUTE natively provides debugfs_attr_read and
>debugfs_attr_write operations that already include debugfs_file_get() and
>debugfs_file_put() protection, passing fops_kp to debugfs_create_file()
>in debugfs_kprobe_init() will cause the debugfs core to wrap it again:
>
>kernel/kprobes.c:debugfs_kprobe_init() {
>    ...
>	debugfs_create_file("enabled", 0600, dir, NULL, &fops_kp);
>    ...
>}
>
>Could this use debugfs_create_file_unsafe() instead?
>
>

Thanks!

Re: [PATCH] kprobes: use DEFINE_DEBUGFS_ATTRIBUTE for the enabled knob

[Steven Rostedt]

On Tue, 18 Aug 2026 16:17:20 +0100
Bradley Morgan <include@grrlz.net> wrote:

> On 18 August 2026 03:01:18 BST, sashiko-bot@kernel.org wrote:
> >Thank you for your contribution! Sashiko AI review found 1 potential
> >issue(s) to consider:
> >- [Low] The 'enabled' debugfs file is created using `debugfs_create_file()` instead of `debugfs_create_file_unsafe()` with a `DEFINE_DEBUGFS_ATTRIBUTE` struct, leading to redundant proxying and overhead.  
> 
> +akpm@linux-foundation.org 
> 
> I hate sashiko so damn much, best to wait for review!

I found sashiko to be extremely useful. 


> >[ ... ]
> >  
> >> -static const struct file_operations fops_kp = {
> >> -	.read =         read_enabled_file_bool,
> >> -	.write =        write_enabled_file_bool,
> >> -	.llseek =	default_llseek,
> >> -};
> >> +DEFINE_DEBUGFS_ATTRIBUTE(fops_kp, kprobes_enabled_get,
> >> +			 kprobes_enabled_set, "%llu\n");
> >>  
> >>  static int __init debugfs_kprobe_init(void)
> >>  {  
> >
> >[Severity: Low]

It is even saying this is of "low priority". That means it's more of an "FYI".

> >Will this lead to double-proxying for the debugfs file?
> >
> >Since DEFINE_DEBUGFS_ATTRIBUTE natively provides debugfs_attr_read and
> >debugfs_attr_write operations that already include debugfs_file_get() and
> >debugfs_file_put() protection, passing fops_kp to debugfs_create_file()
> >in debugfs_kprobe_init() will cause the debugfs core to wrap it again:
> >
> >kernel/kprobes.c:debugfs_kprobe_init() {
> >    ...
> >	debugfs_create_file("enabled", 0600, dir, NULL, &fops_kp);
> >    ...
> >}
> >
> >Could this use debugfs_create_file_unsafe() instead?

As this isn't a critical path, I don't think we really care here if
it's wrapped or not. But it's nice to know that it is.

Anyway, I'll let others review this, but it looks fine to me.

-- Steve

Re: [PATCH] kprobes: use DEFINE_DEBUGFS_ATTRIBUTE for the enabled knob

[Bradley Morgan]

On 18 August 2026 17:10:25 BST, Steven Rostedt <rostedt@goodmis.org> wrote:
>On Tue, 18 Aug 2026 16:17:20 +0100
>Bradley Morgan <include@grrlz.net> wrote:
>
>> On 18 August 2026 03:01:18 BST, sashiko-bot@kernel.org wrote:
>> >Thank you for your contribution! Sashiko AI review found 1 potential
>> >issue(s) to consider:
>> >- [Low] The 'enabled' debugfs file is created using
>`debugfs_create_file()` instead of `debugfs_create_file_unsafe()` with a
>`DEFINE_DEBUGFS_ATTRIBUTE` struct, leading to redundant proxying and
>overhead.  
>> 
>> +akpm@linux-foundation.org 
>> 
>> I hate sashiko so damn much, best to wait for review!
>
>I found sashiko to be extremely useful. 
>
>
>> >[ ... ]
>> >  
>> >> -static const struct file_operations fops_kp = {
>> >> -	.read =         read_enabled_file_bool,
>> >> -	.write =        write_enabled_file_bool,
>> >> -	.llseek =	default_llseek,
>> >> -};
>> >> +DEFINE_DEBUGFS_ATTRIBUTE(fops_kp, kprobes_enabled_get,
>> >> +			 kprobes_enabled_set, "%llu\n");
>> >>  
>> >>  static int __init debugfs_kprobe_init(void)
>> >>  {  
>> >
>> >[Severity: Low]
>
>It is even saying this is of "low priority". That means it's more of an
>"FYI".
>
>> >Will this lead to double-proxying for the debugfs file?
>> >
>> >Since DEFINE_DEBUGFS_ATTRIBUTE natively provides debugfs_attr_read and
>> >debugfs_attr_write operations that already include debugfs_file_get()
>and
>> >debugfs_file_put() protection, passing fops_kp to debugfs_create_file()
>> >in debugfs_kprobe_init() will cause the debugfs core to wrap it again:
>> >
>> >kernel/kprobes.c:debugfs_kprobe_init() {
>> >    ...
>> >	debugfs_create_file("enabled", 0600, dir, NULL, &fops_kp);
>> >    ...
>> >}
>> >
>> >Could this use debugfs_create_file_unsafe() instead?
>
>As this isn't a critical path, I don't think we really care here if
>it's wrapped or not. But it's nice to know that it is.
>
>Anyway, I'll let others review this, but it looks fine to me.
>
>-- Steve
>
the fix is a couple of years in the making, heh
Thanks!