Skip to contentSkip to conversation

tracing: wprobe: x86: Add wprobe for watchpoint

PATCH Superseded Masami Hiramatsu (Google) 2 participants 16 messages Jul 16

Subsystems (5)

v8 of 19 Superseded Latest is v19 →

lore
Share & tools
lore

Masami Hiramatsu (Google)

author maintainer
#0 lore raw

v8 cover tracing: wprobe: x86: Add wprobe for watchpoint

Hi,

Here is the 8th version of the series for adding new wprobe (watch probe) which provides memory access tracing event. Moreover, this can be used via event trigger. Thus it can trace memory access on a dynamically allocated objects too.
The previous version is here:

https://lore.kernel.org/all/178407983818.95826.12714571928538799781.stgit@devnote2/

This version fixes many issues commented by Sashiko. Let me list it up.

[PATCH 1/9]
- Include required header files.
- Use READ_ONCE(tw->addr) in trace handler to safely check dynamically updated addresses.
- Prohibit unsafe perf support by returning -EOPNOTSUPP in wprobe_register().
- Add rollback logic to unregister already-enabled sibling probes if registration fails mid-loop.
- Resolve symbol offsets dynamically in trace_wprobe_show() using kallsyms_lookup_name().
- Fix memory leak of parse_address_spec()'s symbol output in __trace_wprobe_create().
- Print "rw" instead of "x" for read-write type breakpoints in trace_wprobe_show().
- Document the $value fetcharg in wprobetrace.rst.
[PATCH 3/9]
- Fixed silently test failure path.
[PATCH 5/9]
- Add missing barrier() on the disable path of setup_hwbp() to prevent the compiler from reordering this_cpu_write(cpu_dr7, ...) before set_debugreg(dr7, 7).
- Clear existing slot control and enable bits in cpu_dr7 inside setup_hwbp() using __encode_dr7() before setting new ones to prevent register state corruption on update/reinstall.
[PATCH 7/9]
- Parse new attributes into a temporary struct arch_hw_breakpoint copy to prevent in-place mutation and corruption on parse error paths.
- Synchronize logical perf_event attributes by updating bp->attr.bp_type and bp->attr.bp_len in modify_wide_hw_breakpoint_local().
[PATCH 8/9]
- Redesign wprobe_trigger() to be safe in NMI/hardirq contexts by deferring register updates to a workqueue via irq_work.
- Skip trigger execution and increment an atomic missed count (tw->missed) if the tracepoint runs in NMI context to prevent recursive spinlock deadlocks. (this is currently hidden counter)
- Prohibit attaching wprobe triggers to kprobe_events by checking TRACE_EVENT_FL_KPROBE in wprobe_trigger_cmd_parse().
- Use call_rcu() in trigger deactivation path and add synchronize_rcu() in parse failure path to ensure safe RCU lifetime cleanup.
- Acquire the target tracepoint's module reference via trace_event_try_get_ref() to prevent module refcount underflows.
- Fix event_trigger_data memory leak by properly freeing the initial refcount in wprobe_trigger_cmd_parse() on success.
- Call on_each_cpu() with wait=true in wprobe_work_func() to prevent use-after-free during trigger unregistration.
- Synchronize concurrent wprobe triggers on the same event by using a shared raw spinlock (tw->lock).
- Drop the support of kprobe events (that should be done later).
[PATCH 9/9]
- Use TMPDIR for making a test file.
- Ensure the fprobe and target functions exists, if not, exit with UNRESOLVED (== skip).

To support kprobe events, we need to check the previous NMI context or need to use an atomic refcount etc. But that should be another patch for review.

To support arm64, we need to avoid major pagefault on single-stepping issue. I think we can solve it with checking whether the address is the kernel (which must not cause a major page fault), or not.

Usage
-----

The basic usage of this wprobe is similar to other probes;

w:[GRP/][EVENT] [r|w|rw]@<ADDRESS|SYMBOL[+OFFS]> [FETCHARGS]

This defines a new wprobe event. For example, to trace jiffies update, you can do;

echo 'w:my_jiffies w@jiffies:8 value=+0($addr)' >> dynamic_events echo 1 > events/wprobes/my_jiffies/enable

Moreover, this can be combined with event trigger to trace the memory accecss on slab objects. The trigger syntax is;

set_wprobe:WPROBE_EVENT:FIELD[+OFFSET] [if FILTER] clear_wprobe:WPROBE_EVENT[:FIELD[+OFFSET]] [if FILTER]

set_wprobe sets WPROBE_EVENT's watch address on FIELD[+OFFSET]. clear_wprobe clears WPROBE_EVENT's watch address if it is set to FIELD[+OFFSET]. If FIELD is omitted, forcibly clear the watch address when trigger event is hit.

For example, trace the first 8 byte of the dentry data structure passed to do_truncate() until it is deleted by dentry_kill().
(Note: all tracefs setup uses '>>' so that it does not kick do_truncate())

  # echo 'w:watch rw@0:8 address=$addr value=+0($addr)' > dynamic_events
  # echo 'f:truncate do_truncate dentry=$arg2' >> dynamic_events
  # echo 'set_wprobe:watch:dentry' >> events/fprobes/truncate/trigger
  # echo 'f:dentry_kill dentry_kill dentry=$arg1' >> dynamic_events
  # echo 'clear_wprobe:watch:dentry' >> events/fprobes/dentry_kill/trigger
  # echo 1 >> events/fprobes/truncate/enable
  # echo 1 >> events/fprobes/dentry_kill/enable
  # echo aaa > /tmp/hoge
  # echo bbb > /tmp/hoge
  # echo ccc > /tmp/hoge
  # rm /tmp/hoge

Then, the trace data will show;

# tracer: nop
#
# entries-in-buffer/entries-written: 32/32 #P:8
#
# _-----=> irqs-off/BH-disabled
# / _----=> need-resched

 #                              | / _---=> hardirq/softirq
 #                              || / _--=> preempt-depth
 #                              ||| / _-=> migrate-disable
 #                              |||| /     delay
 #           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
 #              | |         |   |||||     |         |
               sh-107     [004] ...1.     9.990418: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004ad6618
               sh-107     [004] ...1.     9.990914: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004b3de78
               sh-107     [004] ...1.     9.993175: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049ddd40
               sh-107     [004] .....     9.995198: truncate: (do_truncate+0x4/0x120) dentry=0xffff8880048083a8
               sh-107     [004] ...1.     9.995389: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049db998
               sh-107     [004] ..Zff     9.997503: watch: (lookup_fast+0xaa/0x150) address=0xffff8880048083a8 value=0x8200080
               sh-107     [004] ..Zff     9.997509: watch: (path_openat+0x211/0xda0) address=0xffff8880048083a8 value=0x8200080
               sh-107     [004] ..Zff     9.997514: watch: (path_openat+0xa56/0xda0) address=0xffff8880048083a8 value=0x8200080
               sh-107     [004] ..Zff     9.997518: watch: (path_openat+0xae2/0xda0) address=0xffff8880048083a8 value=0x8200080
               sh-107     [004] .....     9.997521: truncate: (do_truncate+0x4/0x120) dentry=0xffff8880048083a8
               sh-107     [004] ...1.     9.997582: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004808270
               sh-107     [004] ...1.     9.999365: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049db728
               sh-107     [004] ...1.     9.999388: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004b1c000
               rm-113     [005] ..Zff    10.000965: watch: (lookup_fast+0xaa/0x150) address=0xffff8880048083a8 value=0x8200080
               rm-113     [005] ..Zff    10.000971: watch: (path_lookupat+0x97/0x1e0) address=0xffff8880048083a8 value=0x8200080
               rm-113     [005] ..Zff    10.000984: watch: (lookup_fast+0xaa/0x150) address=0xffff8880048083a8 value=0x8200080
               rm-113     [005] ..Zff    10.000988: watch: (path_lookupat+0x97/0x1e0) address=0xffff8880048083a8 value=0x8200080
               rm-113     [005] ..Zff    10.001010: watch: (lookup_one_qstr_excl+0x28/0x140) address=0xffff8880048083a8 value=0x8200080
               rm-113     [005] ..Zff    10.001014: watch: (lookup_one_qstr_excl+0xd1/0x140) address=0xffff8880048083a8 value=0x8200080
               rm-113     [005] ..Zff    10.001018: watch: (may_delete_dentry+0x1c/0x200) address=0xffff8880048083a8 value=0x8200080
               rm-113     [005] ..Zff    10.001021: watch: (may_delete_dentry+0x195/0x200) address=0xffff8880048083a8 value=0x8200080
               rm-113     [005] ..Zff    10.001031: watch: (vfs_unlink+0x5e/0x260) address=0xffff8880048083a8 value=0x8200080
               rm-113     [005] d.Z..    10.001067: watch: (d_make_discardable+0x1b/0x40) address=0xffff8880048083a8 value=0x8200080
               rm-113     [005] d.Z..    10.001071: watch: (d_make_discardable+0x29/0x40) address=0xffff8880048083a8 value=0x200080
               rm-113     [005] ...1.    10.001072: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880048083a8
               rm-113     [005] ...1.    10.001218: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880048083a8
               sh-107     [004] ...1.    10.001416: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049db110
               sh-107     [004] ...1.    10.001444: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049db248
               sh-107     [004] ...1.    10.001500: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004ad6618
               sh-107     [004] ...1.    10.002067: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004b41e78
               sh-107     [004] ...1.    10.904920: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004b41e78
               sh-107     [004] ...1.    10.905129: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004ad6618

Thank you,

---

Jinchao Wang (2):
x86/hw_breakpoint: Unify breakpoint install/uninstall x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint

Masami Hiramatsu (Google) (7):
tracing: wprobe: Add watchpoint probe event based on hardware breakpoint
x86: hw_breakpoint: Add a kconfig to clarify when a breakpoint fires
selftests: tracing: Add a basic testcase for wprobe
selftests: tracing: Add syntax testcase for wprobe
HWBP: Add modify_wide_hw_breakpoint_local() API
tracing: wprobe: Add wprobe event trigger
selftests: ftrace: Add wprobe trigger testcase

 Documentation/trace/index.rst                      |    1 
 Documentation/trace/wprobetrace.rst                |  163 +++
 arch/Kconfig                                       |   20 
 arch/x86/Kconfig                                   |    2 
 arch/x86/include/asm/hw_breakpoint.h               |    8 
 arch/x86/kernel/hw_breakpoint.c                    |  151 ++-
 include/linux/hw_breakpoint.h                      |    6 
 include/linux/trace_events.h                       |    3 
 kernel/events/hw_breakpoint.c                      |   43 +
 kernel/trace/Kconfig                               |   24 
 kernel/trace/Makefile                              |    1 
 kernel/trace/trace.c                               |    9 
 kernel/trace/trace.h                               |    6 
 kernel/trace/trace_probe.c                         |   22 
 kernel/trace/trace_probe.h                         |    8 
 kernel/trace/trace_wprobe.c                        | 1146 ++++++++++++++++++++
 tools/testing/selftests/ftrace/config              |    2 
 .../ftrace/test.d/dynevent/add_remove_wprobe.tc    |   69 +
 .../test.d/dynevent/wprobes_syntax_errors.tc       |   20 
 .../ftrace/test.d/trigger/trigger-wprobe.tc        |   65 +
 20 files changed, 1703 insertions(+), 66 deletions(-)
 create mode 100644 Documentation/trace/wprobetrace.rst
 create mode 100644 kernel/trace/trace_wprobe.c
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/wprobes_syntax_errors.tc
 create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-wprobe.tc

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

author maintainer
#1 lore raw

PATCH v8 1/9 tracing: wprobe: Add watchpoint probe event based on hardware breakpoint

From: Masami Hiramatsu (Google) <[email protected]>

Add a new probe event for the hardware breakpoint called wprobe-event. This wprobe allows user to trace (watch) the memory access at the specified memory address.
The new syntax is;

w[:[GROUP/]EVENT] [r|w|rw]@[ADDR|SYM][:SIZE] [FETCH_ARGs]

User also can use $addr to fetch the accessed address and $value to fetch the accessed memory value (shorthand for '+0($addr)'). No other variables are supported.

For example, tracing updates of the jiffies;

/sys/kernel/tracing # echo 'w:my_jiffies w@jiffies' >> dynamic_events /sys/kernel/tracing # cat dynamic_events w:wprobes/my_jiffies w@jiffies:4
/sys/kernel/tracing # echo 1 > events/wprobes/my_jiffies/enable /sys/kernel/tracing # head -n 20 trace | tail -n 5

 #           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
 #              | |         |   |||||     |         |
          <idle>-0       [000] d.Z1.   206.547317: my_jiffies: (tick_do_update_jiffies64+0xbe/0x130)
          <idle>-0       [000] d.Z1.   206.548341: my_jiffies: (tick_do_update_jiffies64+0xbe/0x130)
          <idle>-0       [000] d.Z1.   206.549346: my_jiffies: (tick_do_update_jiffies64+0xbe/0x130)
Assisted-by Antigravity:gemini-3.5-flash

---
Changes in v8:
- Include required header files.
- Use READ_ONCE(tw->addr) in trace handler to safely check dynamically updated addresses.
- Prohibit unsafe perf support by returning -EOPNOTSUPP in wprobe_register().
- Add rollback logic to unregister already-enabled sibling probes if registration fails mid-loop.
- Resolve symbol offsets dynamically in trace_wprobe_show() using kallsyms_lookup_name().
- Fix memory leak of parse_address_spec()'s symbol output in

    __trace_wprobe_create().
  - Print "rw" instead of "x" for read-write type breakpoints in
    trace_wprobe_show().
  - Document the $value fetcharg in wprobetrace.rst.
 Changes in v7:
  - Include IS_ERR_PCPU fix.
  - use seq_print_ip_sym_offset().
  - fix checkpatch warning on DEFINE_FREE()
  - Use bp->attr.bp_addr instead of tw->addr because it can be updated from another CPU.
---
 Documentation/trace/index.rst       |    1 
 Documentation/trace/wprobetrace.rst |   70 +++
 include/linux/trace_events.h        |    2 
 kernel/trace/Kconfig                |   13 +
 kernel/trace/Makefile               |    1 
 kernel/trace/trace.c                |    9 
 kernel/trace/trace.h                |    5 
 kernel/trace/trace_probe.c          |   22 +
 kernel/trace/trace_probe.h          |    8 
 kernel/trace/trace_wprobe.c         |  709 +++++++++++++++++++++++++++++++++++
 10 files changed, 837 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/trace/wprobetrace.rst
 create mode 100644 kernel/trace/trace_wprobe.c
Documentation/trace/index.rst +1 -0
index 5d9bf4694d5d..2f04f32001ed 100644
@@ -36,6 +36,7 @@ the Linux kernel.
    kprobes
    kprobetrace
    fprobetrace
+   wprobetrace
    eprobetrace
    fprobe
    ring-buffer-design
Documentation/trace/wprobetrace.rst new file +70 -0

70 lines not on this page · raw message

include/linux/trace_events.h +2 -0
index 308c76b57d13..d1e5ab71d928 100644
@@ -328,6 +328,7 @@ enum {
 	TRACE_EVENT_FL_UPROBE_BIT,
 	TRACE_EVENT_FL_EPROBE_BIT,
 	TRACE_EVENT_FL_FPROBE_BIT,
+	TRACE_EVENT_FL_WPROBE_BIT,
 	TRACE_EVENT_FL_CUSTOM_BIT,
 	TRACE_EVENT_FL_TEST_STR_BIT,
 };
@@ -358,6 +359,7 @@ enum {
 	TRACE_EVENT_FL_UPROBE		= (1 << TRACE_EVENT_FL_UPROBE_BIT),
 	TRACE_EVENT_FL_EPROBE		= (1 << TRACE_EVENT_FL_EPROBE_BIT),
 	TRACE_EVENT_FL_FPROBE		= (1 << TRACE_EVENT_FL_FPROBE_BIT),
+	TRACE_EVENT_FL_WPROBE		= (1 << TRACE_EVENT_FL_WPROBE_BIT),
 	TRACE_EVENT_FL_CUSTOM		= (1 << TRACE_EVENT_FL_CUSTOM_BIT),
 	TRACE_EVENT_FL_TEST_STR		= (1 << TRACE_EVENT_FL_TEST_STR_BIT),
 };
kernel/trace/Kconfig +13 -0
index 0ab5916575a9..b58c2565024f 100644
@@ -862,6 +862,19 @@ config EPROBE_EVENTS
 	  convert the type of an event field. For example, turn an
 	  address into a string.
 
+config WPROBE_EVENTS
+	bool "Enable wprobe-based dynamic events"
+	depends on TRACING
+	depends on HAVE_HW_BREAKPOINT
+	select PROBE_EVENTS
+	select DYNAMIC_EVENTS
+	help
+	  This allows the user to add watchpoint tracing events based on
+	  hardware breakpoints on the fly via the ftrace interface.
+
+	  Those events can be inserted wherever hardware breakpoints can be
+	  set, and record accessed memory address and values.
+
 config BPF_EVENTS
 	depends on BPF_SYSCALL
 	depends on (KPROBE_EVENTS || UPROBE_EVENTS) && PERF_EVENTS
kernel/trace/Makefile +1 -0
index f934ff586bd4..141c8323de20 100644
@@ -126,6 +126,7 @@ obj-$(CONFIG_FTRACE_RECORD_RECURSION) += trace_recursion_record.o
 obj-$(CONFIG_FPROBE) += fprobe.o
 obj-$(CONFIG_RETHOOK) += rethook.o
 obj-$(CONFIG_FPROBE_EVENTS) += trace_fprobe.o
+obj-$(CONFIG_WPROBE_EVENTS) += trace_wprobe.o
 
 obj-$(CONFIG_TRACEPOINT_BENCHMARK) += trace_benchmark.o
 obj-$(CONFIG_RV) += rv/
kernel/trace/trace.c +8 -1
index c9e182d40059..1bc27c0ad029 100644
@@ -4294,8 +4294,12 @@ static const char readme_msg[] =
 	"  uprobe_events\t\t- Create/append/remove/show the userspace dynamic events\n"
 	"\t\t\t  Write into this file to define/undefine new trace events.\n"
 #endif
+#ifdef CONFIG_WPROBE_EVENTS
+	"  wprobe_events\t\t- Create/append/remove/show the hardware breakpoint dynamic events\n"
+	"\t\t\t  Write into this file to define/undefine new trace events.\n"
+#endif
 #if defined(CONFIG_KPROBE_EVENTS) || defined(CONFIG_UPROBE_EVENTS) || \
-    defined(CONFIG_FPROBE_EVENTS)
+    defined(CONFIG_FPROBE_EVENTS) || defined(CONFIG_WPROBE_EVENTS)
 	"\t  accepts: event-definitions (one definition per line)\n"
 #if defined(CONFIG_KPROBE_EVENTS) || defined(CONFIG_UPROBE_EVENTS)
 	"\t   Format: p[:[<group>/][<event>]] <place> [<args>]\n"
@@ -4305,6 +4309,9 @@ static const char readme_msg[] =
 	"\t           f[:[<group>/][<event>]] <func-name>[%return] [<args>]\n"
 	"\t           t[:[<group>/][<event>]] <tracepoint> [<args>]\n"
 #endif
+#ifdef CONFIG_WPROBE_EVENTS
+	"\t           w[:[<group>/][<event>]] [r|w|rw]@<addr>[:<len>]\n"
+#endif
 #ifdef CONFIG_HIST_TRIGGERS
 	"\t           s:[synthetic/]<event> <field> [<field>]\n"
 #endif
kernel/trace/trace.h +5 -0
index 80fe152af1dd..2f07c5c4ffc8 100644
@@ -179,6 +179,11 @@ struct fexit_trace_entry_head {
 	unsigned long		ret_ip;
 };
 
+struct wprobe_trace_entry_head {
+	struct trace_entry	ent;
+	unsigned long		ip;
+};
+
 #define TRACE_BUF_SIZE		1024
 
 struct trace_array;
kernel/trace/trace_probe.c +21 -1
index 18c212122344..2600d9699bd8 100644
@@ -1344,6 +1344,24 @@ static int parse_probe_vars(char *orig_arg, const struct fetch_type *t,
 		return 0;
 	}
 
+	/* wprobe only support "$addr" and "$value" variable */
+	if (ctx->flags & TPARG_FL_WPROBE) {
+		if (!strcmp(arg, "addr")) {
+			code->op = FETCH_OP_BADDR;
+			return 0;
+		}
+		if (!strcmp(arg, "value")) {
+			code->op = FETCH_OP_BADDR;
+			code++;
+			code->op = FETCH_OP_DEREF;
+			code->offset = 0;
+			*pcode = code;
+			return 0;
+		}
+		err = TP_ERR_BAD_VAR;
+		goto inval;
+	}
+
 	if (str_has_prefix(arg, "retval")) {
 		if (!(ctx->flags & TPARG_FL_RETURN)) {
 			err = TP_ERR_RETVAL_ON_PROBE;
@@ -1491,8 +1509,9 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
 		ret = parse_probe_vars(arg, type, pcode, end, ctx);
 		break;
 
+#ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
 	case '%':	/* named register */
-		if (ctx->flags & (TPARG_FL_TEVENT | TPARG_FL_FPROBE)) {
+		if (ctx->flags & (TPARG_FL_TEVENT | TPARG_FL_FPROBE | TPARG_FL_WPROBE)) {
 			/* eprobe and fprobe do not handle registers */
 			trace_probe_log_err(ctx->offset, BAD_VAR);
 			break;
@@ -1505,6 +1524,7 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
 		} else
 			trace_probe_log_err(ctx->offset, BAD_REG_NAME);
 		break;
+#endif
 
 	case '@':	/* memory, file-offset or symbol */
 		if (isdigit(arg[1])) {
kernel/trace/trace_probe.h +7 -1
index e6268a8dc378..64c5fe9bdfc9 100644
@@ -90,6 +90,7 @@ typedef int (*print_type_func_t)(struct trace_seq *, void *, void *);
 	FETCH_OP(STACK, param),		/* Stack: .param = index */	\
 	FETCH_OP(STACKP, none),		/* Stack pointer */		\
 	FETCH_OP(RETVAL, none),		/* Return value */		\
+	FETCH_OP(BADDR, none),		/* Break address */		\
 	FETCH_OP(IMM, imm),		/* Immediate: .immediate */	\
 	FETCH_OP(COMM, none),		/* Current comm */		\
 	FETCH_OP(CURRENT, none),	/* Current task_struct address */\
@@ -419,6 +420,7 @@ static inline int traceprobe_get_entry_data_size(struct trace_probe *tp)
 #define TPARG_FL_USER   BIT(4)
 #define TPARG_FL_FPROBE BIT(5)
 #define TPARG_FL_TPOINT BIT(6)
+#define TPARG_FL_WPROBE BIT(7)
 #define TPARG_FL_LOC_MASK	GENMASK(4, 0)
 
 static inline bool tparg_is_function_entry(unsigned int flags)
@@ -600,7 +602,11 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
 	C(TYPECAST_SYM_OFFSET,	"@SYM+/-OFFSET with typecast needs parentheses"), \
 	C(TYPECAST_NOT_ALIGNED,	"Typecast field option is not byte-aligned"), \
 	C(TYPECAST_BAD_ARROW,	"Typecast field option does not support -> operator"), \
-	C(NOSUP_PERCPU,		"Per-cpu variable access is only for kernel probes"),
+	C(NOSUP_PERCPU,		"Per-cpu variable access is only for kernel probes"), \
+	C(BAD_ACCESS_FMT,	"Access memory address requires @"),	\
+	C(BAD_ACCESS_TYPE,	"Bad memory access type"),	\
+	C(BAD_ACCESS_LEN,	"This memory access length is not supported"), \
+	C(BAD_ACCESS_ADDR,	"Invalid access memory address"),
 
 #undef C
 #define C(a, b)		TP_ERR_##a
kernel/trace/trace_wprobe.c new file +709 -0

709 lines not on this page · raw message

#2 lore raw

Re: [PATCH v8 1/9] tracing: wprobe: Add watchpoint probe event based on hardware breakpoint

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] Missing traceprobe_update_arg() call leaves @SYM fetch arguments unresolved, causing reads from…

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Missing traceprobe_update_arg() call leaves @SYM fetch arguments unresolved, causing reads from address 0.
- [Medium] Sibling probe support is broken by a hardcoded uniqueness check, rendering the new rollback logic as dead code and limiting functionality.
- [Medium] The implementation fails to dynamically generate a unique event name for anonymous address-based watchpoints, contradicting the documentation.
- [Medium] Missing group and event name parsing allows invalid characters and breaks tracefs directory creation.
--

--- Patch [1]: [PATCH v8 1/9] tracing: wprobe: Add watchpoint probe event based on hardware breakpoint --- commit c7cef5689391cf83a545269b44666666b82afb31
Author: Masami Hiramatsu (Google) <[email protected]>

tracing: wprobe: Add watchpoint probe event based on hardware breakpoint

This commit adds a new wprobe-event for hardware breakpoints, allowing users to trace memory accesses at specific addresses. It supports fetching the accessed address and value, and can be configured via the dynamic_events interface.

kernel/trace/trace_wprobe.c new file +0 -0
new file mode 100644

[ ... ]

Quoted 11 lines

static int register_trace_wprobe_event(struct trace_wprobe *tw)

{
	struct trace_wprobe *old_tb;
	int ret;

	guard(mutex)(&event_mutex);

	old_tb = find_trace_wprobe(trace_probe_name(&tw->tp),
				   trace_probe_group_name(&tw->tp));
	if (old_tb)
		return -EBUSY;

[Severity: Medium]
Does this uniqueness check inadvertently break sibling probe support?

The commit message mentions rollback logic for already-enabled sibling probes, but returning -EBUSY here unconditionally rejects adding a wprobe to an existing event name.

Shouldn't this call trace_probe_append() for existing events similar to how kprobes and fprobes handle them, instead of returning -EBUSY?

[ ... ]

Quoted 2 lines

static int __trace_wprobe_create(int argc, const char *argv[]) {

[ ... ]

Quoted 9 lines
	if (argv[0][1] != '\0') {
		if (argv[0][1] != ':') {
			trace_probe_log_set_index(0);
			trace_probe_log_err(1, BAD_MAXACT_TYPE);
			/* Invalid format */
			return -EINVAL;
		}
		event = &argv[0][2];
	}

[Severity: Medium]
Is this missing a call to traceprobe_parse_event_name()?

By assigning the remainder of the command directly to the event name, it bypasses separating the group from the event and checking against is_good_name().

If a user provides a group like w:mygroup/myevent, could this cause tracefs directory creation to fail or create a malformed structure?

Quoted 8 lines
	trace_probe_log_set_index(1);
	ret = parse_address_spec(argv[1], &addr, &type, &len, &symbol);
	if (ret < 0)
		return ret;

	if (!event)
		event = symbol ? symbol : "wprobe";

[Severity: Medium]
If multiple anonymous address-based watchpoints are created without a symbol (for example, w@0x12345678 and w@0xcafebabe), won't they all be assigned the hardcoded wprobe name?

Does this need to dynamically generate a unique name based on the address (perhaps similar to what kprobes does with snprintf) to avoid naming collisions?

Quoted 12 lines
	argc -= 2; argv += 2;
	tw = alloc_trace_wprobe(group, event, symbol, addr, len, type, argc);
	if (IS_ERR(tw))
		return PTR_ERR(tw);

	ctx = kzalloc_obj(*ctx);
	if (!ctx)
		return -ENOMEM;

	ctx->flags = TPARG_FL_KERNEL | TPARG_FL_WPROBE;
Quoted 8 lines
	/* parse arguments */
	for (i = 0; i < argc; i++) {
		trace_probe_log_set_index(i + 2);
		ctx->offset = 0;
		ret = traceprobe_parse_probe_arg(&tw->tp, i, argv[i], ctx);
		if (ret)
			return ret;	/* This can be -ENOMEM */
	}

[Severity: High]
Does this argument parsing loop need to call traceprobe_update_arg() to resolve symbol references?

If a user registers a wprobe event with a symbol fetch argument like @my_symbol, the parser generates a FETCH_NOP_SYMBOL instruction. Without calling traceprobe_update_arg(&tw->tp.args[i]), the fetch instruction's immediate address remains 0.

Won't this cause the probe to silently fail by attempting to read from address 0 at runtime?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/178417033089.209165.16717079876036408877.stgit@devnote2?part=1
author maintainer
#3 lore raw

PATCH v8 2/9 x86: hw_breakpoint: Add a kconfig to clarify when a breakpoint fires

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Add CONFIG_HAVE_POST_BREAKPOINT_HOOK which indicates the hw_breakpoint on that architecture fires after the target memory has been modified. This is currently x86 only behavior.

---
 arch/Kconfig         |   10 ++++++++++
 arch/x86/Kconfig     |    1 +
 kernel/trace/Kconfig |    1 +
 3 files changed, 12 insertions(+)
arch/Kconfig +10 -0
index fa7507ac8e13..959aee9568ff 100644
@@ -457,6 +457,16 @@ config HAVE_MIXED_BREAKPOINTS_REGS
 	  Select this option if your arch implements breakpoints under the
 	  latter fashion.
 
+config HAVE_POST_BREAKPOINT_HOOK
+	bool
+	depends on HAVE_HW_BREAKPOINT
+	help
+	  Depending on the arch implementation of hardware breakpoints,
+	  some of them provide breakpoint hook after the target memory
+	  is modified.
+	  Select this option if your arch implements breakpoints overflow
+	  handler hooks after the target memory is modified.
+
 config HAVE_USER_RETURN_NOTIFIER
 	bool
 
arch/x86/Kconfig +1 -0
index bdad90f210e4..6b7e14ef8cfb 100644
@@ -246,6 +246,7 @@ config X86
 	select HAVE_FUNCTION_TRACER
 	select HAVE_GCC_PLUGINS
 	select HAVE_HW_BREAKPOINT
+	select HAVE_POST_BREAKPOINT_HOOK
 	select HAVE_IOREMAP_PROT
 	select HAVE_IRQ_EXIT_ON_IRQ_STACK	if X86_64
 	select HAVE_IRQ_TIME_ACCOUNTING
kernel/trace/Kconfig +1 -0
index b58c2565024f..d9b6fa5c35d9 100644
@@ -866,6 +866,7 @@ config WPROBE_EVENTS
 	bool "Enable wprobe-based dynamic events"
 	depends on TRACING
 	depends on HAVE_HW_BREAKPOINT
+	depends on HAVE_POST_BREAKPOINT_HOOK
 	select PROBE_EVENTS
 	select DYNAMIC_EVENTS
 	help
author maintainer
#4 lore raw

PATCH v8 3/9 selftests: tracing: Add a basic testcase for wprobe

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Add 'add_remove_wprobe.tc' testcase for testing wprobe event that tests adding and removing operations of the wprobe event.

---
Changes in v8:
- Fixed silently test failure path.

---
 tools/testing/selftests/ftrace/config              |    1 
 .../ftrace/test.d/dynevent/add_remove_wprobe.tc    |   69 ++++++++++++++++++++
 2 files changed, 70 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc
tools/testing/selftests/ftrace/config +1 -0
index 544de0db5f58..d2f503722020 100644
@@ -27,3 +27,4 @@ CONFIG_STACK_TRACER=y
 CONFIG_TRACER_SNAPSHOT=y
 CONFIG_UPROBES=y
 CONFIG_UPROBE_EVENTS=y
+CONFIG_WPROBE_EVENTS=y
tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc new file +69 -0

69 lines not on this page · raw message

#5 lore raw

Re: [PATCH v8 3/9] selftests: tracing: Add a basic testcase for wprobe

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Error handling blocks checking `$?` are dead code because the test script runs under `set -e`. --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Error handling blocks checking $? are dead code because the test script runs under set -e. --

--- Patch [3]: [PATCH v8 3/9] selftests: tracing: Add a basic testcase for wprobe --- commit 0ee1282cbf519c56b378956b7875111e8d6bc222
Author: Masami Hiramatsu (Google) <[email protected]>

selftests: tracing: Add a basic testcase for wprobe

This commit adds a basic testcase 'add_remove_wprobe.tc' to verify the adding and removing operations for wprobe events.

Quoted 7 lines
tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc new file +1 -0
new file mode 100644
index 0000000000000..5ddf28ddd2c4b
@@ -0,0 +1,69 @@
+#!/bin/sh

[ ... ]

tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc line 15

Quoted 4 lines
tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc new file +4 -0
+echo "w:my_wprobe w@$TARGET" >> dynamic_events
+
+grep -q my_wprobe dynamic_events
+if [ $? -ne 0 ]; then

[Severity: Low]
Since ftracetest executes these scripts with errexit (set -e) enabled, will the script exit immediately if the grep command fails?

If the script aborts on the failed command, this if block and the others like it throughout the script become dead code, preventing the custom diagnostic messages from being printed in the failure logs.

tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc line 21

Quoted 6 lines
tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc new file +6 -0
+    echo "Failed to create wprobe event"
+    exit_fail
+fi
+
+test -d events/wprobes/my_wprobe
+if [ $? -ne 0 ]; then

[Severity: Low]
Similarly here, does the test command cause an immediate exit before $? can be evaluated?

Quoted 3 lines
tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc new file +3 -0
+    echo "Failed to create wprobe event directory"
+    exit_fail
+fi

[ ... ]

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/178417033089.209165.16717079876036408877.stgit@devnote2?part=3
author maintainer
#6 lore raw

PATCH v8 4/9 selftests: tracing: Add syntax testcase for wprobe

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Add "wprobe_syntax_errors.tc" testcase for testing syntax errors of the watch probe events.

---
 .../test.d/dynevent/wprobes_syntax_errors.tc       |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/wprobes_syntax_errors.tc
tools/testing/selftests/ftrace/test.d/dynevent/wprobes_syntax_errors.tc new file +20 -0
new file mode 100644
index 000000000000..56ac579d60ae
@@ -0,0 +1,20 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Watch probe event parser error log check
+# requires: dynamic_events "w[:[<group>/][<event>]] [r|w|rw]@<addr>[:<len>]":README
+
+check_error() { # command-with-error-pos-by-^
+    ftrace_errlog_check 'wprobe' "$1" 'dynamic_events'
+}
+
+check_error 'w ^symbol'			# BAD_ACCESS_FMT
+check_error 'w ^a@symbol'		# BAD_ACCESS_TYPE
+check_error 'w w@^symbol'		# BAD_ACCESS_ADDR
+check_error 'w w@jiffies^+offset'	# BAD_ACCESS_ADDR
+check_error 'w w@jiffies:^100'		# BAD_ACCESS_LEN
+check_error 'w w@jiffies ^$arg1'	# BAD_VAR
+check_error 'w w@jiffies ^$retval'	# BAD_VAR
+check_error 'w w@jiffies ^$stack'	# BAD_VAR
+check_error 'w w@jiffies ^%ax'		# BAD_VAR
+
+exit 0
author maintainer
#7 lore raw

PATCH v8 5/9 x86/hw_breakpoint: Unify breakpoint install/uninstall

In reply to Masami Hiramatsu (Google)

From: Jinchao Wang <[email protected]>

Consolidate breakpoint management to reduce code duplication. The diffstat was misleading, so the stripped code size is compared instead. After refactoring, it is reduced from 11976 bytes to 11448 bytes on my x86_64 system built with clang.

This also makes it easier to introduce arch_reinstall_hw_breakpoint().

In addition, including linux/types.h to fix a missing build dependency.

Assisted-by Antigravity:gemini-3.5-flash

---
Changes in v8:
- Add missing barrier() on the disable path of setup_hwbp() to prevent the compiler from reordering this_cpu_write(cpu_dr7, ...) before set_debugreg(dr7, 7).
- Clear existing slot control and enable bits in cpu_dr7 inside setup_hwbp() using __encode_dr7() before setting new ones to prevent register state corruption on update/reinstall.

---
 arch/x86/include/asm/hw_breakpoint.h |    6 +
 arch/x86/kernel/hw_breakpoint.c      |  144 +++++++++++++++++++---------------
 2 files changed, 86 insertions(+), 64 deletions(-)
arch/x86/include/asm/hw_breakpoint.h +6 -0
index 0bc931cd0698..aa6adac6c3a2 100644
@@ -5,6 +5,7 @@
 #include <uapi/asm/hw_breakpoint.h>
 
 #define	__ARCH_HW_BREAKPOINT_H
+#include <linux/types.h>
 
 /*
  * The name should probably be something dealt in
@@ -18,6 +19,11 @@ struct arch_hw_breakpoint {
 	u8		type;
 };
 
+enum bp_slot_action {
+	BP_SLOT_ACTION_INSTALL,
+	BP_SLOT_ACTION_UNINSTALL,
+};
+
 #include <linux/kdebug.h>
 #include <linux/percpu.h>
 #include <linux/list.h>
arch/x86/kernel/hw_breakpoint.c +80 -64

183 lines not on this page · raw message

author maintainer
#8 lore raw

PATCH v8 6/9 x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint

In reply to Masami Hiramatsu (Google)

From: Jinchao Wang <[email protected]>

The new arch_reinstall_hw_breakpoint() function can be used in an atomic context, unlike the more expensive free and re-allocation path. This allows callers to efficiently re-establish an existing breakpoint.

---
 arch/x86/include/asm/hw_breakpoint.h |    2 ++
 arch/x86/kernel/hw_breakpoint.c      |    9 +++++++++
 2 files changed, 11 insertions(+)
arch/x86/include/asm/hw_breakpoint.h +2 -0
index aa6adac6c3a2..c22cc4e87fc5 100644
@@ -21,6 +21,7 @@ struct arch_hw_breakpoint {
 
 enum bp_slot_action {
 	BP_SLOT_ACTION_INSTALL,
+	BP_SLOT_ACTION_REINSTALL,
 	BP_SLOT_ACTION_UNINSTALL,
 };
 
@@ -65,6 +66,7 @@ extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
 
 
 int arch_install_hw_breakpoint(struct perf_event *bp);
+int arch_reinstall_hw_breakpoint(struct perf_event *bp);
 void arch_uninstall_hw_breakpoint(struct perf_event *bp);
 void hw_breakpoint_pmu_read(struct perf_event *bp);
 void hw_breakpoint_pmu_unthrottle(struct perf_event *bp);
arch/x86/kernel/hw_breakpoint.c +9 -0
index c323c2aab2af..5821bb4650ce 100644
@@ -100,6 +100,10 @@ static int manage_bp_slot(struct perf_event *bp, enum bp_slot_action action)
 		old_bp = NULL;
 		new_bp = bp;
 		break;
+	case BP_SLOT_ACTION_REINSTALL:
+		old_bp = bp;
+		new_bp = bp;
+		break;
 	case BP_SLOT_ACTION_UNINSTALL:
 		old_bp = bp;
 		new_bp = NULL;
@@ -189,6 +193,11 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
 	return arch_manage_bp(bp, BP_SLOT_ACTION_INSTALL);
 }
 
+int arch_reinstall_hw_breakpoint(struct perf_event *bp)
+{
+	return arch_manage_bp(bp, BP_SLOT_ACTION_REINSTALL);
+}
+
 void arch_uninstall_hw_breakpoint(struct perf_event *bp)
 {
 	arch_manage_bp(bp, BP_SLOT_ACTION_UNINSTALL);
#9 lore raw

Re: [PATCH v8 6/9] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Modifying an active breakpoint via `arch_reinstall_hw_breakpoint()` updates the debug address…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Modifying an active breakpoint via arch_reinstall_hw_breakpoint() updates the debug address register before disabling it in the control register, violating architectural requirements and creating a race condition that can trigger spurious #DB exceptions.
- [Low] The newly added function arch_reinstall_hw_breakpoint() has no callers in the kernel tree.
--

--- Patch [6]: [PATCH v8 6/9] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint --- commit 26fd09a1abe10d2182564575b6e349d84a888d45
Author: Jinchao Wang <[email protected]>

x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint

The new arch_reinstall_hw_breakpoint() function can be used in an atomic context to efficiently re-establish an existing breakpoint without going through the more expensive free and re-allocation path.

Quoted 4 lines
arch/x86/kernel/hw_breakpoint.c +0 -0
index c323c2aab2af9..5821bb4650cec 100644

[ ... ]

arch/x86/kernel/hw_breakpoint.c int arch_install_hw_breakpoint(struct perf_event *bp) · line 199

Quoted 8 lines
arch/x86/kernel/hw_breakpoint.c +4 -0
@@ -189,6 +193,11 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
 	return arch_manage_bp(bp, BP_SLOT_ACTION_INSTALL);
 }
 
+int arch_reinstall_hw_breakpoint(struct perf_event *bp)
+{
+	return arch_manage_bp(bp, BP_SLOT_ACTION_REINSTALL);
+}

[Severity: High]
Does this sequence risk triggering spurious hardware debug exceptions?

When arch_reinstall_hw_breakpoint() modifies an active breakpoint, it calls arch_manage_bp() which then invokes setup_hwbp(). In setup_hwbp(), the debug address register is updated before the control register (dr7) is safely cleared:

arch/x86/kernel/hw_breakpoint.c:setup_hwbp() {
    ...
    set_debugreg(info->address, slot);
    ...
    dr7 &= ~(__encode_dr7(slot, 0xc, 0x3) |
             (DR_LOCAL_ENABLE << (slot * DR_ENABLE_SIZE)));
    ...
}

Because BP_SLOT_ACTION_REINSTALL operates on a breakpoint that might already be active, dr7 is currently enabled in hardware during this window. Can writing to the address register while the slot is still enabled in dr7 trigger a spurious debug exception if a concurrent execution accesses the new address?

[Severity: Low]
This isn't a bug, but are there any planned callers for this function?

A search of the tree indicates that arch_reinstall_hw_breakpoint() is not currently invoked by any subsystem or exported to modules.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/178417033089.209165.16717079876036408877.stgit@devnote2?part=6
author maintainer
#10 lore raw

PATCH v8 7/9 HWBP: Add modify_wide_hw_breakpoint_local() API

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Add modify_wide_hw_breakpoint_local() arch-wide interface which allows hwbp users to update watch address on-line. This is available if the arch supports CONFIG_HAVE_REINSTALL_HW_BREAKPOINT.
Note that this allows to change the type only for compatible types, because it does not release and reserve the hwbp slot based on type. For instance, you can not change HW_BREAKPOINT_W to HW_BREAKPOINT_X.

Assisted-by Antigravity:gemini-3.5-flash

---
Changes in v8:
- Parse new attributes into a temporary struct arch_hw_breakpoint copy to prevent in-place mutation and corruption on parse error paths.
- Synchronize logical perf_event attributes by updating bp->attr.bp_type and bp->attr.bp_len in modify_wide_hw_breakpoint_local().
Changes in v7:
- Update bp->attr.bp_attr so that we can correctly check the address on it.
- Use -EOPNOTSUPP instead of -ENOSYS.
Changes in v4:
- Update kerneldoc comment about modify_wide_hw_breakpoint_local according to Randy's comment.
Changes in v2:
- Check type compatibility by checking slot. (Thanks Jinchao!)

---
 arch/Kconfig                  |   10 ++++++++++
 arch/x86/Kconfig              |    1 +
 include/linux/hw_breakpoint.h |    6 ++++++
 kernel/events/hw_breakpoint.c |   43 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+)
arch/Kconfig +10 -0
index 959aee9568ff..4a87e843bb4c 100644
@@ -467,6 +467,16 @@ config HAVE_POST_BREAKPOINT_HOOK
 	  Select this option if your arch implements breakpoints overflow
 	  handler hooks after the target memory is modified.
 
+config HAVE_REINSTALL_HW_BREAKPOINT
+	bool
+	depends on HAVE_HW_BREAKPOINT
+	help
+	  Depending on the arch implementation of hardware breakpoints,
+	  some of them are able to update the breakpoint configuration
+	  without release and reserve the hardware breakpoint register.
+	  What configuration is able to update depends on hardware and
+	  software implementation.
+
 config HAVE_USER_RETURN_NOTIFIER
 	bool
 
arch/x86/Kconfig +1 -0
index 6b7e14ef8cfb..588218da8f41 100644
@@ -247,6 +247,7 @@ config X86
 	select HAVE_GCC_PLUGINS
 	select HAVE_HW_BREAKPOINT
 	select HAVE_POST_BREAKPOINT_HOOK
+	select HAVE_REINSTALL_HW_BREAKPOINT
 	select HAVE_IOREMAP_PROT
 	select HAVE_IRQ_EXIT_ON_IRQ_STACK	if X86_64
 	select HAVE_IRQ_TIME_ACCOUNTING
include/linux/hw_breakpoint.h +6 -0
index db199d653dd1..6754ffbee9ed 100644
@@ -81,6 +81,9 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
 			    perf_overflow_handler_t triggered,
 			    void *context);
 
+extern int modify_wide_hw_breakpoint_local(struct perf_event *bp,
+					   struct perf_event_attr *attr);
+
 extern int register_perf_hw_breakpoint(struct perf_event *bp);
 extern void unregister_hw_breakpoint(struct perf_event *bp);
 extern void unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events);
@@ -124,6 +127,9 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
 			    perf_overflow_handler_t triggered,
 			    void *context)		{ return NULL; }
 static inline int
+modify_wide_hw_breakpoint_local(struct perf_event *bp,
+				struct perf_event_attr *attr) { return -EOPNOTSUPP; }
+static inline int
 register_perf_hw_breakpoint(struct perf_event *bp)	{ return -ENOSYS; }
 static inline void unregister_hw_breakpoint(struct perf_event *bp)	{ }
 static inline void
kernel/events/hw_breakpoint.c +43 -0

49 lines not on this page · raw message

#11 lore raw

Re: [PATCH v8 7/9] HWBP: Add modify_wide_hw_breakpoint_local() API

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] State Corruption on Error Path - [High] Race Condition / Unprotected State Modification with NMI…

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] State Corruption on Error Path
- [High] Race Condition / Unprotected State Modification with NMI
- [Medium] Locking Context / Lockdep Splat Due to Missing IRQ Disabling
--

--- Patch [7]: [PATCH v8 7/9] HWBP: Add modify_wide_hw_breakpoint_local() API --- commit 12f28341e1b86eda783641bab63a14b074ec1ca3
Author: Masami Hiramatsu (Google) <[email protected]>
Subject: HWBP: Add modify_wide_hw_breakpoint_local() API

This patch adds an architecture-wide interface to allow hardware breakpoint users to update watch addresses online. It supports modifying compatible breakpoint types without releasing and reserving the hardware slot.

Quoted 3 lines
kernel/events/hw_breakpoint.c +0 -0

[ ... ]

Quoted 1 line
kernel/events/hw_breakpoint.c +0 -0
@@ -888,6 +888,49 @@ void unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events)

[ ... ]

Quoted 8 lines
kernel/events/hw_breakpoint.c +8 -0
+#ifdef CONFIG_HAVE_REINSTALL_HW_BREAKPOINT
+int modify_wide_hw_breakpoint_local(struct perf_event *bp,
+				    struct perf_event_attr *attr)
+{
+	struct arch_hw_breakpoint info;
+	int ret;
+
+	if (find_slot_idx(bp->attr.bp_type) != find_slot_idx(attr->bp_type))

kernel/events/hw_breakpoint.c void unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events) · line 921

Quoted 10 lines
kernel/events/hw_breakpoint.c +10 -0
+		return -EINVAL;
+
+	ret = hw_breakpoint_arch_parse(bp, attr, &info);
+	if (ret)
+		return ret;
+
+	*counter_arch_bp(bp) = info;
+	bp->attr.bp_addr = attr->bp_addr;
+	bp->attr.bp_type = attr->bp_type;
+	bp->attr.bp_len = attr->bp_len;

[Severity: High]
Is it safe to modify these software attributes non-atomically while the hardware breakpoint is still fully enabled?

If a concurrent hardware debug exception fires and acts as a non-maskable interrupt during this modification, could the exception handler read partially updated or torn attributes and report incorrect trace data?

kernel/events/hw_breakpoint.c

Quoted 2 lines
kernel/events/hw_breakpoint.c +2 -0
+
+	return arch_reinstall_hw_breakpoint(bp);

[Severity: High]
If arch_reinstall_hw_breakpoint() fails, this returns the error code directly, but the internal state of the event has already been permanently modified.

Could this leave the breakpoint object in an inconsistent state where the caller receives an error but the object was mutated?

Should we restore the original state if the architecture update fails?

[Severity: Medium]
Do callers of modify_wide_hw_breakpoint_local() need to have interrupts disabled?

Calling arch_reinstall_hw_breakpoint() on x86 eventually reaches arch_manage_bp(), which contains an unconditional lockdep_assert_irqs_disabled():

modify_wide_hw_breakpoint_local()
arch_reinstall_hw_breakpoint()
arch_manage_bp()
lockdep_assert_irqs_disabled()

Since this exported function doesn't disable interrupts itself or document the requirement, will callers from standard contexts trigger a lockdep splat?

Quoted 2 lines
kernel/events/hw_breakpoint.c +2 -0
+}
+#else

[ ... ]

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/178417033089.209165.16717079876036408877.stgit@devnote2?part=7
author maintainer
#12 lore raw

PATCH v8 8/9 tracing: wprobe: Add wprobe event trigger

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Add wprobe event trigger to set and clear the watch event dynamically. This allows us to set an watchpoint on a given local variables and a slab object instead of static objects.

The trigger syntax is below:

- set_wprobe:WPROBE:FIELD[+OFFSET] [if FILTER]
- clear_wprobe:WPROBE[:FIELD[+OFFSET]] [if FILTER]

set_wprobe sets the address pointed by FIELD[+offset] to the WPROBE event. The FIELD is the field name of trigger event. clear_wprobe clears the watch address of WPROBE event. If the FIELD option is specified, it clears only if the current watch address is same as the given FIELD[+OFFSET] value.

The set_wprobe trigger does not change the type and length, these must be set when creating a new wprobe.

Also, the WPROBE event must be disabled when setting the new trigger and it will be busy afterwards. Recommended usage is to add a new wprobe at NULL address and keep disabled.

Assisted-by Antigravity:gemini-3.5-flash

---
Changes in v8:
- Redesign wprobe_trigger() to be safe in NMI/hardirq contexts by deferring register updates to a workqueue via irq_work.
- Skip trigger execution and increment an atomic missed count (tw->missed) if the tracepoint runs in NMI context to prevent recursive spinlock deadlocks. (this is currently hidden counter)
- Prohibit attaching wprobe triggers to kprobe_events by checking TRACE_EVENT_FL_KPROBE in wprobe_trigger_cmd_parse().
- Use call_rcu() in trigger deactivation path and add synchronize_rcu() in parse failure path to ensure safe RCU lifetime cleanup.
- Acquire the target tracepoint's module reference via trace_event_try_get_ref() to prevent module refcount underflows.
- Fix event_trigger_data memory leak by properly freeing the initial refcount in wprobe_trigger_cmd_parse() on success.
- Call on_each_cpu() with wait=true in wprobe_work_func() to prevent use-after-free during trigger unregistration.
- Synchronize concurrent wprobe triggers on the same event by

    using a shared raw spinlock (tw->lock).
  - Drop the support of kprobe events (that should be done later).
 Changes in v7:
  - Use kzalloc_obj().
  - Update sample code in document.
 Changes in v6:
  - Update according to the latest change of trigger ops.
 Changes in v5:
  - Following the suggestions, the documentation was revised to suit rst.
 Changes in v3:
  - Add FIELD option support for clear_wprobe and update document.
  - Fix to unregister/free event_trigger_data on file correctly.
  - Fix syntax comments.
 Changes in v2:
  - Getting local cpu perf_event from trace_wprobe directly.
  - Remove trace_wprobe_local_perf() because it is conditionally unused.
  - Make CONFIG_WPROBE_TRIGGERS a hidden config.
---
 Documentation/trace/wprobetrace.rst |   93 +++++++
 include/linux/trace_events.h        |    1 
 kernel/trace/Kconfig                |   10 +
 kernel/trace/trace.h                |    1 
 kernel/trace/trace_wprobe.c         |  437 +++++++++++++++++++++++++++++++++++
 5 files changed, 542 insertions(+)
Documentation/trace/wprobetrace.rst +93 -0
index eb4f10607530..a4c0f0e676fd 100644
@@ -68,3 +68,96 @@ Here is an example to add a wprobe event on a variable `jiffies`.
            <idle>-0       [000] d.Z1.  717.026373: my_jiffies: (tick_do_update_jiffies64+0xbe/0x130)
 
 You can see the code which writes to `jiffies` is `tick_do_update_jiffies64()`.
+
+Combination with trigger action
+-------------------------------
+The event trigger action can extend the utilization of this wprobe.
+
+- set_wprobe:WPEVENT:FIELD[+|-ADJUST]
+- clear_wprobe:WPEVENT[:FIELD[+|-]ADJUST]
+
+Set these triggers to the target event, then the WPROBE event will be
+setup to trace the memory access at FIELD[+|-ADJUST] address.
+When clear_wprobe is hit, if FIELD is NOT specified, the WPEVENT is
+forcibly cleared. If FIELD[[+|-]ADJUST] is set, it clears WPEVENT only
+if its watching address is the same as the FIELD[[+|-]ADJUST] value.
+
+Notes:
+The set_wprobe trigger does not change the type and length, these
+must be set when creating a new wprobe.
+
+The WPROBE event must be disabled when setting the new trigger
+and it will be busy afterwards. Recommended usage is to add a new
+wprobe at NULL address and keep disabled.
+
+Wprobe triggers are not supported on kprobe_events, because kprobes
+themselves can use software breakpoints which conflicts with wprobe
+operation.
+
+
+For example, trace the first 8 bytes of the dentry data structure passed
+to do_truncate() until it is deleted by dentry_kill().
+(Note: all tracefs setup uses '>>' so that it does not kick do_truncate())
+::
+
+  # echo 'w:watch rw@0:8 address=$addr value=+0($addr)' >> dynamic_events
+  # echo 'f:truncate do_truncate dentry=$arg2' >> dynamic_events
+  # echo 'set_wprobe:watch:dentry' >> events/fprobes/truncate/trigger
+  # echo 'f:dentry_kill dentry_kill dentry=$arg1' >> dynamic_events
+  # echo 'clear_wprobe:watch:dentry' >> events/fprobes/dentry_kill/trigger
+  # echo 1 >> events/fprobes/truncate/enable
+  # echo 1 >> events/fprobes/dentry_kill/enable
+
+  # echo aaa > /tmp/hoge
+  # echo bbb > /tmp/hoge
+  # echo ccc > /tmp/hoge
+  # rm /tmp/hoge
+
+Then, the trace data will show::
+
+ # tracer: nop
+ #
+ # entries-in-buffer/entries-written: 32/32   #P:8
+ #
+ #                                _-----=> irqs-off/BH-disabled
+ #                               / _----=> need-resched
+ #                              | / _---=> hardirq/softirq
+ #                              || / _--=> preempt-depth
+ #                              ||| / _-=> migrate-disable
+ #                              |||| /     delay
+ #           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
+ #              | |         |   |||||     |         |
+               sh-107     [004] ...1.     9.990418: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004ad6618
+               sh-107     [004] ...1.     9.990914: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004b3de78
+               sh-107     [004] ...1.     9.993175: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049ddd40
+               sh-107     [004] .....     9.995198: truncate: (do_truncate+0x4/0x120) dentry=0xffff8880048083a8
+               sh-107     [004] ...1.     9.995389: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049db998
+               sh-107     [004] ..Zff     9.997503: watch: (lookup_fast+0xaa/0x150) address=0xffff8880048083a8 value=0x8200080
+               sh-107     [004] ..Zff     9.997509: watch: (path_openat+0x211/0xda0) address=0xffff8880048083a8 value=0x8200080
+               sh-107     [004] ..Zff     9.997514: watch: (path_openat+0xa56/0xda0) address=0xffff8880048083a8 value=0x8200080
+               sh-107     [004] ..Zff     9.997518: watch: (path_openat+0xae2/0xda0) address=0xffff8880048083a8 value=0x8200080
+               sh-107     [004] .....     9.997521: truncate: (do_truncate+0x4/0x120) dentry=0xffff8880048083a8
+               sh-107     [004] ...1.     9.997582: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004808270
+               sh-107     [004] ...1.     9.999365: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049db728
+               sh-107     [004] ...1.     9.999388: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004b1c000
+               rm-113     [005] ..Zff    10.000965: watch: (lookup_fast+0xaa/0x150) address=0xffff8880048083a8 value=0x8200080
+               rm-113     [005] ..Zff    10.000971: watch: (path_lookupat+0x97/0x1e0) address=0xffff8880048083a8 value=0x8200080
+               rm-113     [005] ..Zff    10.000984: watch: (lookup_fast+0xaa/0x150) address=0xffff8880048083a8 value=0x8200080
+               rm-113     [005] ..Zff    10.000988: watch: (path_lookupat+0x97/0x1e0) address=0xffff8880048083a8 value=0x8200080
+               rm-113     [005] ..Zff    10.001010: watch: (lookup_one_qstr_excl+0x28/0x140) address=0xffff8880048083a8 value=0x8200080
+               rm-113     [005] ..Zff    10.001014: watch: (lookup_one_qstr_excl+0xd1/0x140) address=0xffff8880048083a8 value=0x8200080
+               rm-113     [005] ..Zff    10.001018: watch: (may_delete_dentry+0x1c/0x200) address=0xffff8880048083a8 value=0x8200080
+               rm-113     [005] ..Zff    10.001021: watch: (may_delete_dentry+0x195/0x200) address=0xffff8880048083a8 value=0x8200080
+               rm-113     [005] ..Zff    10.001031: watch: (vfs_unlink+0x5e/0x260) address=0xffff8880048083a8 value=0x8200080
+               rm-113     [005] d.Z..    10.001067: watch: (d_make_discardable+0x1b/0x40) address=0xffff8880048083a8 value=0x8200080
+               rm-113     [005] d.Z..    10.001071: watch: (d_make_discardable+0x29/0x40) address=0xffff8880048083a8 value=0x200080
+               rm-113     [005] ...1.    10.001072: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880048083a8
+               rm-113     [005] ...1.    10.001218: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880048083a8
+               sh-107     [004] ...1.    10.001416: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049db110
+               sh-107     [004] ...1.    10.001444: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049db248
+               sh-107     [004] ...1.    10.001500: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004ad6618
+               sh-107     [004] ...1.    10.002067: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004b41e78
+               sh-107     [004] ...1.    10.904920: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004b41e78
+               sh-107     [004] ...1.    10.905129: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004ad6618
+
+You can see the watch event is correctly configured on the dentry.
include/linux/trace_events.h +1 -0
index d1e5ab71d928..e6f3bbcbb9af 100644
@@ -729,6 +729,7 @@ enum event_trigger_type {
 	ETT_EVENT_HIST		= (1 << 4),
 	ETT_HIST_ENABLE		= (1 << 5),
 	ETT_EVENT_EPROBE	= (1 << 6),
+	ETT_EVENT_WPROBE	= (1 << 7),
 };
 
 extern int filter_match_preds(struct event_filter *filter, void *rec);
kernel/trace/Kconfig +10 -0
index d9b6fa5c35d9..af570fa2a882 100644
@@ -876,6 +876,16 @@ config WPROBE_EVENTS
 	  Those events can be inserted wherever hardware breakpoints can be
 	  set, and record accessed memory address and values.
 
+config WPROBE_TRIGGERS
+	depends on WPROBE_EVENTS
+	depends on HAVE_REINSTALL_HW_BREAKPOINT
+	bool
+	default y
+	help
+	  This adds an event trigger which will set the wprobe on a specific
+	  field of an event. This allows user to trace the memory access of
+	  an address pointed by the event field.
+
 config BPF_EVENTS
 	depends on BPF_SYSCALL
 	depends on (KPROBE_EVENTS || UPROBE_EVENTS) && PERF_EVENTS
kernel/trace/trace.h +1 -0
index 2f07c5c4ffc8..b4f964839350 100644
@@ -1982,6 +1982,7 @@ trigger_data_alloc(struct event_command *cmd_ops, char *cmd, char *param,
 		   void *private_data);
 extern void trigger_data_free(struct event_trigger_data *data);
 extern int event_trigger_init(struct event_trigger_data *data);
+extern void event_trigger_free(struct event_trigger_data *data);
 extern int trace_event_trigger_enable_disable(struct trace_event_file *file,
 					      int trigger_enable);
 extern void update_cond_flag(struct trace_event_file *file);
kernel/trace/trace_wprobe.c +437 -0

479 lines not on this page · raw message

#13 lore raw

Re: [PATCH v8 8/9] tracing: wprobe: Add wprobe event trigger

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: - [High] Uncancelled asynchronous work causes NULL pointer dereference when disabling wprobes. - [High]…

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Uncancelled asynchronous work causes NULL pointer dereference when disabling wprobes.
- [High] State corruption in wprobe_trigger_cmd_parse when registering clear_wprobe.
- [High] Use-After-Free in wprobe triggers due to incorrect RCU grace period handling.
- [Medium] Memory leak of trigger_data->filter on error path in wprobe_trigger_cmd_parse.
- [Low] Build failure due to static declaration of event_trigger_free.
--

--- Patch [8]: [PATCH v8 8/9] tracing: wprobe: Add wprobe event trigger --- commit aadb441dbb3ae8cb3459288e9388e6fa92216b82

Author: Masami Hiramatsu (Google) <[email protected]>
Date:   Thu Jul 16 02:53:46 2026 +0000

tracing: wprobe: Add wprobe event trigger

Add wprobe event trigger to set and clear the watch event dynamically. This allows us to set an watchpoint on a given local variables and a slab object instead of static objects.

kernel/trace/trace.h trigger_data_alloc(struct event_command *cmd_ops, char *cmd, char *param, · line 1985

Quoted 9 lines
kernel/trace/trace.h +1 -0
index 2f07c5c4ffc87..b4f964839350f 100644
@@ -1982,6 +1982,7 @@ trigger_data_alloc(struct event_command *cmd_ops, char *cmd, char *param,
 		   void *private_data);
 extern void trigger_data_free(struct event_trigger_data *data);
 extern int event_trigger_init(struct event_trigger_data *data);
+extern void event_trigger_free(struct event_trigger_data *data);

[Severity: Low]
Does this code build successfully when CONFIG_WPROBE_TRIGGERS is enabled?

The patch declares event_trigger_free() as extern here and calls it from trace_wprobe.c, but its definition in kernel/trace/trace_events_trigger.c remains static.

Quoted 7 lines
kernel/trace/trace.h +0 -0
 extern int trace_event_trigger_enable_disable(struct trace_event_file *file,
 					      int trigger_enable);
 extern void update_cond_flag(struct trace_event_file *file);
kernel/trace/trace_wprobe.c +0 -0
index 942ad2810d369..69ad48ac210c6 100644

[ ... ]

kernel/trace/trace_wprobe.c static void __unregister_trace_wprobe(struct trace_wprobe *tw) · line 213

Quoted 7 lines
kernel/trace/trace_wprobe.c +3 -0
@@ -197,9 +208,42 @@ static void __unregister_trace_wprobe(struct trace_wprobe *tw)
 	}
 }
 
+static int trace_wprobe_update_local(struct trace_wprobe *tw, unsigned long addr)
+{
+	struct perf_event *bp = *this_cpu_ptr(tw->bp_event);

[Severity: High]
If a wprobe trigger has recently fired, tw->irq_work and tw->work may be queued. If a user disables the wprobe, __unregister_trace_wprobe() clears tw->bp_event to NULL.

When tw->work runs concurrently and calls trace_wprobe_update_local(), will this_cpu_ptr(tw->bp_event) calculate a garbage per-CPU offset from NULL and cause a panic when dereferencing bp?

Should the pending work be cancelled when the wprobe is unregistered?

Quoted 5 lines
kernel/trace/trace_wprobe.c +5 -0
+	struct perf_event_attr attr = bp->attr;
+
+	attr.bp_addr = addr;
+	return modify_wide_hw_breakpoint_local(bp, &attr);
+}

[ ... ]

Quoted 6 lines
kernel/trace/trace_wprobe.c +6 -0
+static void wprobe_trigger_free(struct event_trigger_data *data)
+{
+	struct wprobe_trigger_data *wprobe_data = data->private_data;
+
+	if (WARN_ON_ONCE(data->ref <= 0))
+		return;

kernel/trace/trace_wprobe.c static __init int init_wprobe_trace(void) · line 891

Quoted 10 lines
kernel/trace/trace_wprobe.c +10 -0
+
+	data->ref--;
+	if (!data->ref) {
+		/* Remove the SOFT_MODE flag */
+		trace_event_enable_disable(wprobe_data->file, 0, 1);
+		trace_event_put_ref(wprobe_data->file->event_call);
+		trigger_data_free(data);
+
+		/* Wait for RCU readers (tracepoint executions) to finish */
+		call_rcu(&wprobe_data->rcu, wprobe_trigger_data_rcu_free);

[Severity: High]
Since triggers can be attached to faultable syscall tracepoints (which execute in a sleepable RCU Tasks Trace context), is call_rcu() sufficient here?

If a sleeping tracepoint reader is iterating triggers when this unregistration occurs, the normal RCU grace period will expire and wprobe_data will be freed. When the tracepoint wakes up and executes wprobe_trigger(), won't it dereference the freed wprobe_data?

Should this use tracepoint_synchronize_unregister() to safely wait for all tracepoint contexts?

Quoted 8 lines
kernel/trace/trace_wprobe.c +8 -0
+	}
+}
+
+static int wprobe_trigger_cmd_parse(struct event_command *cmd_ops,
+				    struct trace_event_file *file,
+				    char *glob, char *cmd,
+				    char *param_and_filter)
+{

[ ... ]

kernel/trace/trace_wprobe.c static __init int init_wprobe_trace(void) · line 1003

Quoted 7 lines
kernel/trace/trace_wprobe.c +7 -0
+	ret = event_trigger_set_filter(cmd_ops, file, filter, trigger_data);
+	if (ret < 0)
+		return ret;
+
+	/* Soft-enable (register) wprobe event on WPROBE_DEFAULT_CLEAR_ADDRESS */
+	if (!trace_event_try_get_ref(wprobe_file->event_call))
+		return -ENODEV;

[Severity: Medium]
If trace_event_try_get_ref() fails, this returns -ENODEV directly.

Since event_trigger_reset_filter() is bypassed on this error path, does this leak the dynamically allocated filter memory?

kernel/trace/trace_wprobe.c static __init int init_wprobe_trace(void) · line 1005

Quoted 2 lines
kernel/trace/trace_wprobe.c +2 -0
+
+	WRITE_ONCE(tw->addr, WPROBE_DEFAULT_CLEAR_ADDRESS);

[Severity: High]
Registering a clear_wprobe trigger on an already-active wprobe is permitted.

However, this unconditionally overwrites tw->addr with WPROBE_DEFAULT_CLEAR_ADDRESS. When the clear_wprobe trigger fires, it checks if tw->addr != WPROBE_DEFAULT_CLEAR_ADDRESS.

Since it has already been set to WPROBE_DEFAULT_CLEAR_ADDRESS here, will this prevent the clear trigger from actually clearing the hardware breakpoints, thereby corrupting the wprobe state and permanently leaking hardware breakpoints?

kernel/trace/trace_wprobe.c static __init int init_wprobe_trace(void) · line 1017

Quoted 12 lines
kernel/trace/trace_wprobe.c +12 -0
+	ret = trace_event_enable_disable(wprobe_file, 1, 1);
+	if (ret < 0) {
+		trace_event_put_ref(wprobe_file->event_call);
+		event_trigger_reset_filter(cmd_ops, trigger_data);
+		return ret;
+	}
+	ret = event_trigger_register(cmd_ops, file, glob, trigger_data);
+	if (ret) {
+		event_trigger_reset_filter(cmd_ops, trigger_data);
+		trace_event_enable_disable(wprobe_file, 0, 1);
+		trace_event_put_ref(wprobe_file->event_call);
+		synchronize_rcu();

[Severity: High]
Similar to call_rcu() above, is synchronize_rcu() sufficient here for tracepoints that rely on RCU Tasks Trace?

Quoted 2 lines
kernel/trace/trace_wprobe.c +2 -0
+		return ret;
+	}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/178417033089.209165.16717079876036408877.stgit@devnote2?part=8
author maintainer
#14 lore raw

PATCH v8 9/9 selftests: ftrace: Add wprobe trigger testcase

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Add a testcase for checking wprobe trigger. This sets set_wprobe and clear_wprobe triggers on fprobe events to watch dentry access. So this depends on both wprobe and fprobe.

---
Changes in v8:
- Use TMPDIR for making a test file.
- Ensure the fprobe and target functions exists, if not, exit

    with UNRESOLVED (== skip).
 Changes in v7:
  - Add a newline at the end of file.(style fix)
  - Use dentry_kill instead of __dentry_kill.
 Changes in v5:
  - Enable CONFIG_WPROBE_TRIGGERS in the config for ftrace test.
 Changes in v3:
  - Newly added.
---
 tools/testing/selftests/ftrace/config              |    1 
 .../ftrace/test.d/trigger/trigger-wprobe.tc        |   65 ++++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-wprobe.tc
tools/testing/selftests/ftrace/config +1 -0
index d2f503722020..ecdee77f360f 100644
@@ -28,3 +28,4 @@ CONFIG_TRACER_SNAPSHOT=y
 CONFIG_UPROBES=y
 CONFIG_UPROBE_EVENTS=y
 CONFIG_WPROBE_EVENTS=y
+CONFIG_WPROBE_TRIGGERS=y
tools/testing/selftests/ftrace/test.d/trigger/trigger-wprobe.tc new file +65 -0

65 lines not on this page · raw message

#15 lore raw

Re: [PATCH v8 9/9] selftests: ftrace: Add wprobe trigger testcase

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The selftest incorrectly assumes that unlinking a file (`rm`) will synchronously trigger…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The selftest incorrectly assumes that unlinking a file (rm) will synchronously trigger dentry_kill(). This only works on filesystems that do not cache negative dentries (like tmpfs). On standard disk filesystems (like ext4), the test will fail. --

--- Patch [9]: [PATCH v8 9/9] selftests: ftrace: Add wprobe trigger testcase --- commit 02c2b6c1500ba6cce360f9d5447d679e62801d61
Author: Masami Hiramatsu (Google) <[email protected]>

selftests: ftrace: Add wprobe trigger testcase

This commit adds a testcase for checking wprobe triggers. It sets set_wprobe and clear_wprobe triggers on fprobe events to watch dentry access.

Quoted 5 lines
tools/testing/selftests/ftrace/test.d/trigger/trigger-wprobe.tc new file +0 -0
new file mode 100644
index 0000000000000..5ffe43f886d1a

[ ... ]

tools/testing/selftests/ftrace/test.d/trigger/trigger-wprobe.tc line 57

Quoted 13 lines

+:;: "Start test workload" ;:

tools/testing/selftests/ftrace/test.d/trigger/trigger-wprobe.tc new file +12 -0
+echo 1 >> tracing_on
+
+echo aaa > $TMPDIR/hoge
+echo bbb > $TMPDIR/hoge
+echo ccc > $TMPDIR/hoge
+rm $TMPDIR/hoge
+
+:;: "Check trace results" ;:
+cat trace | grep watch
+
+:;: "Ensure wprobe becomes disabled again" ;:
+cat events/wprobes/watch/enable | grep 0

[Severity: Medium]
Will this regression cause the test to fail when $TMPDIR is on a standard disk filesystem?

This selftest appears to rely on rm synchronously triggering dentry_kill() to fire the clear_wprobe trigger.

While this works on memory-backed filesystems like tmpfs that do not cache negative dentries (their d_delete operation immediately leads to dentry_kill()), standard disk filesystems like ext4, xfs, or btrfs cache negative dentries to optimize subsequent lookups.

On those filesystems, unlinking the file leaves the dentry hashed on the dcache LRU list, deferring dentry_kill() until memory pressure or a manual eviction occurs. This would cause the test to fail when checking if the wprobe is disabled.

Does this need to trigger a manual cache eviction or ensure the test runs on a tmpfs mount to avoid failing?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/178417033089.209165.16717079876036408877.stgit@devnote2?part=9