On Tue, 18 Aug 2026 01:53:18 +0000
Bradley Morgan <[email protected]> wrote:

> 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.

In other words, does this mean something like the following?

This "enable" knob implemented custom read/write logic, manually writing
"1" or "0" to a 3-byte buffer solely to output a single character. As
suggested by comments calling for `debugfs` to support write callbacks
for boolean files, using `DEFINE_DEBUGFS_ATTRIBUTE` eliminates the need
for these callbacks.

Thanks,

> 
> The behavior does not change, except the write only accepts 0/1 now
> instead of y/n/on/off, and nothing uses anything else.
> 
> Signed-off-by: Bradley Morgan <[email protected]>
> ---
> Changes since v1:
>  - Added the missing Signed-off-by, sorry.
> 
>  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
> 


-- 
Masami Hiramatsu (Google) <[email protected]>

Reply via email to