* [RFC PATCH 01/13] arch: add HAVE_REINSTALL_HW_BREAKPOINT
2026-07-14 18:22 [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption Jinchao Wang
@ 2026-07-14 18:22 ` Jinchao Wang
2026-07-14 18:22 ` [RFC PATCH 02/13] x86/hw_breakpoint: Unify breakpoint install/uninstall Jinchao Wang
` (11 subsequent siblings)
12 siblings, 0 replies; 26+ messages in thread
From: Jinchao Wang @ 2026-07-14 18:22 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
Some architectures can update the address, length or type of an
installed hardware breakpoint in place, without releasing and
re-reserving its slot. Add an opt-in Kconfig symbol so generic code
can offer such an operation on architectures that implement
arch_reinstall_hw_breakpoint().
This is a prerequisite for KWatch, which re-points preallocated
per-CPU breakpoints from atomic context, where the register/release
path (which may sleep and rebalances slot constraints) cannot be
used.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
arch/Kconfig | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig
index fa7507ac8e13..41b3784e0ddd 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -457,6 +457,16 @@ config HAVE_MIXED_BREAKPOINTS_REGS
Select this option if your arch implements breakpoints under the
latter fashion.
+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
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [RFC PATCH 02/13] x86/hw_breakpoint: Unify breakpoint install/uninstall
2026-07-14 18:22 [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption Jinchao Wang
2026-07-14 18:22 ` [RFC PATCH 01/13] arch: add HAVE_REINSTALL_HW_BREAKPOINT Jinchao Wang
@ 2026-07-14 18:22 ` Jinchao Wang
2026-07-14 18:47 ` sashiko-bot
2026-07-14 18:22 ` [RFC PATCH 03/13] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint Jinchao Wang
` (10 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Jinchao Wang @ 2026-07-14 18:22 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
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.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
arch/x86/include/asm/hw_breakpoint.h | 6 ++
arch/x86/kernel/hw_breakpoint.c | 141 +++++++++++++++------------
2 files changed, 84 insertions(+), 63 deletions(-)
diff --git a/arch/x86/include/asm/hw_breakpoint.h b/arch/x86/include/asm/hw_breakpoint.h
index 0bc931cd0698..aa6adac6c3a2 100644
--- a/arch/x86/include/asm/hw_breakpoint.h
+++ b/arch/x86/include/asm/hw_breakpoint.h
@@ -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>
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index f846c15f21ca..877509539300 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -49,7 +49,6 @@ static DEFINE_PER_CPU(unsigned long, cpu_debugreg[HBP_NUM]);
*/
static DEFINE_PER_CPU(struct perf_event *, bp_per_reg[HBP_NUM]);
-
static inline unsigned long
__encode_dr7(int drnum, unsigned int len, unsigned int type)
{
@@ -86,96 +85,112 @@ int decode_dr7(unsigned long dr7, int bpnum, unsigned *len, unsigned *type)
}
/*
- * Install a perf counter breakpoint.
- *
- * We seek a free debug address register and use it for this
- * breakpoint. Eventually we enable it in the debug control register.
- *
- * Atomic: we hold the counter->ctx->lock and we only handle variables
- * and registers local to this cpu.
+ * We seek a slot and change it or keep it based on the action.
+ * Returns slot number on success, negative error on failure.
+ * Must be called with IRQs disabled.
*/
-int arch_install_hw_breakpoint(struct perf_event *bp)
+static int manage_bp_slot(struct perf_event *bp, enum bp_slot_action action)
{
- struct arch_hw_breakpoint *info = counter_arch_bp(bp);
- unsigned long *dr7;
- int i;
-
- lockdep_assert_irqs_disabled();
+ struct perf_event *old_bp;
+ struct perf_event *new_bp;
+ int slot;
+
+ switch (action) {
+ case BP_SLOT_ACTION_INSTALL:
+ old_bp = NULL;
+ new_bp = bp;
+ break;
+ case BP_SLOT_ACTION_UNINSTALL:
+ old_bp = bp;
+ new_bp = NULL;
+ break;
+ default:
+ return -EINVAL;
+ }
- for (i = 0; i < HBP_NUM; i++) {
- struct perf_event **slot = this_cpu_ptr(&bp_per_reg[i]);
+ for (slot = 0; slot < HBP_NUM; slot++) {
+ struct perf_event **curr = this_cpu_ptr(&bp_per_reg[slot]);
- if (!*slot) {
- *slot = bp;
- break;
+ if (*curr == old_bp) {
+ *curr = new_bp;
+ return slot;
}
}
- if (WARN_ONCE(i == HBP_NUM, "Can't find any breakpoint slot"))
- return -EBUSY;
+ if (old_bp) {
+ WARN_ONCE(1, "Can't find matching breakpoint slot");
+ return -EINVAL;
+ }
+
+ WARN_ONCE(1, "No free breakpoint slots");
+ return -EBUSY;
+}
+
+static void setup_hwbp(struct arch_hw_breakpoint *info, int slot, bool enable)
+{
+ unsigned long dr7;
- set_debugreg(info->address, i);
- __this_cpu_write(cpu_debugreg[i], info->address);
+ set_debugreg(info->address, slot);
+ __this_cpu_write(cpu_debugreg[slot], info->address);
- dr7 = this_cpu_ptr(&cpu_dr7);
- *dr7 |= encode_dr7(i, info->len, info->type);
+ dr7 = this_cpu_read(cpu_dr7);
+ if (enable)
+ dr7 |= encode_dr7(slot, info->len, info->type);
+ else
+ dr7 &= ~__encode_dr7(slot, info->len, info->type);
/*
- * Ensure we first write cpu_dr7 before we set the DR7 register.
- * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
+ * Enabling:
+ * Ensure we first write cpu_dr7 before we set the DR7 register.
+ * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
*/
+ if (enable)
+ this_cpu_write(cpu_dr7, dr7);
+
barrier();
- set_debugreg(*dr7, 7);
+ set_debugreg(dr7, 7);
+
if (info->mask)
- amd_set_dr_addr_mask(info->mask, i);
+ amd_set_dr_addr_mask(enable ? info->mask : 0, slot);
- return 0;
+ /*
+ * Disabling:
+ * Ensure the write to cpu_dr7 is after we've set the DR7 register.
+ * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
+ */
+ if (!enable)
+ this_cpu_write(cpu_dr7, dr7);
}
/*
- * Uninstall the breakpoint contained in the given counter.
- *
- * First we search the debug address register it uses and then we disable
- * it.
- *
- * Atomic: we hold the counter->ctx->lock and we only handle variables
- * and registers local to this cpu.
+ * find suitable breakpoint slot and set it up based on the action
*/
-void arch_uninstall_hw_breakpoint(struct perf_event *bp)
+static int arch_manage_bp(struct perf_event *bp, enum bp_slot_action action)
{
- struct arch_hw_breakpoint *info = counter_arch_bp(bp);
- unsigned long dr7;
- int i;
+ struct arch_hw_breakpoint *info;
+ int slot;
lockdep_assert_irqs_disabled();
- for (i = 0; i < HBP_NUM; i++) {
- struct perf_event **slot = this_cpu_ptr(&bp_per_reg[i]);
-
- if (*slot == bp) {
- *slot = NULL;
- break;
- }
- }
-
- if (WARN_ONCE(i == HBP_NUM, "Can't find any breakpoint slot"))
- return;
+ slot = manage_bp_slot(bp, action);
+ if (slot < 0)
+ return slot;
- dr7 = this_cpu_read(cpu_dr7);
- dr7 &= ~__encode_dr7(i, info->len, info->type);
+ info = counter_arch_bp(bp);
+ setup_hwbp(info, slot, action != BP_SLOT_ACTION_UNINSTALL);
- set_debugreg(dr7, 7);
- if (info->mask)
- amd_set_dr_addr_mask(0, i);
+ return 0;
+}
- /*
- * Ensure the write to cpu_dr7 is after we've set the DR7 register.
- * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
- */
- barrier();
+int arch_install_hw_breakpoint(struct perf_event *bp)
+{
+ return arch_manage_bp(bp, BP_SLOT_ACTION_INSTALL);
+}
- this_cpu_write(cpu_dr7, dr7);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp)
+{
+ arch_manage_bp(bp, BP_SLOT_ACTION_UNINSTALL);
}
static int arch_bp_generic_len(int x86_len)
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 02/13] x86/hw_breakpoint: Unify breakpoint install/uninstall
2026-07-14 18:22 ` [RFC PATCH 02/13] x86/hw_breakpoint: Unify breakpoint install/uninstall Jinchao Wang
@ 2026-07-14 18:47 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-07-14 18:47 UTC (permalink / raw)
To: Jinchao Wang; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing compiler barrier during hardware breakpoint uninstall reintroduces an NMI race condition.
- [Medium] Redundant hardware debug address register write during breakpoint uninstall degrades performance.
--
--- Patch [2]: [RFC PATCH 02/13] x86/hw_breakpoint: Unify breakpoint install/uninstall ---
commit 856f1186c52b7f5043f0e5e6063055a66d542dc1
Author: Jinchao Wang <wangjinchao600@gmail.com>
x86/hw_breakpoint: Unify breakpoint install/uninstall
Consolidate breakpoint management to reduce code duplication and shrink
stripped code size. This refactoring paves the way for reinstalling
breakpoints and adds a missing types dependency.
> diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
> --- a/arch/x86/kernel/hw_breakpoint.c
> +++ b/arch/x86/kernel/hw_breakpoint.c
[ ... ]
> +static void setup_hwbp(struct arch_hw_breakpoint *info, int slot, bool enable)
> +{
> + unsigned long dr7;
> +
> + set_debugreg(info->address, slot);
> + __this_cpu_write(cpu_debugreg[slot], info->address);
[Severity: Medium]
Does this code unconditionally write the hardware debug address register
even on the uninstall path?
Writing to DR0-DR3 is typically a slow serializing operation, which can
also trigger expensive VMExits in virtualized environments. Since the
breakpoint is being disabled via DR7 anyway when enable is false, is
this address register write necessary here? Could this introduce
unnecessary overhead on the hot path during perf event scheduling?
> +
> + dr7 = this_cpu_read(cpu_dr7);
> + if (enable)
> + dr7 |= encode_dr7(slot, info->len, info->type);
> + else
> + dr7 &= ~__encode_dr7(slot, info->len, info->type);
> +
> + /*
> + * Enabling:
> + * Ensure we first write cpu_dr7 before we set the DR7 register.
> + * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
> + */
> + if (enable)
> + this_cpu_write(cpu_dr7, dr7);
> +
> + barrier();
> +
> + set_debugreg(dr7, 7);
> +
> + if (info->mask)
> + amd_set_dr_addr_mask(enable ? info->mask : 0, slot);
> +
> + /*
> + * Disabling:
> + * Ensure the write to cpu_dr7 is after we've set the DR7 register.
> + * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
> + */
> + if (!enable)
> + this_cpu_write(cpu_dr7, dr7);
[Severity: Medium]
Could the compiler reorder this write to cpu_dr7 before the DR7 register
is updated on the disable path?
The original uninstall code included an explicit compiler barrier before
updating cpu_dr7, but it appears to be missing here. If the compiler
reorders this memory access, could an NMI hit in the middle, observe a
cleared cpu_dr7, and incorrectly assume no breakpoints are active?
If the NMI handler then touches watched memory, could this lead to a fatal
nested hardware breakpoint exception?
> +}
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714182243.10687-1-wangjinchao600@gmail.com?part=2
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 03/13] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint
2026-07-14 18:22 [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption Jinchao Wang
2026-07-14 18:22 ` [RFC PATCH 01/13] arch: add HAVE_REINSTALL_HW_BREAKPOINT Jinchao Wang
2026-07-14 18:22 ` [RFC PATCH 02/13] x86/hw_breakpoint: Unify breakpoint install/uninstall Jinchao Wang
@ 2026-07-14 18:22 ` Jinchao Wang
2026-07-14 19:22 ` sashiko-bot
2026-07-14 18:30 ` [RFC PATCH 04/13] HWBP: Add modify_wide_hw_breakpoint_local() API Jinchao Wang
` (9 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Jinchao Wang @ 2026-07-14 18:22 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
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,
and x86 advertises the capability via HAVE_REINSTALL_HW_BREAKPOINT.
Since a REINSTALL may change bp_len, setup_hwbp() must clear the
slot's stale len/type and enable bits in DR7 before re-encoding:
OR-merging the new encoding over the old one would keep the CPU
watching with the stale width (verified in QEMU by reading DR7 after
re-arming watch_len=1 over a len8 breakpoint: 0x999906aa merged
without the clearing, 0x199906aa with it).
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
arch/x86/Kconfig | 1 +
arch/x86/include/asm/hw_breakpoint.h | 2 ++
arch/x86/kernel/hw_breakpoint.c | 16 ++++++++++++++--
3 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index bdad90f210e4..5be698db0241 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -246,6 +246,7 @@ config X86
select HAVE_FUNCTION_TRACER
select HAVE_GCC_PLUGINS
select HAVE_HW_BREAKPOINT
+ select HAVE_REINSTALL_HW_BREAKPOINT
select HAVE_IOREMAP_PROT
select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
select HAVE_IRQ_TIME_ACCOUNTING
diff --git a/arch/x86/include/asm/hw_breakpoint.h b/arch/x86/include/asm/hw_breakpoint.h
index aa6adac6c3a2..c22cc4e87fc5 100644
--- a/arch/x86/include/asm/hw_breakpoint.h
+++ b/arch/x86/include/asm/hw_breakpoint.h
@@ -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);
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 877509539300..4221dbb899f9 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -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;
@@ -134,10 +138,13 @@ static void setup_hwbp(struct arch_hw_breakpoint *info, int slot, bool enable)
__this_cpu_write(cpu_debugreg[slot], info->address);
dr7 = this_cpu_read(cpu_dr7);
+ /*
+ * Clear the slot's stale len/type and enable bits first: a REINSTALL
+ * with a different bp_len would otherwise OR-merge both encodings.
+ */
+ dr7 &= ~__encode_dr7(slot, 0xf, 0);
if (enable)
dr7 |= encode_dr7(slot, info->len, info->type);
- else
- dr7 &= ~__encode_dr7(slot, info->len, info->type);
/*
* Enabling:
@@ -188,6 +195,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);
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 03/13] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint
2026-07-14 18:22 ` [RFC PATCH 03/13] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint Jinchao Wang
@ 2026-07-14 19:22 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-07-14 19:22 UTC (permalink / raw)
To: Jinchao Wang; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] In-place hardware breakpoint reinstallation via `arch_reinstall_hw_breakpoint()` fails to clear the AMD hardware breakpoint address mask when transitioning from a range breakpoint to an exact address breakpoint.
--
--- Patch [3]: [RFC PATCH 03/13] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint ---
commit 59c50868129e1541bda319c3957c2b199256f005
Author: Jinchao Wang <wangjinchao600@gmail.com>
x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint
This commit introduces arch_reinstall_hw_breakpoint to allow efficient
re-establishment of existing breakpoints in atomic contexts. It also modifies
setup_hwbp to clear the slot's stale length/type and enable bits in DR7
before re-encoding to avoid OR-merging with the old encoding.
> diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
> index 877509539300c..4221dbb899f90 100644
> --- a/arch/x86/kernel/hw_breakpoint.c
> +++ b/arch/x86/kernel/hw_breakpoint.c
[ ... ]
> @@ -188,6 +195,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: Medium]
When arch_reinstall_hw_breakpoint is called to transition an active
breakpoint from an AMD range breakpoint to an exact address breakpoint,
does it fail to clear the old hardware breakpoint address mask?
If the caller updates the breakpoint configuration to an exact address
where the new mask is 0 and reinstalls it, the flow bypasses the uninstall
phase and calls setup_hwbp with enable set to true:
arch_manage_bp()->setup_hwbp()
if (info->mask)
amd_set_dr_addr_mask(enable ? info->mask : 0, slot);
Since info->mask evaluates to 0 for the new exact address breakpoint,
this condition skips calling amd_set_dr_addr_mask. This leaves the old
non-zero mask in the CPU MSR, which could cause the CPU to continue
monitoring the old, wider address range and potentially trigger spurious
hardware traps on memory accesses.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714182243.10687-1-wangjinchao600@gmail.com?part=3
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 04/13] HWBP: Add modify_wide_hw_breakpoint_local() API
2026-07-14 18:22 [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption Jinchao Wang
` (2 preceding siblings ...)
2026-07-14 18:22 ` [RFC PATCH 03/13] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint Jinchao Wang
@ 2026-07-14 18:30 ` Jinchao Wang
2026-07-14 19:41 ` sashiko-bot
2026-07-14 18:31 ` [RFC PATCH 05/13] mm/kwatch: add watch expression parser and dereference engine Jinchao Wang
` (8 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Jinchao Wang @ 2026-07-14 18:30 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
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.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
include/linux/hw_breakpoint.h | 6 ++++++
kernel/events/hw_breakpoint.c | 37 +++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+)
diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index db199d653dd1..ea373f2587f8 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -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 -ENOSYS; }
+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
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 789add0c185a..20ca64f30508 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -888,6 +888,43 @@ void unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events)
}
EXPORT_SYMBOL_GPL(unregister_wide_hw_breakpoint);
+/**
+ * modify_wide_hw_breakpoint_local - update breakpoint config for local CPU
+ * @bp: the hwbp perf event for this CPU
+ * @attr: the new attribute for @bp
+ *
+ * This does not release and reserve the slot of a HWBP; it just reuses the
+ * current slot on local CPU. So the users must update the other CPUs by
+ * themselves.
+ * Also, since this does not release/reserve the slot, this can not change the
+ * type to incompatible type of the HWBP.
+ * Return err if attr is invalid or the CPU fails to update debug register
+ * for new @attr.
+ */
+#ifdef CONFIG_HAVE_REINSTALL_HW_BREAKPOINT
+int modify_wide_hw_breakpoint_local(struct perf_event *bp,
+ struct perf_event_attr *attr)
+{
+ int ret;
+
+ if (find_slot_idx(bp->attr.bp_type) != find_slot_idx(attr->bp_type))
+ return -EINVAL;
+
+ ret = hw_breakpoint_arch_parse(bp, attr, counter_arch_bp(bp));
+ if (ret)
+ return ret;
+
+ return arch_reinstall_hw_breakpoint(bp);
+}
+#else
+int modify_wide_hw_breakpoint_local(struct perf_event *bp,
+ struct perf_event_attr *attr)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+EXPORT_SYMBOL_GPL(modify_wide_hw_breakpoint_local);
+
/**
* hw_breakpoint_is_used - check if breakpoints are currently used
*
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 04/13] HWBP: Add modify_wide_hw_breakpoint_local() API
2026-07-14 18:30 ` [RFC PATCH 04/13] HWBP: Add modify_wide_hw_breakpoint_local() API Jinchao Wang
@ 2026-07-14 19:41 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-07-14 19:41 UTC (permalink / raw)
To: Jinchao Wang; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] In-place mutation of active hardware breakpoint state (`bp->hw.info`) without disabling the event causes NMI data races and error-path corruption.
- [High] The DRn debug address register is updated while the hardware breakpoint is still enabled in DR7, risking spurious debug exceptions.
- [Medium] `modify_wide_hw_breakpoint_local` fails to update `bp->attr` on successful modification, causing state desynchronization.
--
--- Patch [4]: [RFC PATCH 04/13] HWBP: Add modify_wide_hw_breakpoint_local() API ---
commit 861178eefb9a8774dffa3900f118da48cde078a9
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>
HWBP: Add modify_wide_hw_breakpoint_local() API
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.
[ ... ]
> diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
> index 789add0c185a0..20ca64f305083 100644
> --- a/kernel/events/hw_breakpoint.c
> +++ b/kernel/events/hw_breakpoint.c
> @@ -888,6 +888,43 @@ void unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events)
[ ... ]
> +int modify_wide_hw_breakpoint_local(struct perf_event *bp,
> + struct perf_event_attr *attr)
> +{
> + int ret;
> +
> + if (find_slot_idx(bp->attr.bp_type) != find_slot_idx(attr->bp_type))
> + return -EINVAL;
> +
> + ret = hw_breakpoint_arch_parse(bp, attr, counter_arch_bp(bp));
[Severity: High]
Does this code call hw_breakpoint_arch_parse() on the active event's hardware
info without temporarily disabling the event?
Architecture implementations like x86's arch_build_bp_info() mutate the struct
incrementally:
arch/x86/kernel/hw_breakpoint.c:arch_build_bp_info() {
hw->address = attr->bp_addr;
hw->mask = 0;
/* Type */
switch (attr->bp_type) {
...
}
If validation fails midway, does bp->hw.info remain partially overwritten and
corrupted?
Additionally, could an NMI hitting the active breakpoint during this window
read torn state?
> + if (ret)
> + return ret;
> +
> + return arch_reinstall_hw_breakpoint(bp);
[Severity: High]
Does arch_reinstall_hw_breakpoint() risk spurious debug exceptions here?
Because modify_wide_hw_breakpoint_local() does not temporarily disable the
breakpoint first, the slot is already enabled in the physical DR7.
Modifying DRn before the control register DR7 is cleared or updated can
violate architectural rules:
arch/x86/kernel/hw_breakpoint.c:setup_hwbp() {
set_debugreg(info->address, slot);
__this_cpu_write(cpu_debugreg[slot], info->address);
dr7 = this_cpu_read(cpu_dr7);
/*
* Clear the slot's stale len/type and enable bits first
...
}
Could this trigger spurious #DB debug exceptions if the CPU accesses the new
address during the window before DR7 is updated?
[Severity: Medium]
When this modification is successful, should bp->attr be updated with the new
user-provided attributes?
It looks like the newly parsed attr is never copied back to bp->attr.
Any subsequent read of the breakpoint properties or perf sample
initializations might silently use the old, stale address and length:
kernel/events/core.c:perf_bp_event() {
perf_sample_data_init(&sample, bp->attr.bp_addr, 0);
...
}
Will this cause state desynchronization and silently corrupt trace data?
> +}
> +#else
> +int modify_wide_hw_breakpoint_local(struct perf_event *bp,
> + struct perf_event_attr *attr)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714182243.10687-1-wangjinchao600@gmail.com?part=4
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 05/13] mm/kwatch: add watch expression parser and dereference engine
2026-07-14 18:22 [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption Jinchao Wang
` (3 preceding siblings ...)
2026-07-14 18:30 ` [RFC PATCH 04/13] HWBP: Add modify_wide_hw_breakpoint_local() API Jinchao Wang
@ 2026-07-14 18:31 ` Jinchao Wang
2026-07-14 18:45 ` sashiko-bot
2026-07-14 18:31 ` [RFC PATCH 06/13] mm/kwatch: add lockless per-task context pool Jinchao Wang
` (7 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Jinchao Wang @ 2026-07-14 18:31 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
KWatch watches a memory address that is only known once the target
function runs, e.g. "argument 1, plus 8, dereferenced once". Add the
two halves of that mechanism:
- kwatch_deref_parse() turns a textual watch expression
{base}[+-off][->[+-]off]... into a kwatch_config: a base anchor
(arg1..arg6, stack, an absolute address or - for built-in KWatch -
a symbol name) plus a static offset chain.
- kwatch_deref_resolve() replays the chain at probe time against
pt_regs. Every pointer load goes through get_kernel_nofault() and
the final address must be a kernel address.
Also add the internal kwatch.h header shared by the rest of the
series. Nothing is built yet; the Kconfig entry comes with the
control plane.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
mm/kwatch/Makefile | 3 +
mm/kwatch/deref.c | 174 +++++++++++++++++++++++++++++++++++++++++++++
mm/kwatch/kwatch.h | 107 ++++++++++++++++++++++++++++
3 files changed, 284 insertions(+)
create mode 100644 mm/kwatch/Makefile
create mode 100644 mm/kwatch/deref.c
create mode 100644 mm/kwatch/kwatch.h
diff --git a/mm/kwatch/Makefile b/mm/kwatch/Makefile
new file mode 100644
index 000000000000..69c21ae62123
--- /dev/null
+++ b/mm/kwatch/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_KWATCH) += kwatch.o
+
+kwatch-y := deref.o
diff --git a/mm/kwatch/deref.c b/mm/kwatch/deref.c
new file mode 100644
index 000000000000..a93c76139e7c
--- /dev/null
+++ b/mm/kwatch/deref.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/ptrace.h>
+#include <linux/sched.h>
+#include <linux/uaccess.h>
+#include <linux/kallsyms.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+
+#include "kwatch.h"
+
+int kwatch_deref_resolve(const struct kwatch_config *cfg, struct pt_regs *regs,
+ unsigned long *out_addr, u16 *out_len)
+{
+ unsigned long addr = 0;
+ int i;
+
+ /* 1. Resolve the Base Anchor */
+ if (cfg->base == KWATCH_BASE_STACK) {
+ addr = kernel_stack_pointer(regs);
+ if (unlikely(!addr))
+ return -EINVAL;
+ } else if (cfg->base >= KWATCH_BASE_ARG1 &&
+ cfg->base <= KWATCH_BASE_ARG6) {
+ int arg_idx = cfg->base - KWATCH_BASE_ARG1;
+
+ addr = regs_get_kernel_argument(regs, arg_idx);
+ } else if (cfg->base == KWATCH_BASE_ABS_ADDR ||
+ cfg->base == KWATCH_BASE_GLOBAL_SYM) {
+ /* Zero-latency load of the static symbol location */
+ addr = cfg->sym_addr;
+ } else {
+ return -EINVAL;
+ }
+
+ /* 2. The Pointer-Chasing FSM */
+ for (i = 0; i < cfg->offset_count; i++) {
+ addr += cfg->offsets[i];
+
+ if (i < cfg->offset_count - 1) {
+ unsigned long next_addr;
+
+ /* Dynamically read the pointer contents at runtime */
+ if (get_kernel_nofault(next_addr, (unsigned long *)addr))
+ return -EFAULT;
+
+ addr = next_addr;
+ }
+ }
+
+ /* Enforce strict Kernel-Space boundary */
+ if (unlikely(addr < TASK_SIZE_MAX))
+ return -EINVAL;
+
+ *out_addr = addr;
+ *out_len = cfg->watch_len;
+ return 0;
+}
+
+int kwatch_deref_parse(struct kwatch_config *cfg, const char *watch_expr)
+{
+ char *p, *sep, *dup_expr;
+ char type = '\0';
+ bool is_deref = false;
+ int ret = 0;
+
+ dup_expr = kstrdup(watch_expr, GFP_KERNEL);
+ if (!dup_expr)
+ return -ENOMEM;
+
+ cfg->offset_count = 1;
+ cfg->offsets[0] = 0;
+
+ /* 1. Isolate and Resolve Base Anchor */
+ p = dup_expr;
+ sep = NULL;
+ while (*p) {
+ if (*p == '+') {
+ sep = p;
+ type = '+';
+ break;
+ }
+ if (*p == '-') {
+ sep = p;
+ type = '-';
+ if (p[1] == '>')
+ is_deref = true;
+ break;
+ }
+ p++;
+ }
+
+ if (type)
+ *sep = '\0';
+
+ if (!strcmp(dup_expr, "stack")) {
+ cfg->base = KWATCH_BASE_STACK;
+ } else if (!strncmp(dup_expr, "arg", 3) && strlen(dup_expr) == 4) {
+ int arg_num;
+
+ if (kstrtoint(dup_expr + 3, 10, &arg_num) || arg_num < 1 ||
+ arg_num > 6) {
+ ret = -EINVAL;
+ goto out;
+ }
+ cfg->base = KWATCH_BASE_ARG1 + (arg_num - 1);
+ } else if (kstrtoul(dup_expr, 0, &cfg->sym_addr) == 0) {
+ cfg->base = KWATCH_BASE_ABS_ADDR;
+ } else {
+#if IS_BUILTIN(CONFIG_KWATCH)
+ cfg->sym_addr = kallsyms_lookup_name(dup_expr);
+ if (!cfg->sym_addr) {
+ pr_err("Failed to resolve symbol name: %s\n", dup_expr);
+ ret = -EINVAL;
+ goto out;
+ }
+ cfg->base = KWATCH_BASE_GLOBAL_SYM;
+#else
+ pr_err("cannot resolve symbol %s when built as a module, use a hex address\n",
+ dup_expr);
+ ret = -EINVAL;
+ goto out;
+#endif
+ }
+
+ if (!type)
+ goto out;
+
+ /* 2. Resolve Base Offset (if + or - exists) */
+ if (!is_deref) {
+ char *next;
+
+ *sep = type; /* Restore the '+' or '-' for kstrtol */
+ next = strstr(sep, "->");
+ if (next)
+ *next = '\0';
+
+ if (kstrtol(sep, 0, &cfg->offsets[0])) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ p = next ? next + 2 : NULL;
+ } else {
+ /* Jump directly to the first dereference after '->' */
+ p = sep + 2;
+ }
+
+ /* 3. Resolve Dereference Chain */
+ while (p) {
+ char *next;
+
+ if (cfg->offset_count >= MAX_DEREF_CHAIN) {
+ ret = -E2BIG;
+ goto out;
+ }
+
+ next = strstr(p, "->");
+ if (next)
+ *next = '\0';
+
+ if (kstrtol(p, 0, &cfg->offsets[cfg->offset_count++])) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ p = next ? next + 2 : NULL;
+ }
+
+out:
+ kfree(dup_expr);
+ return ret;
+}
diff --git a/mm/kwatch/kwatch.h b/mm/kwatch/kwatch.h
new file mode 100644
index 000000000000..e1ac8ae312f6
--- /dev/null
+++ b/mm/kwatch/kwatch.h
@@ -0,0 +1,107 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _MM_KWATCH_H
+#define _MM_KWATCH_H
+
+#include <linux/fprobe.h>
+#include <linux/kprobes.h>
+#include <linux/perf_event.h>
+#include <linux/sched.h>
+#include <linux/types.h>
+#include <linux/compiler.h>
+#include <linux/atomic.h>
+
+#define MAX_CONFIG_STR_LEN 512
+#define MAX_DEREF_CHAIN 4
+
+struct kwatch_watchpoint;
+
+struct kwatch_tsk_ctx {
+ struct task_struct *task;
+ struct kwatch_watchpoint *wp;
+ u16 depth;
+ u32 epoch;
+};
+
+struct kwatch_watchpoint {
+ struct perf_event *__percpu *event;
+ call_single_data_t __percpu *csd_arm;
+ call_single_data_t __percpu *csd_disarm;
+ struct perf_event_attr attr;
+ atomic_t in_use; // multi-consumer safe get/put
+ struct list_head list; // for cpu online and offline
+
+ struct task_struct *arm_tsk;
+ atomic_t pending_ipis;
+ atomic_t refcount;
+ bool teardown;
+};
+
+enum kwatch_access_type {
+ KWATCH_ACCESS_W,
+ KWATCH_ACCESS_R,
+ KWATCH_ACCESS_RW,
+ KWATCH_ACCESS_X,
+};
+
+enum kwatch_base_type {
+ KWATCH_BASE_STACK,
+ KWATCH_BASE_ABS_ADDR,
+ KWATCH_BASE_GLOBAL_SYM,
+ KWATCH_BASE_ARG1,
+ KWATCH_BASE_ARG2,
+ KWATCH_BASE_ARG3,
+ KWATCH_BASE_ARG4,
+ KWATCH_BASE_ARG5,
+ KWATCH_BASE_ARG6,
+};
+
+struct kwatch_config {
+ u16 max_watch;
+ char func_name[KSYM_NAME_LEN];
+ u16 func_offset;
+ u16 depth;
+ u16 duration;
+ enum kwatch_access_type access_type;
+ u16 watch_len;
+
+ /* Unified Deref Engine State */
+ enum kwatch_base_type base;
+ char watch_expr[MAX_CONFIG_STR_LEN];
+ unsigned long sym_addr;
+ long offsets[MAX_DEREF_CHAIN];
+ u8 offset_count;
+ u16 max_concurrency;
+};
+
+int kwatch_hwbp_prealloc(u16 max_watch, enum kwatch_access_type access_type);
+void kwatch_hwbp_free(void);
+int kwatch_hwbp_get(struct kwatch_watchpoint **out_wp);
+void kwatch_hwbp_arm(struct kwatch_watchpoint *wp, unsigned long addr, u16 len);
+int kwatch_hwbp_put(struct kwatch_watchpoint *wp);
+
+int kwatch_probe_start(struct kwatch_config *cfg);
+void kwatch_probe_stop(void);
+void kwatch_probe_mute(bool mute);
+bool kwatch_probe_validate_hit(struct pt_regs *regs, struct task_struct *arm_tsk);
+unsigned long kwatch_probe_nmi_rejected(void);
+
+int kwatch_tsk_ctx_prealloc(u16 max_concurrency);
+struct kwatch_tsk_ctx *kwatch_tsk_ctx_get(bool can_alloc);
+void kwatch_tsk_ctx_put(void);
+void kwatch_tsk_ctx_reset(struct kwatch_tsk_ctx *ctx, u32 new_epoch);
+void kwatch_tsk_ctx_release_wps(void);
+void kwatch_tsk_ctx_free(void);
+
+void kwatch_global_anchor(unsigned long duration_sec);
+int kwatch_anchor_start(u16 duration);
+void kwatch_anchor_stop(void);
+void kwatch_anchor_cancel_work(void);
+bool kwatch_anchor_has_expired(void);
+void kwatch_anchor_clear_expired(void);
+void kwatch_auto_stop(void);
+
+int kwatch_deref_resolve(const struct kwatch_config *cfg, struct pt_regs *regs,
+ unsigned long *out_addr, u16 *out_len);
+int kwatch_deref_parse(struct kwatch_config *cfg, const char *watch_expr);
+
+#endif /* _MM_KWATCH_H */
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 05/13] mm/kwatch: add watch expression parser and dereference engine
2026-07-14 18:31 ` [RFC PATCH 05/13] mm/kwatch: add watch expression parser and dereference engine Jinchao Wang
@ 2026-07-14 18:45 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-07-14 18:45 UTC (permalink / raw)
To: Jinchao Wang; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kwatch_deref_resolve fails to validate intermediate pointers against the kernel-space boundary, leading to potential kernel panics.
--
--- Patch [5]: [RFC PATCH 05/13] mm/kwatch: add watch expression parser and dereference engine ---
commit 2105aab4719c42ad84e86aced2cb286ed992fb63
Author: Jinchao Wang <wangjinchao600@gmail.com>
mm/kwatch: add watch expression parser and dereference engine
This commit introduces the watch expression parser and dereference engine
for KWatch. It provides functions to parse a textual watch expression into
a configuration and resolve the pointer chain dynamically at probe time.
> diff --git a/mm/kwatch/deref.c b/mm/kwatch/deref.c
> new file mode 100644
> index 0000000000000..a93c76139e7cd
> --- /dev/null
> +++ b/mm/kwatch/deref.c
[ ... ]
> +int kwatch_deref_resolve(const struct kwatch_config *cfg, struct pt_regs *regs,
> + unsigned long *out_addr, u16 *out_len)
> +{
[ ... ]
> + /* 2. The Pointer-Chasing FSM */
> + for (i = 0; i < cfg->offset_count; i++) {
> + addr += cfg->offsets[i];
> +
> + if (i < cfg->offset_count - 1) {
> + unsigned long next_addr;
> +
> + /* Dynamically read the pointer contents at runtime */
> + if (get_kernel_nofault(next_addr, (unsigned long *)addr))
[Severity: High]
Is it safe to read from addr using get_kernel_nofault() before validating
that it falls within the kernel address space?
If a user configures a watch expression where an intermediate pointer
evaluates to a user-space address, reading it via get_kernel_nofault()
might trigger a hardware protection violation (e.g., SMAP or PAN) on
architectures where copy_from_kernel_nofault() does not internally
validate the address boundary.
Should the boundary check be moved inside the pointer-chasing loop prior
to the get_kernel_nofault() call?
> + return -EFAULT;
> +
> + addr = next_addr;
> + }
> + }
> +
> + /* Enforce strict Kernel-Space boundary */
> + if (unlikely(addr < TASK_SIZE_MAX))
> + return -EINVAL;
> +
> + *out_addr = addr;
> + *out_len = cfg->watch_len;
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714182243.10687-1-wangjinchao600@gmail.com?part=5
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 06/13] mm/kwatch: add lockless per-task context pool
2026-07-14 18:22 [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption Jinchao Wang
` (4 preceding siblings ...)
2026-07-14 18:31 ` [RFC PATCH 05/13] mm/kwatch: add watch expression parser and dereference engine Jinchao Wang
@ 2026-07-14 18:31 ` Jinchao Wang
2026-07-14 18:44 ` sashiko-bot
2026-07-14 18:31 ` [RFC PATCH 07/13] stacktrace: export stack_trace_save_regs() Jinchao Wang
` (6 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Jinchao Wang @ 2026-07-14 18:31 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
A task that enters the watched function needs somewhere to keep its
window state (nesting depth, owned watchpoint, config epoch). The
lookup runs in kprobe and NMI-like contexts, so it must not allocate
or take locks.
Use a preallocated open-addressing array hashed by task_struct
pointer. Slots are claimed with cmpxchg() and released with
smp_store_release(); lookup is a read-only probe sequence. The pool
size (max_concurrency) bounds how many tasks can be inside watch
windows concurrently; excess tasks are simply not tracked.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
mm/kwatch/Makefile | 2 +-
mm/kwatch/task_ctx.c | 105 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 106 insertions(+), 1 deletion(-)
create mode 100644 mm/kwatch/task_ctx.c
diff --git a/mm/kwatch/Makefile b/mm/kwatch/Makefile
index 69c21ae62123..cc6574df0d68 100644
--- a/mm/kwatch/Makefile
+++ b/mm/kwatch/Makefile
@@ -1,3 +1,3 @@
obj-$(CONFIG_KWATCH) += kwatch.o
-kwatch-y := deref.o
+kwatch-y := deref.o task_ctx.o
diff --git a/mm/kwatch/task_ctx.c b/mm/kwatch/task_ctx.c
new file mode 100644
index 000000000000..f8e582f0dcfe
--- /dev/null
+++ b/mm/kwatch/task_ctx.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/slab.h>
+#include <linux/hash.h>
+#include <linux/sched.h>
+#include <linux/log2.h>
+#include "kwatch.h"
+
+static u16 kwatch_ctx_pool_size;
+static u16 kwatch_ctx_pool_mask;
+
+static struct kwatch_tsk_ctx *kwatch_ctx_pool;
+
+int kwatch_tsk_ctx_prealloc(u16 max_concurrency)
+{
+ if (!max_concurrency)
+ max_concurrency = 256;
+
+ kwatch_ctx_pool_size = roundup_pow_of_two(max_concurrency);
+ kwatch_ctx_pool_mask = kwatch_ctx_pool_size - 1;
+
+ if (unlikely(!kwatch_ctx_pool)) {
+ kwatch_ctx_pool = kcalloc(kwatch_ctx_pool_size,
+ sizeof(struct kwatch_tsk_ctx),
+ GFP_KERNEL);
+ if (!kwatch_ctx_pool)
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+struct kwatch_tsk_ctx *kwatch_tsk_ctx_get(bool can_alloc)
+{
+ int start_idx, i, idx;
+ struct task_struct *t;
+
+ if (unlikely(!kwatch_ctx_pool))
+ return NULL;
+
+ start_idx = hash_ptr(current, ilog2(kwatch_ctx_pool_size));
+
+ for (i = 0; i < kwatch_ctx_pool_size; i++) {
+ idx = (start_idx + i) & kwatch_ctx_pool_mask;
+ t = READ_ONCE(kwatch_ctx_pool[idx].task);
+ if (t == current)
+ return &kwatch_ctx_pool[idx];
+ }
+
+ if (!can_alloc)
+ return NULL;
+
+ for (i = 0; i < kwatch_ctx_pool_size; i++) {
+ idx = (start_idx + i) & kwatch_ctx_pool_mask;
+ t = READ_ONCE(kwatch_ctx_pool[idx].task);
+ if (!t) {
+ if (!cmpxchg(&kwatch_ctx_pool[idx].task, NULL, current))
+ return &kwatch_ctx_pool[idx];
+ }
+ }
+
+ return NULL;
+}
+
+void kwatch_tsk_ctx_reset(struct kwatch_tsk_ctx *ctx, u32 new_epoch)
+{
+ struct kwatch_watchpoint *wp = xchg(&ctx->wp, NULL);
+
+ if (wp)
+ kwatch_hwbp_put(wp);
+ ctx->depth = 0;
+ ctx->epoch = new_epoch;
+}
+
+void kwatch_tsk_ctx_put(void)
+{
+ struct kwatch_tsk_ctx *ctx = kwatch_tsk_ctx_get(false);
+
+ if (unlikely(!ctx))
+ return;
+
+ kwatch_tsk_ctx_reset(ctx, 0);
+
+ /* Pairs with READ_ONCE() in kwatch_tsk_ctx_get() */
+ smp_store_release(&ctx->task, NULL);
+}
+
+void kwatch_tsk_ctx_release_wps(void)
+{
+ int i;
+
+ if (!kwatch_ctx_pool)
+ return;
+
+ for (i = 0; i < kwatch_ctx_pool_size; i++) {
+ struct kwatch_watchpoint *wp = xchg(&kwatch_ctx_pool[i].wp,
+ NULL);
+ if (wp)
+ kwatch_hwbp_put(wp);
+ }
+}
+
+void kwatch_tsk_ctx_free(void)
+{
+ kfree(kwatch_ctx_pool);
+ kwatch_ctx_pool = NULL;
+}
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 06/13] mm/kwatch: add lockless per-task context pool
2026-07-14 18:31 ` [RFC PATCH 06/13] mm/kwatch: add lockless per-task context pool Jinchao Wang
@ 2026-07-14 18:44 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-07-14 18:44 UTC (permalink / raw)
To: Jinchao Wang; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] NMI/IRQ reentrancy during context allocation and release causes shared context corruption and Use-After-Free.
- [High] `kwatch_tsk_ctx_free` frees the context pool without RCU synchronization, causing a Use-After-Free.
- [High] `kwatch_tsk_ctx_prealloc` unconditionally updates global size bounds without reallocating, causing Out-Of-Bounds accesses.
- [High] `kwatch_ctx_pool_size` truncates large inputs to 0, causing `ZERO_SIZE_PTR` dereferences and shift-by-64 Undefined Behavior.
- [High] The context pool stores un-refcounted task pointers, leading to state hijacking if a task exits abruptly (ABA problem).
--
--- Patch [6]: [RFC PATCH 06/13] mm/kwatch: add lockless per-task context pool ---
commit 70e01018388f64f2db26eb6d33251317f890288b
Author: Jinchao Wang <wangjinchao600@gmail.com>
mm/kwatch: add lockless per-task context pool
This patch introduces a lockless per-task context pool for kwatch to
track task window states in NMI/kprobe contexts without allocating or
taking locks. It uses an open-addressing array hashed by task_struct
pointer, claiming slots with cmpxchg() and releasing with
smp_store_release().
> diff --git a/mm/kwatch/task_ctx.c b/mm/kwatch/task_ctx.c
> new file mode 100644
> index 0000000000000..f8e582f0dcfe9
> --- /dev/null
> +++ b/mm/kwatch/task_ctx.c
> @@ -0,0 +1,105 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/slab.h>
> +#include <linux/hash.h>
> +#include <linux/sched.h>
> +#include <linux/log2.h>
> +#include "kwatch.h"
> +
> +static u16 kwatch_ctx_pool_size;
> +static u16 kwatch_ctx_pool_mask;
> +
> +static struct kwatch_tsk_ctx *kwatch_ctx_pool;
> +
> +int kwatch_tsk_ctx_prealloc(u16 max_concurrency)
> +{
> + if (!max_concurrency)
> + max_concurrency = 256;
> +
> + kwatch_ctx_pool_size = roundup_pow_of_two(max_concurrency);
> + kwatch_ctx_pool_mask = kwatch_ctx_pool_size - 1;
[Severity: High]
Does this code silently truncate the pool size and cause undefined behavior?
If max_concurrency > 32768, roundup_pow_of_two() will return 65536, which gets
truncated to 0 when assigned to the u16 kwatch_ctx_pool_size.
If this happens, kcalloc(0, ...) bypasses the allocation check by returning
ZERO_SIZE_PTR (0x10). Later, calling ilog2(0) in kwatch_tsk_ctx_get() evaluates
to -1, which results in a shift by 65 in hash_ptr().
Also, if max_concurrency is 1, ilog2(1) evaluates to 0, which would result in
a shift by 64 in hash_ptr(). Both cases trigger undefined behavior.
> +
> + if (unlikely(!kwatch_ctx_pool)) {
[Severity: High]
Can calling this function multiple times lead to out-of-bounds accesses?
If kwatch_tsk_ctx_prealloc() is called again with a larger max_concurrency,
kwatch_ctx_pool_size and kwatch_ctx_pool_mask will be updated to the larger
values. However, since kwatch_ctx_pool is already non-NULL, the array won't be
reallocated. Subsequent lookups will use the new, larger mask to calculate
indices, potentially accessing unallocated memory past the end of the original
array.
> + kwatch_ctx_pool = kcalloc(kwatch_ctx_pool_size,
> + sizeof(struct kwatch_tsk_ctx),
> + GFP_KERNEL);
> + if (!kwatch_ctx_pool)
> + return -ENOMEM;
> + }
> + return 0;
> +}
> +
> +struct kwatch_tsk_ctx *kwatch_tsk_ctx_get(bool can_alloc)
> +{
> + int start_idx, i, idx;
> + struct task_struct *t;
> +
> + if (unlikely(!kwatch_ctx_pool))
> + return NULL;
> +
> + start_idx = hash_ptr(current, ilog2(kwatch_ctx_pool_size));
> +
> + for (i = 0; i < kwatch_ctx_pool_size; i++) {
> + idx = (start_idx + i) & kwatch_ctx_pool_mask;
> + t = READ_ONCE(kwatch_ctx_pool[idx].task);
> + if (t == current)
[Severity: High]
Could this raw address comparison match a completely different task?
Since the task pointers aren't refcounted, if a task is killed (e.g., via
SIGKILL) while inside a watched function, it exits without calling
kwatch_tsk_ctx_put(). Its task_struct is eventually freed, but the raw memory
address is left dangling in kwatch_ctx_pool[idx].task.
If a new task is spawned and its task_struct is allocated at the exact same
memory address, kwatch_tsk_ctx_get() will find a false match here. The new
task will unintentionally inherit the dead task's depth and epoch state.
> + return &kwatch_ctx_pool[idx];
> + }
> +
> + if (!can_alloc)
> + return NULL;
> +
> + for (i = 0; i < kwatch_ctx_pool_size; i++) {
> + idx = (start_idx + i) & kwatch_ctx_pool_mask;
> + t = READ_ONCE(kwatch_ctx_pool[idx].task);
> + if (!t) {
> + if (!cmpxchg(&kwatch_ctx_pool[idx].task, NULL, current))
[Severity: High]
Can an NMI interrupting this path corrupt the context slot?
If an NMI interrupts the current task immediately after this cmpxchg() succeeds,
the NMI handler might call kwatch_tsk_ctx_get(), find the same slot (since
t == current), use it, and call kwatch_tsk_ctx_put().
kwatch_tsk_ctx_put() will then unconditionally set ctx->task to NULL:
smp_store_release(&ctx->task, NULL);
When the NMI returns, the interrupted task resumes thinking it owns the slot,
but the slot is now marked free and can be claimed by another task on another
CPU, leading to concurrent modification of the same context.
> + return &kwatch_ctx_pool[idx];
> + }
> + }
> +
> + return NULL;
> +}
[ ... ]
> +void kwatch_tsk_ctx_free(void)
> +{
> + kfree(kwatch_ctx_pool);
[Severity: High]
Does this code trigger a use-after-free if tasks are actively being watched?
If kwatch_tsk_ctx_free() is called on one CPU while another task (or NMI)
is inside kwatch_tsk_ctx_get() reading kwatch_ctx_pool on another CPU,
kfree() frees the memory immediately without waiting for an RCU grace period.
The lockless reader will then dereference the freed memory.
> + kwatch_ctx_pool = NULL;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714182243.10687-1-wangjinchao600@gmail.com?part=6
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 07/13] stacktrace: export stack_trace_save_regs()
2026-07-14 18:22 [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption Jinchao Wang
` (5 preceding siblings ...)
2026-07-14 18:31 ` [RFC PATCH 06/13] mm/kwatch: add lockless per-task context pool Jinchao Wang
@ 2026-07-14 18:31 ` Jinchao Wang
2026-07-14 18:42 ` sashiko-bot
2026-07-14 18:32 ` [RFC PATCH 08/13] mm/kwatch: add hardware breakpoint backend Jinchao Wang
` (5 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Jinchao Wang @ 2026-07-14 18:31 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
The other stack_trace_save_*() flavours are exported, but the regs
variant is not, so no module can capture a stack trace for a given
pt_regs. KWatch, which may be built as a module, uses it to record
who wrote to a watched address from the hardware breakpoint handler.
Export it like its siblings.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/stacktrace.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/stacktrace.c b/kernel/stacktrace.c
index afb3c116da91..d853c40f916b 100644
--- a/kernel/stacktrace.c
+++ b/kernel/stacktrace.c
@@ -175,6 +175,7 @@ unsigned int stack_trace_save_regs(struct pt_regs *regs, unsigned long *store,
arch_stack_walk(consume_entry, &c, current, regs);
return c.len;
}
+EXPORT_SYMBOL_GPL(stack_trace_save_regs);
#ifdef CONFIG_HAVE_RELIABLE_STACKTRACE
/**
@@ -325,6 +326,7 @@ unsigned int stack_trace_save_regs(struct pt_regs *regs, unsigned long *store,
save_stack_trace_regs(regs, &trace);
return trace.nr_entries;
}
+EXPORT_SYMBOL_GPL(stack_trace_save_regs);
#ifdef CONFIG_HAVE_RELIABLE_STACKTRACE
/**
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 07/13] stacktrace: export stack_trace_save_regs()
2026-07-14 18:31 ` [RFC PATCH 07/13] stacktrace: export stack_trace_save_regs() Jinchao Wang
@ 2026-07-14 18:42 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-07-14 18:42 UTC (permalink / raw)
To: Jinchao Wang; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch exports `stack_trace_save_regs` so it can be used by modules, but fails to update the in-tree kprobes sanity test which contains a workaround specifically because the symbol was previously unexported. Leaving the workaround artificially reduces test coverage for module builds.
--
--- Patch [7]: [RFC PATCH 07/13] stacktrace: export stack_trace_save_regs() ---
commit 594dee655bf56c8a9f1858d2d50ddbf441315c12
Author: Jinchao Wang <wangjinchao600@gmail.com>
stacktrace: export stack_trace_save_regs()
The other stack_trace_save_*() flavours are exported, but the regs
variant is not, so no module can capture a stack trace for a given
pt_regs. KWatch, which may be built as a module, uses it. Export it like
its siblings.
> diff --git a/kernel/stacktrace.c b/kernel/stacktrace.c
> index afb3c116da91a..d853c40f916b2 100644
> --- a/kernel/stacktrace.c
> +++ b/kernel/stacktrace.c
> @@ -175,6 +175,7 @@ unsigned int stack_trace_save_regs(struct pt_regs *regs, unsigned long *store,
> arch_stack_walk(consume_entry, &c, current, regs);
> return c.len;
> }
> +EXPORT_SYMBOL_GPL(stack_trace_save_regs);
[Severity: Medium]
Since stack_trace_save_regs() is now exported for modules, should we also
update the kprobes sanity test to remove the workarounds that skip testing
this function when built as a module?
In lib/tests/test_kprobes.c:stacktrace_return_handler():
#if !IS_MODULE(CONFIG_KPROBES_SANITY_TEST)
/*
* Test stacktrace from pt_regs at the return address. Thus the stack
* trace must start from the target return address.
*/
ret = stack_trace_save_regs(regs, stack_buf, STACK_BUF_SIZE, 0);
And similarly in lib/tests/test_kprobes.c:stacktrace_internal_return_handler():
#if !IS_MODULE(CONFIG_KPROBES_SANITY_TEST)
/* Ditto for the regs version. */
ret = stack_trace_save_regs(regs, stack_buf, STACK_BUF_SIZE, 0);
Removing these checks would restore test coverage for module builds now that
the symbol is available.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714182243.10687-1-wangjinchao600@gmail.com?part=7
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 08/13] mm/kwatch: add hardware breakpoint backend
2026-07-14 18:22 [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption Jinchao Wang
` (6 preceding siblings ...)
2026-07-14 18:31 ` [RFC PATCH 07/13] stacktrace: export stack_trace_save_regs() Jinchao Wang
@ 2026-07-14 18:32 ` Jinchao Wang
2026-07-14 18:50 ` sashiko-bot
2026-07-14 18:32 ` [RFC PATCH 09/13] mm/kwatch: add probe lifecycle runtime Jinchao Wang
` (4 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Jinchao Wang @ 2026-07-14 18:32 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
Manage a preallocated pool of wide (per-CPU) perf hardware
breakpoints. All breakpoints are registered up front against a dummy
address; arming a watchpoint only re-points an already-registered
event, so the arm path can run from a kprobe handler.
- kwatch_hwbp_get()/put() claim and release pool entries with
per-slot cmpxchg, safe for concurrent consumers on any CPU.
- kwatch_hwbp_arm() updates the local CPU synchronously via
modify_wide_hw_breakpoint_local() and broadcasts asynchronous IPIs
to the other CPUs. Arm-side IPIs are rate-limited per CPU; disarm
IPIs are refcounted so an entry is only recycled once every CPU
has dropped it.
- Hits are reported through the kwatch:kwatch_hit tracepoint with a
short stack trace: the ftrace ring buffer is usable from NMI-like
context and survives a subsequent crash, unlike printk.
- A CPU hotplug callback creates/destroys the per-CPU events as CPUs
come and go.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
include/trace/events/kwatch.h | 57 ++++++
mm/kwatch/Makefile | 2 +-
mm/kwatch/hwbp.c | 358 ++++++++++++++++++++++++++++++++++
3 files changed, 416 insertions(+), 1 deletion(-)
create mode 100644 include/trace/events/kwatch.h
create mode 100644 mm/kwatch/hwbp.c
diff --git a/include/trace/events/kwatch.h b/include/trace/events/kwatch.h
new file mode 100644
index 000000000000..edb95405c386
--- /dev/null
+++ b/include/trace/events/kwatch.h
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM kwatch
+
+#if !defined(_TRACE_KWATCH_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_KWATCH_H
+
+#include <linux/tracepoint.h>
+#include <linux/ptrace.h>
+
+#define KWATCH_STACK_DEPTH 8
+
+struct trace_seq;
+const char *kwatch_trace_print_stack(struct trace_seq *p,
+ const unsigned long *stack,
+ unsigned int nr);
+
+TRACE_EVENT(kwatch_hit,
+ TP_PROTO(unsigned long ip, unsigned long sp, unsigned long addr,
+ u64 time_ns,
+ unsigned long *stack_entries, unsigned int stack_nr),
+ TP_ARGS(ip, sp, addr, time_ns, stack_entries, stack_nr),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, ip)
+ __field(unsigned long, sp)
+ __field(unsigned long, addr)
+ __field(u64, time_ns)
+ __field(unsigned int, stack_nr)
+ __array(unsigned long, stack, KWATCH_STACK_DEPTH)
+ ),
+
+ TP_fast_assign(
+ unsigned int i;
+
+ __entry->ip = ip;
+ __entry->sp = sp;
+ __entry->addr = addr;
+ __entry->time_ns = time_ns;
+ __entry->stack_nr = min_t(unsigned int, stack_nr,
+ KWATCH_STACK_DEPTH);
+ for (i = 0; i < __entry->stack_nr; i++)
+ __entry->stack[i] = stack_entries[i];
+ ),
+
+ TP_printk("KWatch HIT: time=%llu.%06lu ip=%pS addr=0x%lx%s",
+ __entry->time_ns / 1000000000ULL,
+ (unsigned long)((__entry->time_ns / 1000ULL) % 1000000ULL),
+ (void *)__entry->ip, __entry->addr,
+ kwatch_trace_print_stack(p, __entry->stack,
+ __entry->stack_nr))
+);
+
+#endif /* _TRACE_KWATCH_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/mm/kwatch/Makefile b/mm/kwatch/Makefile
index cc6574df0d68..b2bc3003c89b 100644
--- a/mm/kwatch/Makefile
+++ b/mm/kwatch/Makefile
@@ -1,3 +1,3 @@
obj-$(CONFIG_KWATCH) += kwatch.o
-kwatch-y := deref.o task_ctx.o
+kwatch-y := deref.o task_ctx.o hwbp.o
diff --git a/mm/kwatch/hwbp.c b/mm/kwatch/hwbp.c
new file mode 100644
index 000000000000..19498ba03826
--- /dev/null
+++ b/mm/kwatch/hwbp.c
@@ -0,0 +1,358 @@
+// SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/cpuhotplug.h>
+#include <linux/ftrace.h>
+#include <linux/hw_breakpoint.h>
+#include <linux/sched/clock.h>
+#include <linux/irqflags.h>
+#include <linux/kallsyms.h>
+#include <linux/mutex.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
+#include <linux/stacktrace.h>
+#include <linux/trace_seq.h>
+#include <linux/workqueue.h>
+
+#include "kwatch.h"
+
+static LIST_HEAD(kwatch_all_wp_list);
+static struct kwatch_watchpoint **kwatch_wp_slots;
+static u16 kwatch_wp_nr;
+static DEFINE_MUTEX(kwatch_all_wp_mutex);
+static unsigned long kwatch_dummy_holder __aligned(8);
+static int kwatch_hwbp_cpuhp_state = CPUHP_INVALID;
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/kwatch.h>
+
+/*
+ * Render the saved stack like the ftrace built-in stacktrace / dump_stack()
+ * style. Symbol resolution runs at trace read time, not in the hit path.
+ */
+const char *kwatch_trace_print_stack(struct trace_seq *p,
+ const unsigned long *stack,
+ unsigned int nr)
+{
+ const char *ret = trace_seq_buffer_ptr(p);
+ unsigned int i;
+
+ for (i = 0; i < nr; i++)
+ trace_seq_printf(p, "\n => %pS", (void *)stack[i]);
+ trace_seq_putc(p, 0);
+ return ret;
+}
+
+static void kwatch_hwbp_handler(struct perf_event *bp,
+ struct perf_sample_data *data,
+ struct pt_regs *regs)
+{
+ struct kwatch_watchpoint *wp = bp->overflow_handler_context;
+ unsigned long stack_entries[KWATCH_STACK_DEPTH];
+ unsigned int stack_nr;
+
+ if (!kwatch_probe_validate_hit(regs, wp->arm_tsk))
+ return;
+
+ stack_nr = stack_trace_save_regs(regs, stack_entries, KWATCH_STACK_DEPTH, 2);
+ trace_kwatch_hit(instruction_pointer(regs), kernel_stack_pointer(regs),
+ wp->attr.bp_addr, local_clock(),
+ stack_entries, stack_nr);
+}
+
+static void kwatch_hwbp_arm_local(void *info)
+{
+ struct kwatch_watchpoint *wp = info;
+ struct perf_event *bp;
+ unsigned long flags;
+ int cpu, err;
+
+ local_irq_save(flags);
+
+ cpu = smp_processor_id();
+ bp = per_cpu(*wp->event, cpu);
+
+ if (unlikely(!bp))
+ goto out;
+
+ kwatch_probe_mute(true);
+ barrier();
+
+ err = modify_wide_hw_breakpoint_local(bp, &wp->attr);
+ if (unlikely(err)) {
+ WARN_ONCE(1,
+ "KWatch: HWBP reinstall failed on CPU%d (err=%d, addr=0x%llx, len=%llu)\n",
+ cpu, err, wp->attr.bp_addr, wp->attr.bp_len);
+ }
+
+ barrier();
+ kwatch_probe_mute(false);
+
+out:
+ local_irq_restore(flags);
+}
+
+static inline void kwatch_hwbp_try_recycle(struct kwatch_watchpoint *wp)
+{
+ if (atomic_dec_and_test(&wp->pending_ipis)) {
+ if (!READ_ONCE(wp->teardown))
+ atomic_set_release(&wp->in_use, 0);
+
+ atomic_dec(&wp->refcount);
+ }
+}
+
+static void kwatch_hwbp_disarm_local(void *info)
+{
+ struct kwatch_watchpoint *wp = info;
+
+ kwatch_hwbp_arm_local(info);
+ kwatch_hwbp_try_recycle(wp);
+}
+
+static int kwatch_hwbp_cpu_online(unsigned int cpu)
+{
+ struct perf_event_attr attr;
+ struct kwatch_watchpoint *wp;
+ struct perf_event *bp;
+
+ mutex_lock(&kwatch_all_wp_mutex);
+ list_for_each_entry(wp, &kwatch_all_wp_list, list) {
+ attr = wp->attr;
+ attr.bp_addr = (unsigned long)&kwatch_dummy_holder;
+ bp = perf_event_create_kernel_counter(&attr, cpu, NULL,
+ kwatch_hwbp_handler, wp);
+ if (IS_ERR(bp)) {
+ pr_warn("%s failed to create watch on CPU %d: %ld\n",
+ __func__, cpu, PTR_ERR(bp));
+ continue;
+ }
+ per_cpu(*wp->event, cpu) = bp;
+ }
+ mutex_unlock(&kwatch_all_wp_mutex);
+ return 0;
+}
+
+static int kwatch_hwbp_cpu_offline(unsigned int cpu)
+{
+ struct kwatch_watchpoint *wp;
+ struct perf_event *bp;
+
+ mutex_lock(&kwatch_all_wp_mutex);
+ list_for_each_entry(wp, &kwatch_all_wp_list, list) {
+ bp = per_cpu(*wp->event, cpu);
+ if (bp) {
+ unregister_hw_breakpoint(bp);
+ per_cpu(*wp->event, cpu) = NULL;
+ }
+ }
+ mutex_unlock(&kwatch_all_wp_mutex);
+ return 0;
+}
+
+int kwatch_hwbp_get(struct kwatch_watchpoint **out_wp)
+{
+ struct kwatch_watchpoint *wp;
+ int i;
+
+ /*
+ * Per-slot cmpxchg claim: safe for concurrent consumers on any CPU,
+ * unlike llist_del_first() which requires a single consumer.
+ */
+ for (i = 0; i < kwatch_wp_nr; i++) {
+ wp = kwatch_wp_slots[i];
+ if (atomic_read(&wp->in_use))
+ continue;
+ if (atomic_cmpxchg(&wp->in_use, 0, 1) == 0) {
+ atomic_inc(&wp->refcount);
+ *out_wp = wp;
+ return 0;
+ }
+ }
+ return -EBUSY;
+}
+
+void kwatch_hwbp_arm(struct kwatch_watchpoint *wp, unsigned long addr, u16 len)
+{
+ static DEFINE_PER_CPU(u64, last_ipi_time);
+ int cur_cpu;
+ call_single_data_t *csd;
+ int cpu;
+ bool is_disarm = (addr == (unsigned long)&kwatch_dummy_holder);
+
+ wp->attr.bp_addr = addr;
+ wp->attr.bp_len = len;
+
+ if (!is_disarm)
+ wp->arm_tsk = current;
+
+ /* ensure attr update visible to other cpu before sending IPI */
+ smp_wmb();
+
+ atomic_set(&wp->pending_ipis, 1);
+ cur_cpu = get_cpu();
+
+ if (!is_disarm) {
+ u64 now = local_clock();
+ u64 last = this_cpu_read(last_ipi_time);
+
+ if (now - last < 1000000ULL) {
+ put_cpu();
+ return;
+ }
+ this_cpu_write(last_ipi_time, now);
+ }
+ for_each_online_cpu(cpu) {
+ if (cpu == cur_cpu)
+ continue;
+
+ if (is_disarm)
+ atomic_inc(&wp->pending_ipis);
+
+ csd = per_cpu_ptr(is_disarm ? wp->csd_disarm : wp->csd_arm,
+ cpu);
+ if (smp_call_function_single_async(cpu, csd) && is_disarm)
+ kwatch_hwbp_try_recycle(wp);
+ }
+ put_cpu();
+
+ if (is_disarm)
+ kwatch_hwbp_disarm_local(wp);
+ else
+ kwatch_hwbp_arm_local(wp);
+}
+
+int kwatch_hwbp_put(struct kwatch_watchpoint *wp)
+{
+ kwatch_hwbp_arm(wp, (unsigned long)&kwatch_dummy_holder,
+ sizeof(unsigned long));
+
+ return 0;
+}
+
+void kwatch_hwbp_free(void)
+{
+ struct kwatch_watchpoint *wp, *tmp;
+
+ kwatch_wp_nr = 0;
+ kfree(kwatch_wp_slots);
+ kwatch_wp_slots = NULL;
+
+ if (kwatch_hwbp_cpuhp_state != CPUHP_INVALID) {
+ cpuhp_remove_state_nocalls(kwatch_hwbp_cpuhp_state);
+ kwatch_hwbp_cpuhp_state = CPUHP_INVALID;
+ }
+
+ mutex_lock(&kwatch_all_wp_mutex);
+ list_for_each_entry_safe(wp, tmp, &kwatch_all_wp_list, list) {
+ list_del(&wp->list);
+
+ WRITE_ONCE(wp->teardown, true);
+ atomic_dec(&wp->refcount);
+
+ /* Wait for all async IPIs to finish */
+ while (atomic_read(&wp->refcount) > 0)
+ cpu_relax();
+
+ unregister_wide_hw_breakpoint(wp->event);
+ free_percpu(wp->csd_arm);
+ free_percpu(wp->csd_disarm);
+ kfree(wp);
+ }
+ mutex_unlock(&kwatch_all_wp_mutex);
+}
+
+int kwatch_hwbp_prealloc(u16 max_watch, enum kwatch_access_type access_type)
+{
+ struct kwatch_watchpoint *wp;
+ int success = 0, cpu;
+ u32 bp_type;
+ int ret;
+
+ switch (access_type) {
+ case KWATCH_ACCESS_X:
+ bp_type = HW_BREAKPOINT_X;
+ break;
+ case KWATCH_ACCESS_R:
+ bp_type = HW_BREAKPOINT_R;
+ break;
+ case KWATCH_ACCESS_RW:
+ bp_type = HW_BREAKPOINT_RW;
+ break;
+ case KWATCH_ACCESS_W:
+ default:
+ bp_type = HW_BREAKPOINT_W;
+ break;
+ }
+
+ while (!max_watch || success < max_watch) {
+ wp = kzalloc_obj(*wp);
+ if (!wp)
+ break;
+
+ wp->csd_arm = alloc_percpu(call_single_data_t);
+ wp->csd_disarm = alloc_percpu(call_single_data_t);
+ if (!wp->csd_arm || !wp->csd_disarm) {
+ free_percpu(wp->csd_arm);
+ free_percpu(wp->csd_disarm);
+ kfree(wp);
+ break;
+ }
+
+ for_each_possible_cpu(cpu) {
+ INIT_CSD(per_cpu_ptr(wp->csd_arm, cpu),
+ kwatch_hwbp_arm_local, wp);
+ INIT_CSD(per_cpu_ptr(wp->csd_disarm, cpu),
+ kwatch_hwbp_disarm_local, wp);
+ }
+
+ wp->teardown = false;
+
+ hw_breakpoint_init(&wp->attr);
+ wp->attr.bp_addr = (unsigned long)&kwatch_dummy_holder;
+ wp->attr.bp_len = sizeof(unsigned long);
+ wp->attr.bp_type = bp_type;
+
+ wp->event = register_wide_hw_breakpoint(&wp->attr,
+ kwatch_hwbp_handler,
+ wp);
+ if (IS_ERR((void *)wp->event)) {
+ free_percpu(wp->csd_arm);
+ free_percpu(wp->csd_disarm);
+ kfree(wp);
+ break;
+ }
+
+ atomic_set(&wp->refcount, 1);
+
+ mutex_lock(&kwatch_all_wp_mutex);
+ list_add(&wp->list, &kwatch_all_wp_list);
+ mutex_unlock(&kwatch_all_wp_mutex);
+ success++;
+ }
+
+ if (!success)
+ return -EBUSY;
+
+ kwatch_wp_slots = kcalloc(success, sizeof(*kwatch_wp_slots),
+ GFP_KERNEL);
+ if (!kwatch_wp_slots) {
+ kwatch_hwbp_free();
+ return -ENOMEM;
+ }
+ mutex_lock(&kwatch_all_wp_mutex);
+ list_for_each_entry(wp, &kwatch_all_wp_list, list)
+ kwatch_wp_slots[kwatch_wp_nr++] = wp;
+ mutex_unlock(&kwatch_all_wp_mutex);
+
+ ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "kwatch:online",
+ kwatch_hwbp_cpu_online,
+ kwatch_hwbp_cpu_offline);
+ if (ret < 0) {
+ kwatch_hwbp_free();
+ return ret;
+ }
+
+ kwatch_hwbp_cpuhp_state = ret;
+ return 0;
+}
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 08/13] mm/kwatch: add hardware breakpoint backend
2026-07-14 18:32 ` [RFC PATCH 08/13] mm/kwatch: add hardware breakpoint backend Jinchao Wang
@ 2026-07-14 18:50 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-07-14 18:50 UTC (permalink / raw)
To: Jinchao Wang; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [Critical] Repeated calls to `kwatch_hwbp_prealloc` write out of bounds of the `kwatch_wp_slots` array, leading to a heap buffer overflow.
- [High] CPU hotplug online callback overwrites active watchpoints with the dummy address, breaking them on newly onlined CPUs.
- [High] A static global per-CPU rate limit in `kwatch_hwbp_arm` silently drops arming requests, causing independent watchpoints to fail.
- [High] Missing IPI completion tracking for arm requests allows `csd_arm` to be reused while still pending, causing subsequent arm requests to silently fail.
- [High] `kwatch_hwbp_get` locklessly accesses `kwatch_wp_slots` while `kwatch_hwbp_free` frees it without synchronization.
- [Medium] Uninitialized kernel memory leaked to the trace ring buffer when the stack depth is less than KWATCH_STACK_DEPTH.
- [Medium] Function calls with side effects in `TP_printk` arguments cause trace buffer corruption.
- [Medium] `kwatch_hwbp_handler` logs the globally shared address instead of the local address, recording incorrect data during concurrent updates.
- [Low] 64-bit division in tracepoint printk causes linker errors on 32-bit architectures.
--
--- Patch [8]: [RFC PATCH 08/13] mm/kwatch: add hardware breakpoint backend ---
commit e30fa607776b2ca60d90cdf9ad4735ab9b2801f7
Author: Jinchao Wang <wangjinchao600@gmail.com>
mm/kwatch: add hardware breakpoint backend
Manage a preallocated pool of wide (per-CPU) perf hardware breakpoints.
All breakpoints are registered up front against a dummy address; arming
a watchpoint only re-points an already-registered event, so the arm path
can run from a kprobe handler.
> diff --git a/include/trace/events/kwatch.h b/include/trace/events/kwatch.h
> new file mode 100644
> index 0000000000000..edb95405c3869
> --- /dev/null
> +++ b/include/trace/events/kwatch.h
[ ... ]
> + TP_fast_assign(
> + unsigned int i;
> +
> + __entry->ip = ip;
> + __entry->sp = sp;
> + __entry->addr = addr;
> + __entry->time_ns = time_ns;
> + __entry->stack_nr = min_t(unsigned int, stack_nr,
> + KWATCH_STACK_DEPTH);
> + for (i = 0; i < __entry->stack_nr; i++)
> + __entry->stack[i] = stack_entries[i];
> + ),
[Severity: Medium]
Will this leave the remaining elements in __entry->stack uninitialized when
stack_nr is less than KWATCH_STACK_DEPTH?
Because this memory comes from the tracing ring buffer, leaking uninitialized
kernel data might allow userspace to read stale kernel trace records.
> +
> + TP_printk("KWatch HIT: time=%llu.%06lu ip=%pS addr=0x%lx%s",
> + __entry->time_ns / 1000000000ULL,
> + (unsigned long)((__entry->time_ns / 1000ULL) % 1000000ULL),
[Severity: Low]
Is it safe to use 64-bit division and modulo operators here?
Native 64-bit division on __entry->time_ns can cause build failures due to
unresolved __udivdi3 symbols on 32-bit architectures. Could div64_u64() or
do_div() be used instead?
> + (void *)__entry->ip, __entry->addr,
> + kwatch_trace_print_stack(p, __entry->stack,
> + __entry->stack_nr))
> +);
[Severity: Medium]
Can executing kwatch_trace_print_stack() as an argument to TP_printk()
corrupt the trace output buffer?
Due to argument evaluation order, kwatch_trace_print_stack() could evaluate
before trace_seq_printf(). Since it writes directly to the shared trace_seq
buffer and inserts a null byte:
trace_seq_putc(p, 0);
trace_seq_printf() would append the main event string after the null byte.
When userspace reads the trace, it stops at the null byte, resulting in a
mangled or truncated trace log.
> diff --git a/mm/kwatch/hwbp.c b/mm/kwatch/hwbp.c
> new file mode 100644
> index 0000000000000..19498ba038260
> --- /dev/null
> +++ b/mm/kwatch/hwbp.c
[ ... ]
> +static void kwatch_hwbp_handler(struct perf_event *bp,
> + struct perf_sample_data *data,
> + struct pt_regs *regs)
> +{
> + struct kwatch_watchpoint *wp = bp->overflow_handler_context;
> + unsigned long stack_entries[KWATCH_STACK_DEPTH];
> + unsigned int stack_nr;
> +
> + if (!kwatch_probe_validate_hit(regs, wp->arm_tsk))
> + return;
> +
> + stack_nr = stack_trace_save_regs(regs, stack_entries, KWATCH_STACK_DEPTH, 2);
> + trace_kwatch_hit(instruction_pointer(regs), kernel_stack_pointer(regs),
> + wp->attr.bp_addr, local_clock(),
> + stack_entries, stack_nr);
> +}
[Severity: Medium]
Could this tracepoint log the wrong address during a concurrent update?
If another CPU changes the watchpoint address, it updates the global
wp->attr.bp_addr and sends IPIs. If a hit on the old hardware breakpoint
triggers here before the IPI is received, it will log the newly updated
global wp->attr.bp_addr instead of the local perf event's configured
address (bp->attr.bp_addr).
[ ... ]
> +static int kwatch_hwbp_cpu_online(unsigned int cpu)
> +{
> + struct perf_event_attr attr;
> + struct kwatch_watchpoint *wp;
> + struct perf_event *bp;
> +
> + mutex_lock(&kwatch_all_wp_mutex);
> + list_for_each_entry(wp, &kwatch_all_wp_list, list) {
> + attr = wp->attr;
> + attr.bp_addr = (unsigned long)&kwatch_dummy_holder;
[Severity: High]
Does this accidentally force newly onlined CPUs to monitor the dummy address
instead of the active watchpoint address?
If a CPU is onlined on a system with active watchpoints, they will silently
fail to trigger on the new CPU because it explicitly configures the hardware
breakpoint for kwatch_dummy_holder instead of the active wp->attr.bp_addr.
[ ... ]
> +int kwatch_hwbp_get(struct kwatch_watchpoint **out_wp)
> +{
> + struct kwatch_watchpoint *wp;
> + int i;
> +
> + /*
> + * Per-slot cmpxchg claim: safe for concurrent consumers on any CPU,
> + * unlike llist_del_first() which requires a single consumer.
> + */
> + for (i = 0; i < kwatch_wp_nr; i++) {
> + wp = kwatch_wp_slots[i];
[Severity: High]
Is it safe to access kwatch_wp_slots here without holding a lock or
using RCU?
If an error occurs during preallocation or a module unload triggers
kwatch_hwbp_free(), the array can be freed:
kwatch_wp_nr = 0;
kfree(kwatch_wp_slots);
If kwatch_hwbp_get() reads kwatch_wp_nr before it is zeroed and dereferences
the array after it is freed, it could lead to a use-after-free.
[ ... ]
> +void kwatch_hwbp_arm(struct kwatch_watchpoint *wp, unsigned long addr, u16 len)
> +{
> + static DEFINE_PER_CPU(u64, last_ipi_time);
> + int cur_cpu;
> + call_single_data_t *csd;
> + int cpu;
> + bool is_disarm = (addr == (unsigned long)&kwatch_dummy_holder);
> +
> + wp->attr.bp_addr = addr;
> + wp->attr.bp_len = len;
> +
> + if (!is_disarm)
> + wp->arm_tsk = current;
> +
> + /* ensure attr update visible to other cpu before sending IPI */
> + smp_wmb();
> +
> + atomic_set(&wp->pending_ipis, 1);
> + cur_cpu = get_cpu();
> +
> + if (!is_disarm) {
> + u64 now = local_clock();
> + u64 last = this_cpu_read(last_ipi_time);
> +
> + if (now - last < 1000000ULL) {
> + put_cpu();
> + return;
> + }
[Severity: High]
Could this static global per-CPU rate limit silently break watchpoints?
If two distinct watchpoints are armed on the same CPU within 1 millisecond,
the second request will return early here. However, this happens after the
global wp->attr.bp_addr and wp->arm_tsk have already been modified.
This leaves the hardware monitoring the old address while the metadata claims
it is armed, causing the new watchpoint to miss events.
> + this_cpu_write(last_ipi_time, now);
> + }
> + for_each_online_cpu(cpu) {
> + if (cpu == cur_cpu)
> + continue;
> +
> + if (is_disarm)
> + atomic_inc(&wp->pending_ipis);
> +
> + csd = per_cpu_ptr(is_disarm ? wp->csd_disarm : wp->csd_arm,
> + cpu);
> + if (smp_call_function_single_async(cpu, csd) && is_disarm)
> + kwatch_hwbp_try_recycle(wp);
> + }
[Severity: High]
Should arm requests also handle the case where the IPI fails with -EBUSY?
If a watchpoint is rapidly toggled, the second arm attempt can try to queue
csd_arm while the remote CPU is still processing the first one. This causes
smp_call_function_single_async() to return -EBUSY.
Because the error is only checked for disarm requests (the && is_disarm
check), the failure to arm is silently ignored, and the new address is never
programmed on the busy CPU.
[ ... ]
> +int kwatch_hwbp_prealloc(u16 max_watch, enum kwatch_access_type access_type)
> +{
[ ... ]
> + kwatch_wp_slots = kcalloc(success, sizeof(*kwatch_wp_slots),
> + GFP_KERNEL);
> + if (!kwatch_wp_slots) {
> + kwatch_hwbp_free();
> + return -ENOMEM;
> + }
> + mutex_lock(&kwatch_all_wp_mutex);
> + list_for_each_entry(wp, &kwatch_all_wp_list, list)
> + kwatch_wp_slots[kwatch_wp_nr++] = wp;
[Severity: Critical]
Will subsequent calls to kwatch_hwbp_prealloc() cause a heap buffer
overflow?
If this function is called more than once, it allocates a new
kwatch_wp_slots array sized only for the new success count. However, the
static kwatch_all_wp_list retains elements from prior calls, and the static
kwatch_wp_nr is never reset.
Iterating the accumulated global list and writing elements via
kwatch_wp_slots[kwatch_wp_nr++] will write out of bounds of the newly
allocated array.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714182243.10687-1-wangjinchao600@gmail.com?part=8
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 09/13] mm/kwatch: add probe lifecycle runtime
2026-07-14 18:22 [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption Jinchao Wang
` (7 preceding siblings ...)
2026-07-14 18:32 ` [RFC PATCH 08/13] mm/kwatch: add hardware breakpoint backend Jinchao Wang
@ 2026-07-14 18:32 ` Jinchao Wang
2026-07-14 19:53 ` sashiko-bot
2026-07-14 18:32 ` [RFC PATCH 10/13] mm/kwatch: add anchor thread for global watchpoints Jinchao Wang
` (3 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Jinchao Wang @ 2026-07-14 18:32 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
Open and close the watch window with a kretprobe on the target
function: the entry handler tracks per-task nesting depth and, when
the configured depth is reached, resolves the watch expression and
arms a watchpoint; the exit handler disarms it. An optional kprobe
at func_offset arms mid-function instead of at entry.
Functions running in a real NMI(-like) context are rejected once, at
function entry, by comparing the NMI nesting count against the one
NMI-like layer that int3-based kprobe delivery itself adds; a
companion kprobe with a post_handler pins the probe point so jump
optimization cannot change the delivery mechanism after it is
sampled. Rejections are counted and exposed to the control plane.
A global epoch versioning scheme invalidates stale per-task state
across reconfigurations, and a per-CPU mute flag keeps window
management quiet while a CPU rewrites its own debug registers.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
mm/kwatch/Makefile | 2 +-
mm/kwatch/probe.c | 263 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 264 insertions(+), 1 deletion(-)
create mode 100644 mm/kwatch/probe.c
diff --git a/mm/kwatch/Makefile b/mm/kwatch/Makefile
index b2bc3003c89b..f04673cc5b1c 100644
--- a/mm/kwatch/Makefile
+++ b/mm/kwatch/Makefile
@@ -1,3 +1,3 @@
obj-$(CONFIG_KWATCH) += kwatch.o
-kwatch-y := deref.o task_ctx.o hwbp.o
+kwatch-y := deref.o task_ctx.o hwbp.o probe.o
diff --git a/mm/kwatch/probe.c b/mm/kwatch/probe.c
new file mode 100644
index 000000000000..af6e0af45c10
--- /dev/null
+++ b/mm/kwatch/probe.c
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/atomic.h>
+#include <linux/kprobes.h>
+#include <linux/kallsyms.h>
+#include <linux/percpu.h>
+#include <linux/preempt.h>
+#include <linux/sched.h>
+
+#include "kwatch.h"
+#define TRAMPOLINE_CHECK_DEPTH 16
+static DEFINE_PER_CPU(bool, kwatch_probe_cpu_muted);
+
+struct kwatch_probe_ctx {
+ struct kprobe kp;
+ struct kretprobe rp;
+ struct kprobe pin_kp;
+ const struct kwatch_config *cfg;
+ bool rp_via_int3;
+
+ u32 epoch;
+};
+
+static struct kwatch_probe_ctx kwatch_probe_ctx;
+static atomic_long_t kwatch_nmi_rejected;
+
+unsigned long kwatch_probe_nmi_rejected(void)
+{
+ return atomic_long_read(&kwatch_nmi_rejected);
+}
+
+/*
+ * True if the probed function itself runs in an NMI-like context.
+ * int3-based kprobe delivery adds one NMI-like layer of its own;
+ * delivery is pinned at registration so the subtraction stays exact.
+ */
+static bool kwatch_probed_ctx_in_nmi(bool via_int3)
+{
+ return (preempt_count() & NMI_MASK) > (via_int3 ? NMI_OFFSET : 0);
+}
+
+static void kwatch_pin_post_handler(struct kprobe *p, struct pt_regs *regs,
+ unsigned long flags)
+{
+ /* a post_handler pins the probepoint: no jump optimization */
+}
+
+bool kwatch_probe_validate_hit(struct pt_regs *regs,
+ struct task_struct *arm_tsk)
+{
+ struct kwatch_tsk_ctx *ctx = kwatch_tsk_ctx_get(false);
+
+ if (unlikely(!ctx))
+ return true;
+
+ if (arm_tsk != current ||
+ ctx->depth != kwatch_probe_ctx.cfg->depth + 1)
+ return true;
+
+ return false;
+}
+
+void kwatch_probe_mute(bool mute)
+{
+ __this_cpu_write(kwatch_probe_cpu_muted, mute);
+}
+
+static inline bool kwatch_probe_is_muted(void)
+{
+ return __this_cpu_read(kwatch_probe_cpu_muted);
+}
+
+enum kwatch_probe_position {
+ KWATCH_PROBE_POSITION_ENTRY,
+ KWATCH_PROBE_POSITION_ACTIVE,
+ KWATCH_PROBE_POSITION_EXIT
+};
+
+static bool kwatch_tsk_ctx_check(enum kwatch_probe_position pos)
+{
+ struct kwatch_tsk_ctx *ctx = kwatch_tsk_ctx_get(true);
+ u32 epoch;
+
+ if (unlikely(!ctx))
+ return false;
+
+ /* Pairs with smp_store_release() in kwatch_probe_start/stop() */
+ epoch = smp_load_acquire(&kwatch_probe_ctx.epoch);
+
+ if (unlikely(ctx->epoch != epoch))
+ kwatch_tsk_ctx_reset(ctx, epoch);
+
+ if (unlikely(!epoch))
+ return false;
+
+ switch (pos) {
+ case KWATCH_PROBE_POSITION_ENTRY:
+ ctx->depth++;
+ return true;
+ case KWATCH_PROBE_POSITION_ACTIVE:
+ return true;
+ case KWATCH_PROBE_POSITION_EXIT:
+ if (unlikely(ctx->depth == 0)) {
+ kwatch_tsk_ctx_put();
+ return false;
+ }
+
+ ctx->depth--;
+ if (ctx->depth == 0) {
+ kwatch_tsk_ctx_put();
+ return false;
+ }
+ return true;
+ }
+ return false;
+}
+
+static int kwatch_activate_handler(struct kprobe *p, struct pt_regs *regs)
+{
+ struct kwatch_tsk_ctx *ctx = kwatch_tsk_ctx_get(false);
+ unsigned long watch_addr;
+ u16 watch_len;
+
+ if (unlikely(!ctx))
+ return 0;
+
+ if (unlikely(kwatch_probe_is_muted()))
+ return 0;
+
+ if (unlikely(!kwatch_tsk_ctx_check(KWATCH_PROBE_POSITION_ACTIVE)))
+ return 0;
+
+ if (ctx->depth != kwatch_probe_ctx.cfg->depth + 1 || ctx->wp)
+ return 0;
+
+ if (kwatch_deref_resolve(kwatch_probe_ctx.cfg, regs, &watch_addr,
+ &watch_len))
+ return 0;
+
+ if (kwatch_hwbp_get(&ctx->wp))
+ return 0;
+
+ kwatch_hwbp_arm(ctx->wp, watch_addr, watch_len);
+ return 0;
+}
+
+static int kwatch_lifecycle_entry(struct kretprobe_instance *ri,
+ struct pt_regs *regs)
+{
+ /*
+ * Single policy point: the target function's context is judged once
+ * here. A rejected invocation never increments depth, so the offset
+ * kprobe path inherits the verdict through the depth check.
+ */
+ if (unlikely(kwatch_probed_ctx_in_nmi(kwatch_probe_ctx.rp_via_int3))) {
+ atomic_long_inc(&kwatch_nmi_rejected);
+ return 1; /* NMI context is unsupported: no window, no return hook */
+ }
+
+ if (!kwatch_tsk_ctx_check(KWATCH_PROBE_POSITION_ENTRY))
+ return 0;
+
+ if (kwatch_probe_ctx.cfg->func_offset == 0)
+ kwatch_activate_handler(NULL, regs);
+
+ return 0;
+}
+
+static int kwatch_lifecycle_exit(struct kretprobe_instance *ri,
+ struct pt_regs *regs)
+{
+ struct kwatch_tsk_ctx *ctx = kwatch_tsk_ctx_get(false);
+
+ if (unlikely(!ctx))
+ return 0;
+
+ if (!kwatch_tsk_ctx_check(KWATCH_PROBE_POSITION_EXIT))
+ return 0;
+
+ if (ctx->depth == kwatch_probe_ctx.cfg->depth) {
+ struct kwatch_watchpoint *wp = xchg(&ctx->wp, NULL);
+
+ if (wp)
+ kwatch_hwbp_put(wp);
+ }
+
+ return 0;
+}
+
+int kwatch_probe_start(struct kwatch_config *cfg)
+{
+ static u32 next_epoch;
+ u32 current_epoch;
+ int ret;
+
+ /*
+ * Lockless check to prevent concurrent starts. Strictly serialized
+ * by the control plane mutex, but serves as a sanity check.
+ */
+ if (smp_load_acquire(&kwatch_probe_ctx.epoch) != 0)
+ return -EBUSY;
+
+ memset(&kwatch_probe_ctx, 0, sizeof(kwatch_probe_ctx));
+ kwatch_probe_ctx.cfg = cfg;
+
+ /*
+ * Pin the entry probepoint before the kretprobe registers, so its
+ * delivery (int3 vs ftrace) can never change under jump optimization.
+ * register_kretprobe() clears kp.post_handler, hence the companion.
+ */
+ kwatch_probe_ctx.pin_kp.symbol_name = cfg->func_name;
+ kwatch_probe_ctx.pin_kp.post_handler = kwatch_pin_post_handler;
+ ret = register_kprobe(&kwatch_probe_ctx.pin_kp);
+ if (ret < 0)
+ return ret;
+
+ kwatch_probe_ctx.rp.entry_handler = kwatch_lifecycle_entry;
+ kwatch_probe_ctx.rp.handler = kwatch_lifecycle_exit;
+ kwatch_probe_ctx.rp.kp.symbol_name = cfg->func_name;
+
+ ret = register_kretprobe(&kwatch_probe_ctx.rp);
+ if (ret < 0) {
+ unregister_kprobe(&kwatch_probe_ctx.pin_kp);
+ return ret;
+ }
+ kwatch_probe_ctx.rp_via_int3 = !kprobe_ftrace(&kwatch_probe_ctx.rp.kp);
+
+ if (cfg->func_offset) {
+ kwatch_probe_ctx.kp.symbol_name = cfg->func_name;
+ kwatch_probe_ctx.kp.offset = cfg->func_offset;
+ kwatch_probe_ctx.kp.pre_handler = kwatch_activate_handler;
+
+ ret = register_kprobe(&kwatch_probe_ctx.kp);
+ if (ret) {
+ unregister_kretprobe(&kwatch_probe_ctx.rp);
+ unregister_kprobe(&kwatch_probe_ctx.pin_kp);
+ return ret;
+ }
+ }
+
+ current_epoch = ++next_epoch;
+ if (unlikely(!current_epoch))
+ current_epoch = ++next_epoch;
+
+ /* Pairs with smp_load_acquire() in kwatch_tsk_ctx_check() */
+ smp_store_release(&kwatch_probe_ctx.epoch, current_epoch);
+
+ return 0;
+}
+
+void kwatch_probe_stop(void)
+{
+ if (!kwatch_probe_ctx.epoch)
+ return;
+
+ /* Pairs with smp_load_acquire() in kwatch_tsk_ctx_check() */
+ smp_store_release(&kwatch_probe_ctx.epoch, 0);
+
+ if (kwatch_probe_ctx.cfg->func_offset > 0)
+ unregister_kprobe(&kwatch_probe_ctx.kp);
+
+ unregister_kretprobe(&kwatch_probe_ctx.rp);
+ unregister_kprobe(&kwatch_probe_ctx.pin_kp);
+}
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 09/13] mm/kwatch: add probe lifecycle runtime
2026-07-14 18:32 ` [RFC PATCH 09/13] mm/kwatch: add probe lifecycle runtime Jinchao Wang
@ 2026-07-14 19:53 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-07-14 19:53 UTC (permalink / raw)
To: Jinchao Wang; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Concurrent hardware breakpoint hits during kwatch_probe_start() cause a NULL pointer dereference in kwatch_probe_validate_hit().
- [High] Context pool slots are permanently leaked if a task hits a kwatch probe while `kwatch_probe_ctx.epoch == 0`.
- [High] The logic to determine if the probe is using int3 falsely evaluates to `false` for ftrace-enabled functions, rejecting normal executions.
- [High] Dangling `task_struct` pointers in `kwatch_ctx_pool` cause ABA aliasing and permanent hardware watchpoint leaks when a traced task terminates abnormally.
- [Medium] The NMI context detection relies on an x86-specific assumption about kprobe traps, which fails to reject NMI contexts on architectures like ARM64.
--
--- Patch [9]: [RFC PATCH 09/13] mm/kwatch: add probe lifecycle runtime ---
commit a56d8337b9a7da3aea3294307119403cdb9289c9
Author: Jinchao Wang <wangjinchao600@gmail.com>
Subject: mm/kwatch: add probe lifecycle runtime
This commit adds the core kwatch probe lifecycle runtime, tracking per-task
nesting depth through entry and exit kretprobes to open and close watch
windows. It includes an epoch versioning scheme to invalidate stale
per-task state and counts rejections for functions in NMI-like contexts.
> diff --git a/mm/kwatch/probe.c b/mm/kwatch/probe.c
> new file mode 100644
> index 0000000000000..af6e0af45c10c
> --- /dev/null
> +++ b/mm/kwatch/probe.c
> @@ -0,0 +1,263 @@
[ ... ]
> +/*
> + * True if the probed function itself runs in an NMI-like context.
> + * int3-based kprobe delivery adds one NMI-like layer of its own;
> + * delivery is pinned at registration so the subtraction stays exact.
> + */
> +static bool kwatch_probed_ctx_in_nmi(bool via_int3)
> +{
> + return (preempt_count() & NMI_MASK) > (via_int3 ? NMI_OFFSET : 0);
> +}
[Severity: Medium]
Does this NMI context check rely on x86-specific kprobe trap behavior?
On architectures like ARM64, the brk trap does not add an NMI_OFFSET to the
preempt count. If the probe is hit inside a genuine NMI on these architectures,
could this evaluate to false and fail to reject the NMI context?
[ ... ]
> +bool kwatch_probe_validate_hit(struct pt_regs *regs,
> + struct task_struct *arm_tsk)
> +{
> + struct kwatch_tsk_ctx *ctx = kwatch_tsk_ctx_get(false);
> +
> + if (unlikely(!ctx))
> + return true;
> +
> + if (arm_tsk != current ||
> + ctx->depth != kwatch_probe_ctx.cfg->depth + 1)
[Severity: High]
Can this cause a NULL pointer dereference if a hardware watchpoint is hit
concurrently during kwatch reconfiguration?
In kwatch_probe_start(), kwatch_probe_ctx is cleared:
memset(&kwatch_probe_ctx, 0, sizeof(kwatch_probe_ctx));
This temporarily sets cfg to NULL. Since stopping a kwatch session
unregisters kprobes but might not synchronously disarm active hardware
watchpoints, could a thread hit an active watchpoint, invoke
kwatch_probe_validate_hit(), and attempt to read kwatch_probe_ctx.cfg->depth
while cfg is NULL?
> + return true;
> +
> + return false;
> +}
[ ... ]
> +static bool kwatch_tsk_ctx_check(enum kwatch_probe_position pos)
> +{
> + struct kwatch_tsk_ctx *ctx = kwatch_tsk_ctx_get(true);
> + u32 epoch;
> +
> + if (unlikely(!ctx))
> + return false;
> +
> + /* Pairs with smp_store_release() in kwatch_probe_start/stop() */
> + epoch = smp_load_acquire(&kwatch_probe_ctx.epoch);
> +
> + if (unlikely(ctx->epoch != epoch))
> + kwatch_tsk_ctx_reset(ctx, epoch);
> +
> + if (unlikely(!epoch))
> + return false;
[Severity: High]
Does this early return cause a permanent context pool slot leak?
If a task enters or exits the kprobe while the global epoch is temporarily 0
during kwatch_probe_start() or kwatch_probe_stop(), this check bypasses the
KWATCH_PROBE_POSITION_EXIT state machine.
Because kwatch_tsk_ctx_put() is completely skipped when epoch is 0, does the
context slot remain permanently allocated to the thread, potentially leading
to a denial of service if the pool is exhausted?
> +
> + switch (pos) {
[ ... ]
> +static int kwatch_lifecycle_exit(struct kretprobe_instance *ri,
> + struct pt_regs *regs)
> +{
> + struct kwatch_tsk_ctx *ctx = kwatch_tsk_ctx_get(false);
> +
> + if (unlikely(!ctx))
> + return 0;
> +
> + if (!kwatch_tsk_ctx_check(KWATCH_PROBE_POSITION_EXIT))
> + return 0;
> +
> + if (ctx->depth == kwatch_probe_ctx.cfg->depth) {
> + struct kwatch_watchpoint *wp = xchg(&ctx->wp, NULL);
> +
> + if (wp)
> + kwatch_hwbp_put(wp);
> + }
> +
> + return 0;
> +}
[Severity: High]
Are context slots and active watchpoints permanently leaked if the probed
task terminates abnormally?
If a task receives a fatal signal while inside the probed function, do_exit()
flushes the kretprobe without calling kwatch_lifecycle_exit(), meaning this
exclusive cleanup path is bypassed.
Since kwatch_tsk_ctx_get() uses the raw task_struct pointer as a lookup key,
could a new task that happens to reuse the freed task_struct memory address
erroneously inherit the corrupted context state and active watchpoint?
[ ... ]
> +int kwatch_probe_start(struct kwatch_config *cfg)
> +{
> + static u32 next_epoch;
> + u32 current_epoch;
> + int ret;
[ ... ]
> + kwatch_probe_ctx.rp.entry_handler = kwatch_lifecycle_entry;
> + kwatch_probe_ctx.rp.handler = kwatch_lifecycle_exit;
> + kwatch_probe_ctx.rp.kp.symbol_name = cfg->func_name;
> +
> + ret = register_kretprobe(&kwatch_probe_ctx.rp);
> + if (ret < 0) {
> + unregister_kprobe(&kwatch_probe_ctx.pin_kp);
> + return ret;
> + }
> + kwatch_probe_ctx.rp_via_int3 = !kprobe_ftrace(&kwatch_probe_ctx.rp.kp);
[Severity: High]
Does this logic incorrectly evaluate to false for ftrace-enabled functions?
The child kprobe (rp.kp) retains KPROBE_FLAG_FTRACE even when the aggregate
probe is forced to use INT3 due to the companion probe's post_handler.
Because rp_via_int3 becomes false, does the trap's artificial NMI increment
make kwatch_probed_ctx_in_nmi() always evaluate to true, falsely identifying
normal contexts as NMIs and rejecting all valid executions?
> +
> + if (cfg->func_offset) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714182243.10687-1-wangjinchao600@gmail.com?part=9
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 10/13] mm/kwatch: add anchor thread for global watchpoints
2026-07-14 18:22 [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption Jinchao Wang
` (8 preceding siblings ...)
2026-07-14 18:32 ` [RFC PATCH 09/13] mm/kwatch: add probe lifecycle runtime Jinchao Wang
@ 2026-07-14 18:32 ` Jinchao Wang
2026-07-14 18:48 ` sashiko-bot
2026-07-14 18:33 ` [RFC PATCH 11/13] mm/kwatch: add debugfs control plane Jinchao Wang
` (2 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Jinchao Wang @ 2026-07-14 18:32 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
Global variables have no function whose execution can bound the
watch window. Provide one: a kernel thread sleeps for the configured
duration inside a dedicated noinline function,
kwatch_global_anchor(), and the probe runtime hooks that function
like any other target.
When the duration expires the thread schedules a work item that
tears the session down; the expired flag is cleared under the
control-plane mutex so a stale work item from a previous session
cannot stop a new one.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
mm/kwatch/Makefile | 2 +-
mm/kwatch/anchor.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 1 deletion(-)
create mode 100644 mm/kwatch/anchor.c
diff --git a/mm/kwatch/Makefile b/mm/kwatch/Makefile
index f04673cc5b1c..b196c794619a 100644
--- a/mm/kwatch/Makefile
+++ b/mm/kwatch/Makefile
@@ -1,3 +1,3 @@
obj-$(CONFIG_KWATCH) += kwatch.o
-kwatch-y := deref.o task_ctx.o hwbp.o probe.o
+kwatch-y := deref.o task_ctx.o hwbp.o probe.o anchor.o
diff --git a/mm/kwatch/anchor.c b/mm/kwatch/anchor.c
new file mode 100644
index 000000000000..11da6aff9413
--- /dev/null
+++ b/mm/kwatch/anchor.c
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kthread.h>
+#include <linux/wait.h>
+#include <linux/jiffies.h>
+#include <linux/err.h>
+#include <linux/workqueue.h>
+
+#include "kwatch.h"
+
+static DECLARE_WAIT_QUEUE_HEAD(kwatch_anchor_wq);
+static struct task_struct *kwatch_anchor_tsk;
+static bool kwatch_anchor_expired;
+
+bool kwatch_anchor_has_expired(void)
+{
+ return READ_ONCE(kwatch_anchor_expired);
+}
+
+void kwatch_anchor_clear_expired(void)
+{
+ WRITE_ONCE(kwatch_anchor_expired, false);
+}
+
+static void kwatch_auto_stop_handler(struct work_struct *work)
+{
+ kwatch_auto_stop();
+}
+
+static DECLARE_WORK(kwatch_auto_stop_work, kwatch_auto_stop_handler);
+
+noinline void kwatch_global_anchor(unsigned long duration_sec)
+{
+ wait_event_timeout(kwatch_anchor_wq, kthread_should_stop(),
+ duration_sec * HZ);
+}
+
+static int kwatch_anchor_thread_fn(void *data)
+{
+ unsigned long duration = (unsigned long)data;
+
+ kwatch_global_anchor(duration);
+
+ if (!kthread_should_stop()) {
+ /* mark before scheduling; cleared under the control mutex */
+ WRITE_ONCE(kwatch_anchor_expired, true);
+ schedule_work(&kwatch_auto_stop_work);
+ }
+
+ while (!kthread_should_stop())
+ schedule_timeout_uninterruptible(HZ);
+
+ return 0;
+}
+
+int kwatch_anchor_start(u16 duration)
+{
+ kwatch_anchor_tsk = kthread_run(kwatch_anchor_thread_fn,
+ (void *)(unsigned long)duration,
+ "kwatch_anchor");
+ if (IS_ERR(kwatch_anchor_tsk)) {
+ int ret = PTR_ERR(kwatch_anchor_tsk);
+
+ kwatch_anchor_tsk = NULL;
+ return ret;
+ }
+ return 0;
+}
+
+void kwatch_anchor_stop(void)
+{
+ if (kwatch_anchor_tsk) {
+ kthread_stop(kwatch_anchor_tsk);
+ kwatch_anchor_tsk = NULL;
+ }
+}
+
+void kwatch_anchor_cancel_work(void)
+{
+ cancel_work_sync(&kwatch_auto_stop_work);
+}
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 10/13] mm/kwatch: add anchor thread for global watchpoints
2026-07-14 18:32 ` [RFC PATCH 10/13] mm/kwatch: add anchor thread for global watchpoints Jinchao Wang
@ 2026-07-14 18:48 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-07-14 18:48 UTC (permalink / raw)
To: Jinchao Wang; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The anchor thread sleeps in TASK_UNINTERRUPTIBLE, artificially inflating the system load average and risking hung task panics.
- [Medium] The thread's polling loop violates voluntary sleeping rules by checking the exit condition before setting the task state, leading to a lost wakeup.
- [Low] The function `kwatch_auto_stop()` is called in the work handler but is never implemented in the patch, leading to a build failure.
--
--- Patch [10]: [RFC PATCH 10/13] mm/kwatch: add anchor thread for global watchpoints ---
commit e6c643e7bd2882c0ca1793a1b13c96d1e132942e
Author: Jinchao Wang <wangjinchao600@gmail.com>
mm/kwatch: add anchor thread for global watchpoints
Global variables have no function whose execution can bound the
watch window. Provide one: a kernel thread sleeps for the configured
duration inside a dedicated noinline function, kwatch_global_anchor(),
and the probe runtime hooks that function like any other target.
> diff --git a/mm/kwatch/anchor.c b/mm/kwatch/anchor.c
> new file mode 100644
> index 0000000000000..11da6aff94133
> --- /dev/null
> +++ b/mm/kwatch/anchor.c
> @@ -0,0 +1,82 @@
[ ... ]
> +static void kwatch_auto_stop_handler(struct work_struct *work)
> +{
> + kwatch_auto_stop();
[Severity: Low]
Does this cause a build failure? This function appears to be referenced
here but isn't implemented in this patch, which could lead to an undefined
symbol linker error and break bisectability.
> +}
> +
> +static DECLARE_WORK(kwatch_auto_stop_work, kwatch_auto_stop_handler);
> +
> +noinline void kwatch_global_anchor(unsigned long duration_sec)
> +{
> + wait_event_timeout(kwatch_anchor_wq, kthread_should_stop(),
> + duration_sec * HZ);
[Severity: High]
Does this use of wait_event_timeout risk triggering the hung task
watchdog? Because wait_event_timeout implicitly uses TASK_UNINTERRUPTIBLE,
long watch durations could trigger a panic (if the watchdog is configured)
and permanently inflate the system load average by 1.0 during the session.
> +}
> +
> +static int kwatch_anchor_thread_fn(void *data)
> +{
> + unsigned long duration = (unsigned long)data;
> +
> + kwatch_global_anchor(duration);
> +
> + if (!kthread_should_stop()) {
> + /* mark before scheduling; cleared under the control mutex */
> + WRITE_ONCE(kwatch_anchor_expired, true);
> + schedule_work(&kwatch_auto_stop_work);
> + }
> +
> + while (!kthread_should_stop())
> + schedule_timeout_uninterruptible(HZ);
[Severity: High]
Can this loop also artificially inflate the load average? Sleeping
in TASK_UNINTERRUPTIBLE here will add to the system load while waiting
for teardown.
[Severity: Medium]
Could this sequence result in a lost wakeup? The stop condition is checked
before the task state is set. If kthread_stop() signals the thread
after the while condition is evaluated but before
schedule_timeout_uninterruptible() sets the state to TASK_UNINTERRUPTIBLE,
the thread might sleep for an unnecessary second before observing the stop
flag.
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714182243.10687-1-wangjinchao600@gmail.com?part=10
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 11/13] mm/kwatch: add debugfs control plane
2026-07-14 18:22 [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption Jinchao Wang
` (9 preceding siblings ...)
2026-07-14 18:32 ` [RFC PATCH 10/13] mm/kwatch: add anchor thread for global watchpoints Jinchao Wang
@ 2026-07-14 18:33 ` Jinchao Wang
2026-07-14 18:58 ` sashiko-bot
2026-07-14 18:33 ` [RFC PATCH 12/13] mm/kwatch: add KUnit tests for the watch expression parser Jinchao Wang
2026-07-14 18:33 ` [RFC PATCH 13/13] Documentation/dev-tools: document KWatch Jinchao Wang
12 siblings, 1 reply; 26+ messages in thread
From: Jinchao Wang @ 2026-07-14 18:33 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
Wire the pieces together behind a single debugfs file,
/sys/kernel/debug/kwatch/config. Writing a key=value configuration
string stops any active session and starts a new one; reading shows
the active configuration and the nmi_rejected counter. An open-count
guard keeps the file single-open and a mutex serializes
start/stop/auto-stop against each other.
Add the Kconfig entry and hook mm/kwatch into the mm build. KWatch
can be built in or as a module; symbol-name watch expressions need
the built-in flavour (kallsyms_lookup_name is not exported).
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
MAINTAINERS | 8 ++
mm/Kconfig | 1 +
mm/Makefile | 1 +
mm/kwatch/Kconfig | 17 +++
mm/kwatch/Makefile | 2 +-
mm/kwatch/core.c | 325 +++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 353 insertions(+), 1 deletion(-)
create mode 100644 mm/kwatch/Kconfig
create mode 100644 mm/kwatch/core.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 7cc4bca5a2c5..b6371f92fe5c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14578,6 +14578,14 @@ S: Supported
T: git git://git.kernel.org/pub/scm/virt/kvm/kvm.git
F: arch/x86/kvm/xen.*
+KWATCH
+M: Jinchao Wang <wangjinchao600@gmail.com>
+L: linux-mm@kvack.org
+S: Maintained
+F: Documentation/dev-tools/kwatch.rst
+F: include/trace/events/kwatch.h
+F: mm/kwatch/
+
L3MDEV
M: David Ahern <dsahern@kernel.org>
L: netdev@vger.kernel.org
diff --git a/mm/Kconfig b/mm/Kconfig
index 9e0ca4824905..cac75a46e21a 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -1510,5 +1510,6 @@ config LAZY_MMU_MODE_KUNIT_TEST
If unsure, say N.
source "mm/damon/Kconfig"
+source "mm/kwatch/Kconfig"
endmenu
diff --git a/mm/Makefile b/mm/Makefile
index eff9f9e7e061..80c688330358 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -92,6 +92,7 @@ obj-$(CONFIG_PAGE_POISONING) += page_poison.o
obj-$(CONFIG_KASAN) += kasan/
obj-$(CONFIG_KFENCE) += kfence/
obj-$(CONFIG_KMSAN) += kmsan/
+obj-$(CONFIG_KWATCH) += kwatch/
obj-$(CONFIG_FAILSLAB) += failslab.o
obj-$(CONFIG_FAIL_PAGE_ALLOC) += fail_page_alloc.o
obj-$(CONFIG_MEMTEST) += memtest.o
diff --git a/mm/kwatch/Kconfig b/mm/kwatch/Kconfig
new file mode 100644
index 000000000000..b1c37a829dd5
--- /dev/null
+++ b/mm/kwatch/Kconfig
@@ -0,0 +1,17 @@
+config KWATCH
+ tristate "Kernel Watch Framework"
+ depends on PERF_EVENTS && HAVE_HW_BREAKPOINT && DEBUG_FS
+ depends on HAVE_REINSTALL_HW_BREAKPOINT
+ select KPROBES
+ select KRETPROBES
+ select STACKTRACE
+ help
+ A generalized hardware-assisted memory monitor utility.
+ It provides a low-overhead, real-time trigger mechanism to monitor
+ kernel memory safely in atomic contexts using hardware breakpoints.
+
+ KWatch is designed to catch silent memory corruptions, stack
+ overwrites, and complex Heisenbugs by synchronously trapping the
+ exact instruction causing the illegal access.
+
+ If unsure, say N.
diff --git a/mm/kwatch/Makefile b/mm/kwatch/Makefile
index b196c794619a..02d7917602f1 100644
--- a/mm/kwatch/Makefile
+++ b/mm/kwatch/Makefile
@@ -1,3 +1,3 @@
obj-$(CONFIG_KWATCH) += kwatch.o
-kwatch-y := deref.o task_ctx.o hwbp.o probe.o anchor.o
+kwatch-y := core.o deref.o task_ctx.o hwbp.o probe.o anchor.o
diff --git a/mm/kwatch/core.c b/mm/kwatch/core.c
new file mode 100644
index 000000000000..548d0cdd0812
--- /dev/null
+++ b/mm/kwatch/core.c
@@ -0,0 +1,325 @@
+// SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kstrtox.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/atomic.h>
+#include <linux/debugfs.h>
+#include <linux/mutex.h>
+#include "kwatch.h"
+
+static struct kwatch_config kwatch_config;
+static bool watching_active;
+
+static struct dentry *dbgfs_dir;
+static struct dentry *dbgfs_config;
+static DEFINE_MUTEX(kwatch_dbgfs_mutex);
+static atomic_t dbgfs_config_busy = ATOMIC_INIT(0);
+
+static int kwatch_start_watching(void)
+{
+ int ret;
+
+ if (!strlen(kwatch_config.func_name)) {
+ if (kwatch_config.duration > 0) {
+ strscpy(kwatch_config.func_name, "kwatch_global_anchor",
+ sizeof(kwatch_config.func_name));
+ } else {
+ pr_err("func_name or duration is required\n");
+ return -EINVAL;
+ }
+ } else if (kwatch_config.duration > 0 &&
+ strcmp(kwatch_config.func_name, "kwatch_global_anchor")) {
+ pr_warn("duration is ignored when watching a specific function\n");
+ }
+
+ if (kwatch_config.access_type > 3) {
+ pr_err("Invalid access_type (must be 0-3)\n");
+ return -EINVAL;
+ }
+
+ ret = kwatch_hwbp_prealloc(kwatch_config.max_watch,
+ kwatch_config.access_type);
+ if (ret) {
+ pr_err("kwatch_hwbp_prealloc ret: %d\n", ret);
+ return ret;
+ }
+
+ ret = kwatch_tsk_ctx_prealloc(kwatch_config.max_concurrency);
+ if (ret) {
+ kwatch_hwbp_free();
+ return ret;
+ }
+
+ ret = kwatch_probe_start(&kwatch_config);
+ if (ret) {
+ pr_err("kwatch_probe_start ret: %d\n", ret);
+ kwatch_tsk_ctx_free();
+ kwatch_hwbp_free();
+ return ret;
+ }
+
+ if (!strcmp(kwatch_config.func_name, "kwatch_global_anchor")) {
+ ret = kwatch_anchor_start(kwatch_config.duration);
+ if (ret) {
+ kwatch_probe_stop();
+ synchronize_rcu();
+ kwatch_tsk_ctx_release_wps();
+ kwatch_hwbp_free();
+ kwatch_tsk_ctx_free();
+ return ret;
+ }
+ }
+
+ watching_active = true;
+ return 0;
+}
+
+static void kwatch_stop_watching(void)
+{
+ watching_active = false;
+
+ kwatch_anchor_stop();
+ /* after kthread_stop: the dead thread cannot re-mark expiry */
+ kwatch_anchor_clear_expired();
+
+ kwatch_probe_stop();
+ synchronize_rcu();
+ kwatch_tsk_ctx_release_wps();
+ /*
+ * Waits for disarm IPIs and unregisters breakpoints: no #DB can
+ * reach the ctx pool once this returns.
+ */
+ kwatch_hwbp_free();
+ kwatch_tsk_ctx_free();
+}
+
+void kwatch_auto_stop(void)
+{
+ mutex_lock(&kwatch_dbgfs_mutex);
+ /* the expired check neutralizes work items from torn-down sessions */
+ if (watching_active && kwatch_anchor_has_expired()) {
+ kwatch_stop_watching();
+ pr_info("watch duration expired, stopped watching\n");
+ }
+ mutex_unlock(&kwatch_dbgfs_mutex);
+}
+
+static int kwatch_config_parse(char *buf, struct kwatch_config *cfg)
+{
+ char *token, *key, *val;
+ int ret = 0;
+
+ memset(cfg, 0, sizeof(*cfg));
+ cfg->max_concurrency = 256;
+ cfg->max_watch = 4;
+ cfg->watch_len = 8;
+ cfg->access_type = 0;
+
+ while ((token = strsep(&buf, " \t\n")) != NULL) {
+ if (!*token)
+ continue;
+ key = strsep(&token, "=");
+ val = token;
+ if (!key || !val)
+ return -EINVAL;
+
+ if (!strcmp(key, "func_name")) {
+ strscpy(cfg->func_name, val, sizeof(cfg->func_name));
+ } else if (!strcmp(key, "func_offset")) {
+ ret = kstrtou16(val, 0, &cfg->func_offset);
+ } else if (!strcmp(key, "depth")) {
+ ret = kstrtou16(val, 0, &cfg->depth);
+ } else if (!strcmp(key, "max_concurrency")) {
+ ret = kstrtou16(val, 0, &cfg->max_concurrency);
+ } else if (!strcmp(key, "max_watch")) {
+ ret = kstrtou16(val, 0, &cfg->max_watch);
+ } else if (!strcmp(key, "access_type")) {
+ ret = kstrtouint(val, 0, &cfg->access_type);
+ } else if (!strcmp(key, "watch_len")) {
+ ret = kstrtou16(val, 0, &cfg->watch_len);
+ if (!ret && cfg->watch_len != 1 &&
+ cfg->watch_len != 2 && cfg->watch_len != 4 &&
+ cfg->watch_len != 8)
+ ret = -EINVAL;
+ } else if (!strcmp(key, "duration")) {
+ ret = kstrtou16(val, 0, &cfg->duration);
+ } else if (!strcmp(key, "watch_expr")) {
+ strscpy(cfg->watch_expr, val, sizeof(cfg->watch_expr));
+ ret = kwatch_deref_parse(cfg, val);
+ }
+
+ if (ret)
+ return ret;
+ }
+ return 0;
+}
+
+static int kwatch_dbgfs_open(struct inode *inode, struct file *file)
+{
+ if (atomic_cmpxchg(&dbgfs_config_busy, 0, 1))
+ return -EBUSY;
+ return 0;
+}
+
+static int kwatch_dbgfs_release(struct inode *inode, struct file *file)
+{
+ atomic_set(&dbgfs_config_busy, 0);
+ return 0;
+}
+
+static ssize_t kwatch_dbgfs_read(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ char *out_buf;
+ size_t len = 0;
+ ssize_t ret;
+
+ out_buf = kzalloc(MAX_CONFIG_STR_LEN, GFP_KERNEL);
+ if (!out_buf)
+ return -ENOMEM;
+
+ if (watching_active) {
+ len += scnprintf(out_buf + len, MAX_CONFIG_STR_LEN - len,
+ "func_name=%s\n"
+ "func_offset=%u\n"
+ "depth=%u\n"
+ "duration=%u\n"
+ "max_concurrency=%u\n"
+ "max_watch=%u\n"
+ "access_type=%u\n"
+ "watch_len=%u\n",
+ kwatch_config.func_name,
+ kwatch_config.func_offset, kwatch_config.depth,
+ kwatch_config.duration,
+ kwatch_config.max_concurrency,
+ kwatch_config.max_watch,
+ kwatch_config.access_type,
+ kwatch_config.watch_len);
+
+ if (kwatch_config.base == KWATCH_BASE_GLOBAL_SYM) {
+ len += scnprintf(out_buf + len, MAX_CONFIG_STR_LEN - len,
+ "sym_addr=0x%lx\n", kwatch_config.sym_addr);
+ }
+
+ len += scnprintf(out_buf + len, MAX_CONFIG_STR_LEN - len,
+ "watch_expr=%s\n"
+ "nmi_rejected=%lu\n",
+ kwatch_config.watch_expr,
+ kwatch_probe_nmi_rejected());
+ } else {
+ len = scnprintf(out_buf, MAX_CONFIG_STR_LEN, "not watching\n");
+ }
+
+ ret = simple_read_from_buffer(user_buf, count, ppos, out_buf, len);
+ kfree(out_buf);
+ return ret;
+}
+
+static ssize_t kwatch_dbgfs_write(struct file *file, const char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ char *input_alloc;
+ char *parse_str;
+ int ret;
+
+ if (count == 0 || count >= MAX_CONFIG_STR_LEN)
+ return -EINVAL;
+
+ input_alloc = memdup_user_nul(buffer, count);
+ if (IS_ERR(input_alloc))
+ return PTR_ERR(input_alloc);
+
+ mutex_lock(&kwatch_dbgfs_mutex);
+
+ if (watching_active)
+ kwatch_stop_watching();
+
+ parse_str = strim(input_alloc);
+
+ if (!strlen(parse_str)) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = kwatch_config_parse(parse_str, &kwatch_config);
+ if (ret) {
+ pr_err("Failed to parse config %d\n", ret);
+ goto out;
+ }
+
+ ret = kwatch_start_watching();
+ if (ret) {
+ pr_err("Failed to start watching with %d\n", ret);
+ goto out;
+ }
+
+ ret = count;
+
+out:
+ mutex_unlock(&kwatch_dbgfs_mutex);
+ kfree(input_alloc);
+ return ret;
+}
+
+static const struct file_operations kwatch_fops = {
+ .owner = THIS_MODULE,
+ .open = kwatch_dbgfs_open,
+ .release = kwatch_dbgfs_release,
+ .read = kwatch_dbgfs_read,
+ .write = kwatch_dbgfs_write,
+};
+
+static int __init kwatch_init(void)
+{
+ int ret = 0;
+
+ memset(&kwatch_config, 0, sizeof(kwatch_config));
+
+ dbgfs_dir = debugfs_create_dir("kwatch", NULL);
+ if (IS_ERR(dbgfs_dir)) {
+ ret = PTR_ERR(dbgfs_dir);
+ goto err_dir;
+ }
+
+ dbgfs_config = debugfs_create_file("config", 0600, dbgfs_dir, NULL,
+ &kwatch_fops);
+ if (IS_ERR(dbgfs_config)) {
+ ret = PTR_ERR(dbgfs_config);
+ goto err_file;
+ }
+
+ pr_info("module loaded\n");
+ return 0;
+
+err_file:
+ debugfs_remove_recursive(dbgfs_dir);
+ dbgfs_dir = NULL;
+err_dir:
+ return ret;
+}
+module_init(kwatch_init);
+
+static void __exit kwatch_exit(void)
+{
+ mutex_lock(&kwatch_dbgfs_mutex);
+ if (watching_active)
+ kwatch_stop_watching();
+ mutex_unlock(&kwatch_dbgfs_mutex);
+
+ /* the anchor thread is dead: nothing can schedule new work now */
+ kwatch_anchor_cancel_work();
+
+ debugfs_remove_recursive(dbgfs_dir);
+ dbgfs_dir = NULL;
+
+ pr_info("kwatch unloaded\n");
+}
+module_exit(kwatch_exit);
+
+MODULE_AUTHOR("Jinchao Wang <wangjinchao600@gmail.com>");
+MODULE_DESCRIPTION("Kernel watchpoint");
+MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 11/13] mm/kwatch: add debugfs control plane
2026-07-14 18:33 ` [RFC PATCH 11/13] mm/kwatch: add debugfs control plane Jinchao Wang
@ 2026-07-14 18:58 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-07-14 18:58 UTC (permalink / raw)
To: Jinchao Wang; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Tasks that exit or are killed while inside a probed function permanently leak their kwatch_ctx_pool slot and leave their hardware breakpoint globally armed.
- [High] Missing kernel lockdown check allows root to bypass confidentiality restrictions by setting hardware breakpoints on arbitrary kernel addresses.
- [Medium] Lockless access to the global `kwatch_config` and `watching_active` variables in `kwatch_dbgfs_read` causes data races and torn reads.
- [Low] The KWATCH config entry selects KPROBES, KRETPROBES, and STACKTRACE without inheriting their architecture-specific dependencies, causing unmet dependency warnings.
--
--- Patch [11]: [RFC PATCH 11/13] mm/kwatch: add debugfs control plane ---
commit fe07df22dbe76645eaac13880523515d1694aec8
Author: Jinchao Wang <wangjinchao600@gmail.com>
mm/kwatch: add debugfs control plane
Wire the pieces together behind a single debugfs file,
/sys/kernel/debug/kwatch/config. Writing a key=value configuration
string stops any active session and starts a new one; reading shows
the active configuration and the nmi_rejected counter. An open-count
guard keeps the file single-open and a mutex serializes
start/stop/auto-stop against each other.
> diff --git a/mm/kwatch/Kconfig b/mm/kwatch/Kconfig
> new file mode 100644
> index 0000000000000..b1c37a829dd53
> --- /dev/null
> +++ b/mm/kwatch/Kconfig
> @@ -0,0 +1,17 @@
> +config KWATCH
> + tristate "Kernel Watch Framework"
> + depends on PERF_EVENTS && HAVE_HW_BREAKPOINT && DEBUG_FS
> + depends on HAVE_REINSTALL_HW_BREAKPOINT
> + select KPROBES
> + select KRETPROBES
> + select STACKTRACE
[Severity: Low]
Should this Kconfig entry inherit architecture-specific dependencies for
KPROBES and STACKTRACE?
Selecting these without depending on HAVE_KPROBES and STACKTRACE_SUPPORT
can cause unmet dependency warnings and potential build failures on
architectures that do not support them.
[ ... ]
> diff --git a/mm/kwatch/core.c b/mm/kwatch/core.c
> new file mode 100644
> index 0000000000000..548d0cdd08126
> --- /dev/null
> +++ b/mm/kwatch/core.c
> @@ -0,0 +1,325 @@
[ ... ]
> +static int kwatch_start_watching(void)
> +{
> + int ret;
[ ... ]
> + ret = kwatch_hwbp_prealloc(kwatch_config.max_watch,
> + kwatch_config.access_type);
> + if (ret) {
> + pr_err("kwatch_hwbp_prealloc ret: %d\n", ret);
> + return ret;
> + }
> +
> + ret = kwatch_tsk_ctx_prealloc(kwatch_config.max_concurrency);
> + if (ret) {
> + kwatch_hwbp_free();
> + return ret;
> + }
> +
> + ret = kwatch_probe_start(&kwatch_config);
[Severity: High]
If a task exits or is killed while inside a probed function, could this leak
the kwatch_ctx_pool slot and leave hardware breakpoints permanently armed?
When a task terminates, the kretprobe framework flushes instances without
executing the exit handlers (like kwatch_lifecycle_exit() in probe.c).
This skips the kwatch_tsk_ctx_put() call, leaving the slot occupied
and the HWBP armed until the kwatch session is manually stopped.
[ ... ]
> +static ssize_t kwatch_dbgfs_read(struct file *file, char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + char *out_buf;
> + size_t len = 0;
> + ssize_t ret;
> +
> + out_buf = kzalloc(MAX_CONFIG_STR_LEN, GFP_KERNEL);
> + if (!out_buf)
> + return -ENOMEM;
> +
> + if (watching_active) {
> + len += scnprintf(out_buf + len, MAX_CONFIG_STR_LEN - len,
[Severity: Medium]
Is it safe to access watching_active and format kwatch_config here without
holding kwatch_dbgfs_mutex?
Since concurrent writes (via kwatch_dbgfs_write) zero out kwatch_config
while holding the mutex, this lockless read could result in data races
or torn reads of the configuration state.
[ ... ]
> +static ssize_t kwatch_dbgfs_write(struct file *file, const char __user *buffer,
> + size_t count, loff_t *ppos)
> +{
> + char *input_alloc;
> + char *parse_str;
> + int ret;
[ ... ]
> + ret = kwatch_config_parse(parse_str, &kwatch_config);
> + if (ret) {
> + pr_err("Failed to parse config %d\n", ret);
> + goto out;
> + }
> +
> + ret = kwatch_start_watching();
[Severity: High]
Could a missing lockdown check here allow root to bypass confidentiality
restrictions?
Since this enables setting hardware breakpoints on arbitrary memory addresses
provided by userspace, it might allow inspecting kernel memory in violation
of LOCKDOWN_CONFIDENTIALITY. perf_event_open() normally enforces
security_locked_down() checks, but register_wide_hw_breakpoint() does not.
> + if (ret) {
> + pr_err("Failed to start watching with %d\n", ret);
> + goto out;
> + }
> +
> + ret = count;
> +
> +out:
> + mutex_unlock(&kwatch_dbgfs_mutex);
> + kfree(input_alloc);
> + return ret;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714182243.10687-1-wangjinchao600@gmail.com?part=11
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 12/13] mm/kwatch: add KUnit tests for the watch expression parser
2026-07-14 18:22 [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption Jinchao Wang
` (10 preceding siblings ...)
2026-07-14 18:33 ` [RFC PATCH 11/13] mm/kwatch: add debugfs control plane Jinchao Wang
@ 2026-07-14 18:33 ` Jinchao Wang
2026-07-14 18:50 ` sashiko-bot
2026-07-14 18:33 ` [RFC PATCH 13/13] Documentation/dev-tools: document KWatch Jinchao Wang
12 siblings, 1 reply; 26+ messages in thread
From: Jinchao Wang @ 2026-07-14 18:33 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
Cover base anchors (stack, argN, absolute address), positive and
negative offsets, dereference chains, and rejection of malformed
expressions (missing offsets, bad argument index, junk offsets).
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
mm/kwatch/.kunitconfig | 9 +++
mm/kwatch/Kconfig | 10 +++
mm/kwatch/Makefile | 1 +
mm/kwatch/deref_test.c | 137 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 157 insertions(+)
create mode 100644 mm/kwatch/.kunitconfig
create mode 100644 mm/kwatch/deref_test.c
diff --git a/mm/kwatch/.kunitconfig b/mm/kwatch/.kunitconfig
new file mode 100644
index 000000000000..7e977ddf0da1
--- /dev/null
+++ b/mm/kwatch/.kunitconfig
@@ -0,0 +1,9 @@
+CONFIG_KUNIT=y
+CONFIG_KWATCH=y
+CONFIG_KWATCH_KUNIT_TEST=y
+CONFIG_PERF_EVENTS=y
+CONFIG_HAVE_HW_BREAKPOINT=y
+CONFIG_HAVE_REINSTALL_HW_BREAKPOINT=y
+CONFIG_KPROBES=y
+CONFIG_KRETPROBES=y
+CONFIG_PRINTK=y
diff --git a/mm/kwatch/Kconfig b/mm/kwatch/Kconfig
index b1c37a829dd5..74083040a1a3 100644
--- a/mm/kwatch/Kconfig
+++ b/mm/kwatch/Kconfig
@@ -15,3 +15,13 @@ config KWATCH
exact instruction causing the illegal access.
If unsure, say N.
+
+config KWATCH_KUNIT_TEST
+ bool "KUnit tests for KWatch" if !KUNIT_ALL_TESTS
+ depends on KWATCH && KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ Enable KUnit tests for the KWatch kernel module.
+ This suite tests the core parsing logic, the pointer-chasing
+ finite state machine, and edge cases involving complex watchpoint
+ expressions. If unsure, say N.
diff --git a/mm/kwatch/Makefile b/mm/kwatch/Makefile
index 02d7917602f1..1d223d73b461 100644
--- a/mm/kwatch/Makefile
+++ b/mm/kwatch/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_KWATCH) += kwatch.o
kwatch-y := core.o deref.o task_ctx.o hwbp.o probe.o anchor.o
+kwatch-$(CONFIG_KWATCH_KUNIT_TEST) += deref_test.o
diff --git a/mm/kwatch/deref_test.c b/mm/kwatch/deref_test.c
new file mode 100644
index 000000000000..094b7afeb235
--- /dev/null
+++ b/mm/kwatch/deref_test.c
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <kunit/test.h>
+#include "kwatch.h"
+#include <linux/string.h>
+
+static void kwatch_test_parse_deref_chain(struct kunit *test)
+{
+ struct kwatch_config cfg;
+ int ret;
+
+ // Test 1: stack
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "stack");
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, cfg.base, KWATCH_BASE_STACK);
+ KUNIT_EXPECT_EQ(test, cfg.offset_count, 1);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[0], 0);
+
+ // Test 2: arg1
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "arg1");
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, cfg.base, KWATCH_BASE_ARG1);
+ KUNIT_EXPECT_EQ(test, cfg.offset_count, 1);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[0], 0);
+
+ // Test 3: arg6+8
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "arg6+8");
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, cfg.base, KWATCH_BASE_ARG6);
+ KUNIT_EXPECT_EQ(test, cfg.offset_count, 1);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[0], 8);
+
+ // Test 4: arg2-16
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "arg2-16");
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, cfg.base, KWATCH_BASE_ARG2);
+ KUNIT_EXPECT_EQ(test, cfg.offset_count, 1);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[0], -16);
+
+ // Test 5: arg3->8
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "arg3->8");
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, cfg.base, KWATCH_BASE_ARG3);
+ KUNIT_EXPECT_EQ(test, cfg.offset_count, 2);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[0], 0);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[1], 8);
+
+ // Test 6: arg4+8->16
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "arg4+8->16");
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, cfg.base, KWATCH_BASE_ARG4);
+ KUNIT_EXPECT_EQ(test, cfg.offset_count, 2);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[0], 8);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[1], 16);
+
+ // Test 7: arg5-8->-16
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "arg5-8->-16");
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, cfg.base, KWATCH_BASE_ARG5);
+ KUNIT_EXPECT_EQ(test, cfg.offset_count, 2);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[0], -8);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[1], -16);
+
+ // Test 8: stack->0->8
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "stack->0->8");
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, cfg.base, KWATCH_BASE_STACK);
+ KUNIT_EXPECT_EQ(test, cfg.offset_count, 3);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[0], 0);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[1], 0);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[2], 8);
+
+ // Test 9: arg1->+8
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "arg1->+8");
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, cfg.base, KWATCH_BASE_ARG1);
+ KUNIT_EXPECT_EQ(test, cfg.offset_count, 2);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[0], 0);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[1], 8);
+
+ // Test 9.1: arg1-> (implicit 0 should fail)
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "arg1->");
+ KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+
+ // Test 9.2: stack->->8 (implicit 0 should fail)
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "stack->->8");
+ KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+
+ // Test 10: Invalid base
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "invalid_base");
+ KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+
+ // Test 11: Invalid offset
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "arg1+abc");
+ KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+
+ // Test 12: Invalid arg
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "arg7");
+ KUNIT_EXPECT_EQ(test, ret, -EINVAL);
+
+ // Test 13: Absolute address
+ memset(&cfg, 0, sizeof(cfg));
+ ret = kwatch_deref_parse(&cfg, "0xffffffff81000000+8");
+ KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_EXPECT_EQ(test, cfg.base, KWATCH_BASE_ABS_ADDR);
+ KUNIT_EXPECT_EQ(test, cfg.sym_addr, 0xffffffff81000000UL);
+ KUNIT_EXPECT_EQ(test, cfg.offset_count, 1);
+ KUNIT_EXPECT_EQ(test, cfg.offsets[0], 8);
+}
+
+static struct kunit_case kwatch_deref_test_cases[] = {
+ KUNIT_CASE(kwatch_test_parse_deref_chain),
+ {}
+};
+
+static struct kunit_suite kwatch_deref_test_suite = {
+ .name = "kwatch_deref",
+ .test_cases = kwatch_deref_test_cases,
+};
+
+kunit_test_suite(kwatch_deref_test_suite);
+
+MODULE_DESCRIPTION("KUnit tests for the KWatch watch expression parser");
+MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 12/13] mm/kwatch: add KUnit tests for the watch expression parser
2026-07-14 18:33 ` [RFC PATCH 12/13] mm/kwatch: add KUnit tests for the watch expression parser Jinchao Wang
@ 2026-07-14 18:50 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-07-14 18:50 UTC (permalink / raw)
To: Jinchao Wang; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Hardcoded 64-bit absolute address in the KUnit test will cause compilation warnings and guaranteed runtime failures on 32-bit platforms.
- [Medium] KUnit test cannot be enabled when KWatch is a loadable module, and fixing the Kconfig exposes a multiple `module_init` build failure.
--
--- Patch [12]: [RFC PATCH 12/13] mm/kwatch: add KUnit tests for the watch expression parser ---
commit 5eda6d9ba13c286416e92d504d097afadcc1400d
Author: Jinchao Wang <wangjinchao600@gmail.com>
mm/kwatch: add KUnit tests for the watch expression parser
This commit adds a KUnit test suite for the KWatch expression parser. It
covers base anchors, offsets, dereference chains, and rejection of malformed
expressions.
> diff --git a/mm/kwatch/Kconfig b/mm/kwatch/Kconfig
> index b1c37a829dd53..74083040a1a33 100644
> --- a/mm/kwatch/Kconfig
> +++ b/mm/kwatch/Kconfig
> @@ -15,3 +15,13 @@ config KWATCH
> exact instruction causing the illegal access.
>
> If unsure, say N.
> +
> +config KWATCH_KUNIT_TEST
> + bool "KUnit tests for KWatch" if !KUNIT_ALL_TESTS
> + depends on KWATCH && KUNIT
[Severity: Medium]
Will this test be silently disabled when KWATCH is built as a loadable module?
Because CONFIG_KWATCH_KUNIT_TEST is a bool, if KWATCH is configured as a
module ('m'), the dependency expression evaluates to 'm' and Kconfig will
automatically downgrade the boolean's visibility to 'n', making it impossible
to run the test suite for module builds.
> + default KUNIT_ALL_TESTS
> + help
> + Enable KUnit tests for the KWatch kernel module.
[ ... ]
> diff --git a/mm/kwatch/deref_test.c b/mm/kwatch/deref_test.c
> new file mode 100644
> index 0000000000000..094b7afeb235e
> --- /dev/null
> +++ b/mm/kwatch/deref_test.c
> @@ -0,0 +1,137 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <kunit/test.h>
> +#include "kwatch.h"
> +#include <linux/string.h>
> +
> +static void kwatch_test_parse_deref_chain(struct kunit *test)
> +{
> + struct kwatch_config cfg;
> + int ret;
[ ... ]
> + // Test 13: Absolute address
> + memset(&cfg, 0, sizeof(cfg));
> + ret = kwatch_deref_parse(&cfg, "0xffffffff81000000+8");
[Severity: High]
Will this hardcoded 64-bit address cause failures on 32-bit platforms?
On 32-bit architectures, this string will exceed ULONG_MAX, causing
the internal kstrtoul() to safely fail and return -ERANGE. The parser
will then fall back to treating it as a global symbol name via
kallsyms_lookup_name(), which will also fail, ultimately returning -EINVAL
and breaking the test logic.
> + KUNIT_EXPECT_EQ(test, ret, 0);
> + KUNIT_EXPECT_EQ(test, cfg.base, KWATCH_BASE_ABS_ADDR);
> + KUNIT_EXPECT_EQ(test, cfg.sym_addr, 0xffffffff81000000UL);
[Severity: High]
Does this 64-bit literal cause implicit truncation compiler warnings
and test comparison failures on 32-bit architectures where unsigned long
is only 32 bits?
> + KUNIT_EXPECT_EQ(test, cfg.offset_count, 1);
> + KUNIT_EXPECT_EQ(test, cfg.offsets[0], 8);
> +}
> +
> +static struct kunit_case kwatch_deref_test_cases[] = {
> + KUNIT_CASE(kwatch_test_parse_deref_chain),
> + {}
> +};
> +
> +static struct kunit_suite kwatch_deref_test_suite = {
> + .name = "kwatch_deref",
> + .test_cases = kwatch_deref_test_cases,
> +};
> +
> +kunit_test_suite(kwatch_deref_test_suite);
[Severity: Medium]
If the Kconfig dependency above is fixed to allow this test to run with module
builds (e.g., depends on KWATCH != n), will this cause a multiple definition
build failure?
Since deref_test.o is linked directly into the kwatch module in the Makefile,
and kunit_test_suite() emits a module_init() alias, this will conflict with
the primary module_init() definition that likely already exists for kwatch.
> +
> +MODULE_DESCRIPTION("KUnit tests for the KWatch watch expression parser");
> +MODULE_LICENSE("GPL");
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714182243.10687-1-wangjinchao600@gmail.com?part=12
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 13/13] Documentation/dev-tools: document KWatch
2026-07-14 18:22 [RFC PATCH 00/13] mm/kwatch: dynamic hardware watchpoints for hunting memory corruption Jinchao Wang
` (11 preceding siblings ...)
2026-07-14 18:33 ` [RFC PATCH 12/13] mm/kwatch: add KUnit tests for the watch expression parser Jinchao Wang
@ 2026-07-14 18:33 ` Jinchao Wang
2026-07-14 18:48 ` sashiko-bot
12 siblings, 1 reply; 26+ messages in thread
From: Jinchao Wang @ 2026-07-14 18:33 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Masami Hiramatsu
Cc: Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc, Jinchao Wang
Describe what KWatch is for, how it compares with KASAN and KFENCE,
the debugfs configuration interface, the watch expression syntax,
how to read hits from the trace buffer (including after a crash),
and the current limitations.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
Documentation/dev-tools/index.rst | 1 +
Documentation/dev-tools/kwatch.rst | 193 +++++++++++++++++++++++++++++
2 files changed, 194 insertions(+)
create mode 100644 Documentation/dev-tools/kwatch.rst
diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst
index 59cbb77b33ff..f4c748da63db 100644
--- a/Documentation/dev-tools/index.rst
+++ b/Documentation/dev-tools/index.rst
@@ -30,6 +30,7 @@ Documentation/process/debugging/index.rst
ubsan
kmemleak
kcsan
+ kwatch
lkmm/index
kfence
kselftest
diff --git a/Documentation/dev-tools/kwatch.rst b/Documentation/dev-tools/kwatch.rst
new file mode 100644
index 000000000000..8ead0beb06b6
--- /dev/null
+++ b/Documentation/dev-tools/kwatch.rst
@@ -0,0 +1,193 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+======================================
+KWatch - Kernel Memory Watchpoint Tool
+======================================
+
+Overview
+========
+
+KWatch is a runtime-configurable debugging tool for locating kernel memory
+corruption. It arms hardware breakpoints (watchpoints) on a target address
+while a chosen function is executing, and reports the exact instruction that
+touches the watched memory, together with a stack trace, through a
+tracepoint.
+
+Unlike shadow-memory sanitizers, KWatch does not detect invalid accesses in
+general; it answers a narrower but common question during corruption hunts:
+"who writes to this address?". This includes in-bounds logical overwrites
+that KASAN cannot see, because the rogue writer modifies valid memory
+through a valid pointer, just at the wrong time or with the wrong data.
+
+Comparison with other tools:
+
+* KASAN detects out-of-bounds and use-after-free accesses, but reports the
+ symptom (the invalid access), not the writer that corrupted the data
+ earlier. It requires a rebuild and has significant CPU and memory
+ overhead, and its redzones perturb memory layout, which can hide
+ timing-sensitive bugs.
+* KFENCE is a low-overhead sampling detector for slab objects; it cannot be
+ pointed at one specific address.
+* Hardware breakpoints via kgdb or perf can watch an address, but only a
+ fixed one, system-wide, for the whole run. KWatch resolves the address
+ dynamically at function entry (for example "argument 2 of this function,
+ plus offset 8, dereferenced once") and disarms it again at function exit,
+ so short-lived and per-invocation objects can be watched too.
+
+KWatch has near-zero overhead while armed: the watched function pays for
+one kprobe/kretprobe pair plus programming of the debug registers; the rest
+of the system runs at full speed.
+
+Requirements
+============
+
+* ``CONFIG_KWATCH=y`` or ``m``. The Kconfig symbol depends on
+ ``CONFIG_PERF_EVENTS``, ``CONFIG_DEBUG_FS`` and an architecture that
+ provides ``HAVE_REINSTALL_HW_BREAKPOINT`` (currently x86 only).
+* Resolving symbol names in watch expressions requires ``CONFIG_KWATCH=y``
+ (built-in); a module can only watch absolute hexadecimal addresses.
+
+Usage
+=====
+
+KWatch is configured through a single debugfs file::
+
+ /sys/kernel/debug/kwatch/config
+
+Writing a configuration string starts a watch session (stopping any previous
+one); reading the file shows the active configuration and hit-rejection
+counters. The configuration is a whitespace-separated list of ``key=value``
+tokens:
+
+=================== ==========================================================
+Key Meaning
+=================== ==========================================================
+``func_name`` Function whose execution opens the watch window.
+``func_offset`` Instruction offset inside ``func_name`` at which the
+ watchpoint is armed (default 0 = function entry).
+``watch_expr`` Expression describing the address to watch (see below).
+``watch_len`` Watched length in bytes: 1, 2, 4 or 8 (default 8).
+``access_type`` 0 = write (default), 1 = read, 2 = read/write,
+ 3 = execute.
+``depth`` Recursion depth at which the window opens (default 0).
+``max_watch`` Number of hardware watchpoints to preallocate
+ (default 4).
+``max_concurrency`` Maximum number of tasks concurrently inside the watch
+ window (default 256).
+``duration`` For global watches: seconds until automatic stop.
+=================== ==========================================================
+
+Watch expressions
+-----------------
+
+The address to watch is computed at function entry from::
+
+ watch_expr={base}[+-offset][->[+-]offset]...
+
+* ``base`` is one of:
+
+ - ``arg1`` ... ``arg6``: a function argument (register calling
+ convention),
+ - ``stack``: the kernel stack pointer at the probe point,
+ - an absolute hexadecimal address, e.g. ``0xffffffff81234567``,
+ - a global symbol name (built-in KWatch only).
+
+* ``+offset`` / ``-offset`` adjusts the current address.
+* ``->offset`` loads the pointer stored at the current address (via
+ ``get_kernel_nofault()``) and then applies the offset. Up to four chain
+ elements are supported; offsets must be explicit (``->`` alone is
+ rejected).
+
+Given::
+
+ struct some_struct {
+ struct some_struct *ptr; /* offset 0 */
+ int num; /* offset 8 */
+ };
+
+ void target_function(struct some_struct *arg1);
+
+typical expressions are:
+
+=========================== ==============================================
+Expression Watches
+=========================== ==============================================
+``watch_expr=arg1`` ``&arg1->ptr`` (the pointer field itself)
+``watch_expr=arg1+8`` ``&arg1->num``
+``watch_expr=arg1->0`` ``&arg1->ptr->ptr`` (one dereference)
+``watch_expr=arg1->8`` ``&arg1->ptr->num``
+``watch_expr=0xffff...+8`` absolute address plus 8
+=========================== ==============================================
+
+Example: catch whoever overwrites ``arg1->num`` of a function while that
+function runs::
+
+ echo "func_name=target_function watch_expr=arg1+8 watch_len=4" \
+ > /sys/kernel/debug/kwatch/config
+
+Watching global variables
+-------------------------
+
+A global variable has no natural function window. When ``duration`` is
+given without ``func_name``, KWatch starts an internal anchor kernel thread
+that sleeps inside a dummy function, and uses that function as the window::
+
+ echo "watch_expr=jiffies_wobble duration=60 watch_len=8" \
+ > /sys/kernel/debug/kwatch/config
+
+The session tears itself down when the duration expires.
+
+Reading hits
+------------
+
+Hits are emitted as the ``kwatch:kwatch_hit`` tracepoint, which is safe in
+NMI-like contexts where printk is not. Each event carries the timestamp,
+the instruction pointer, the watched address and a short stack trace::
+
+ echo 1 > /sys/kernel/debug/tracing/events/kwatch/kwatch_hit/enable
+ cat /sys/kernel/debug/tracing/trace_pipe
+
+If the corruption crashes the machine, the ring buffer can still be
+recovered:
+
+* ``echo 1 > /proc/sys/kernel/ftrace_dump_on_oops`` (or the
+ ``ftrace_dump_on_oops`` boot parameter) dumps the buffer to the console
+ on an oops.
+* With kdump, the buffer is present in the vmcore and can be read with
+ ``crash> trace``.
+* ``CONFIG_PSTORE_FTRACE`` persists it across reboots on supported
+ platforms.
+
+Limitations
+===========
+
+* Functions that run in a genuine NMI(-like) context are rejected at
+ function entry; rejected invocations never open a watch window and are
+ counted in the ``nmi_rejected`` field of the config file. Watching
+ functions reachable from NMI handlers is out of scope.
+* The number of concurrent watchpoints is bounded by the CPU's debug
+ registers (typically 4).
+* If the target address cannot be resolved at arming time (for example a
+ ``get_kernel_nofault()`` failure on a swapped or unmapped page), the
+ watchpoint is not armed for that invocation.
+* Offsets in watch expressions are static; dynamic indexing such as
+ ``arg1->ptr[arg2]`` is not supported.
+* arm64 is not yet supported: stepping over a hit that has a custom
+ overflow handler needs a generic mechanism in the arch code, which is
+ planned as a follow-up series.
+
+Implementation notes
+====================
+
+The implementation lives in ``mm/kwatch/`` and is split into a control
+plane (``core.c``, the debugfs interface), an execution plane (``probe.c``
+and ``deref.c``: kprobe/kretprobe window management and address
+resolution), and a resource plane (``hwbp.c`` and ``task_ctx.c``).
+
+Hardware watchpoints are preallocated as perf events on every CPU and
+re-pointed at hit time with ``modify_wide_hw_breakpoint_local()``, a new
+hw_breakpoint API that updates the breakpoint on the local CPU without
+releasing its slot; other CPUs are updated by asynchronous IPIs. Per-task
+window state is kept in a fixed-size, lockless open-addressing array
+claimed with ``cmpxchg()``, so the hit path performs no allocation and
+takes no locks, which keeps it safe in atomic and NMI-like contexts.
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 13/13] Documentation/dev-tools: document KWatch
2026-07-14 18:33 ` [RFC PATCH 13/13] Documentation/dev-tools: document KWatch Jinchao Wang
@ 2026-07-14 18:48 ` sashiko-bot
0 siblings, 0 replies; 26+ messages in thread
From: sashiko-bot @ 2026-07-14 18:48 UTC (permalink / raw)
To: Jinchao Wang; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Rate-limiting in kwatch_hwbp_arm() silently fails to arm the watchpoint locally and causes state mismatches.
- [High] Using calling convention registers (arg1..arg6) to resolve watch expressions at mid-function offsets leads to reading garbage data.
--
--- Patch [13]: [RFC PATCH 13/13] Documentation/dev-tools: document KWatch ---
commit da418fc283a833027fb7ccca30c188d2536f9a4a
Author: Jinchao Wang <wangjinchao600@gmail.com>
Documentation/dev-tools: document KWatch
Describe what KWatch is for, how it compares with KASAN and KFENCE,
the debugfs configuration interface, the watch expression syntax,
how to read hits from the trace buffer (including after a crash),
and the current limitations.
> diff --git a/mm/kwatch/deref.c b/mm/kwatch/deref.c
> new file mode 100644
> index 0000000000000..a93c76139e7cd
> --- /dev/null
> +++ b/mm/kwatch/deref.c
> @@ -0,0 +1,174 @@
> +// SPDX-License-Identifier: GPL-2.0
[ ... ]
> +int kwatch_deref_resolve(const struct kwatch_config *cfg, struct pt_regs *regs,
> + unsigned long *out_addr, u16 *out_len)
> +{
> + unsigned long addr = 0;
> + int i;
> +
> + /* 1. Resolve the Base Anchor */
> + if (cfg->base == KWATCH_BASE_STACK) {
> + addr = kernel_stack_pointer(regs);
> + if (unlikely(!addr))
> + return -EINVAL;
> + } else if (cfg->base >= KWATCH_BASE_ARG1 &&
> + cfg->base <= KWATCH_BASE_ARG6) {
> + int arg_idx = cfg->base - KWATCH_BASE_ARG1;
> +
> + addr = regs_get_kernel_argument(regs, arg_idx);
[Severity: High]
If a watchpoint is configured with a mid-function offset (func_offset > 0),
are the calling convention registers still guaranteed to hold the arguments?
Since the compiler is free to overwrite argument registers immediately after
function entry, could this read garbage data and cause KWatch to monitor an
incorrect memory address?
[ ... ]
> diff --git a/mm/kwatch/hwbp.c b/mm/kwatch/hwbp.c
> new file mode 100644
> index 0000000000000..19498ba038260
> --- /dev/null
> +++ b/mm/kwatch/hwbp.c
> @@ -0,0 +1,358 @@
> +// SPDX-License-Identifier: GPL-2.0
[ ... ]
> +void kwatch_hwbp_arm(struct kwatch_watchpoint *wp, unsigned long addr, u16 len)
> +{
> + static DEFINE_PER_CPU(u64, last_ipi_time);
> + int cur_cpu;
> + call_single_data_t *csd;
> + int cpu;
> + bool is_disarm = (addr == (unsigned long)&kwatch_dummy_holder);
> +
> + wp->attr.bp_addr = addr;
> + wp->attr.bp_len = len;
> +
> + if (!is_disarm)
> + wp->arm_tsk = current;
> +
> + /* ensure attr update visible to other cpu before sending IPI */
> + smp_wmb();
> +
> + atomic_set(&wp->pending_ipis, 1);
> + cur_cpu = get_cpu();
> +
> + if (!is_disarm) {
> + u64 now = local_clock();
> + u64 last = this_cpu_read(last_ipi_time);
> +
> + if (now - last < 1000000ULL) {
> + put_cpu();
> + return;
> + }
[Severity: High]
Does this early return skip both the global IPI broadcast and the
kwatch_hwbp_arm_local() call?
If a user configures KWatch on a frequently invoked function and this rate
limit is hit, it looks like it will silently fail to arm the watchpoint locally.
Could this also cause state mismatches when disarm operations later broadcast
IPIs for a watchpoint that was never actually armed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714182243.10687-1-wangjinchao600@gmail.com?part=13
^ permalink raw reply [flat|nested] 26+ messages in thread