Re: [PATCH] kprobes: use DEFINE_DEB UGFS_ATTRIBUTE for the enabled knob

On 18 August 2026 03:01:18 BST, [email protected] 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.

[email protected] 

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 <[email protected]>
>
>    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!