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

[Bradley Morgan]

On 8 September 2026 02:10:17 BST, Masami Hiramatsu <mhiramat@xxxxxxxxxx>
wrote:
>On Fri, 21 Aug 2026 09:23:16 +0100
>Bradley Morgan <include@xxxxxxxxx> wrote:
>
>> On 21 August 2026 02:56:48 BST, Masami Hiramatsu <mhiramat@xxxxxxxxxx>
>> wrote:
>> >On Thu, 20 Aug 2026 16:14:42 +0100
>> >Bradley Morgan <include@xxxxxxxxx> wrote:
>> >
>> >> On 19 August 2026 00:12:40 BST, Bradley Morgan <include@xxxxxxxxx>
>> >wrote:
>> >> >On 19 August 2026 00:08:13 BST, Masami Hiramatsu
><mhiramat@xxxxxxxxxx>
>> >> >wrote:
>> >> >>On Tue, 18 Aug 2026 01:53:18 +0000
>> >> >>Bradley Morgan <include@xxxxxxxxx> 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?
>> >
>> >Ah, OK. The phrasing aside, what you said wasn't wrong, so if you're
>happy
>> >with it, I'll pick it. (Let me pick it to probes/core)
>> >
>> 
>> I'm happy with this patch as is, how accurate?
>> 
>> Do the roar! (Pick it, Shrek reference)
>
>OK, I picked this to probes/core. (sorry for late notice)
>
>Thanks,
>
>> 
>> >Thanks.
>> >
>> >
>> >
>> >> 
>> >> 
>> >> >
>> >> >>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@xxxxxxxxx>
>> >> >>> ---
>> >> >>> 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!
>> >
>> >
>> >
>> 
>> Thanks!
>
>
>
Hey, can u change sob to this email if possible? Check lore link below for
info
--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@xxxxxxxxx/