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

[Bradley Morgan <include@grrlz.net>]

On 19 August 2026 00:12:40 BST, Bradley Morgan <include@grrlz.net> wrote:
>On 19 August 2026 00:08:13 BST, Masami Hiramatsu <mhiramat@kernel.org>
>wrote:
>>On Tue, 18 Aug 2026 01:53:18 +0000
>>Bradley Morgan <include@grrlz.net> 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.
>>
>
>ehhhhh, well, my description very well tells someone about why this is
>bad, and a hack, and everything. 
>
>Your description is good though... But I love to have my personality in my
>descriptions :(
>

Masami, could you give a round down of your nits for V3?


>
>>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 <include@grrlz.net>
>>> ---
>>> 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
>>> 
>>
>>
>>
>
>Thanks!

Thanks!