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!
