[RFC PATCH 00/13] mm: Introduce Kernel Stack Watch debugging tool

[RFC PATCH 00/13] mm: Introduce Kernel Stack Watch debugging tool

[Jinchao Wang]

This patch series introduces Kernel Stack Watch (KSW), a lightweight
debugging tool for detecting kernel stack corruption in real-time.
The motivation comes from cases where corruption happens silently in
one function but only manifests later as a crash in another,
with no direct call trace connection. Such problems are often very
difficult to debug with existing tools.

KSW works by combining hardware breakpoints with kprobes/kretprobes.
It can watch a stack canary or a selected local variable, and detect
the moment the corruption actually occurs. This allows developers to
pinpoint the real source, rather than only observing the final crash.

Key features include:
- Lightweight design with minimal impact on bug reproducibility
- Real-time detection of stack corruption
- Simple configuration through `/proc/kstackwatch`
- Support for recursive functions with configurable nesting depth

To validate the approach, I have also prepared test modules and scripts
that simulate corruption scenarios.

I am sharing this work to seek feedback on the idea and the design.
Any comments or suggestions for improvement are very welcome.

The series is structured as follows:

Jinchao Wang (13):
  mm: Add kstackwatch build infrastructure
  x86/HWBP: Add arch_reinstall_hw_breakpoint() for atomic updates
  mm/kstackwatch: Add module core and configuration interface
  mm/kstackwatch: Add HWBP pre-allocation infrastructure
  mm/kstackwatch: Add atomic HWBP arm/disarm operations
  mm/kstackwatch: Add stack address resolution functions
  mm/kstackwatch: Add kprobe and stack watch control
  mm/kstackwatch: Wire up watch and stack subsystems in module core
  mm/kstackwatch: Add architecture support validation
  mm/kstackwatch: Handle nested function calls
  mm/kstackwatch: Ignore corruption in kretprobe trampolines
  mm/kstackwatch: Add debug and test functions
  mm/kstackwatch: Add a test module and script

 arch/x86/include/asm/hw_breakpoint.h  |   1 +
 arch/x86/kernel/hw_breakpoint.c       |  50 ++++++
 mm/Kconfig.debug                      |  23 +++
 mm/Makefile                           |   1 +
 mm/kstackwatch/Makefile               |  11 ++
 mm/kstackwatch/kernel.c               | 248 ++++++++++++++++++++++++++
 mm/kstackwatch/kstackwatch.h          |  56 ++++++
 mm/kstackwatch/kstackwatch_test.c     | 237 ++++++++++++++++++++++++
 mm/kstackwatch/stack.c                | 222 +++++++++++++++++++++++
 mm/kstackwatch/watch.c                | 240 +++++++++++++++++++++++++
 tools/kstackwatch/kstackwatch_test.sh | 122 +++++++++++++
 11 files changed, 1211 insertions(+)
 create mode 100644 mm/kstackwatch/Makefile
 create mode 100644 mm/kstackwatch/kernel.c
 create mode 100644 mm/kstackwatch/kstackwatch.h
 create mode 100644 mm/kstackwatch/kstackwatch_test.c
 create mode 100644 mm/kstackwatch/stack.c
 create mode 100644 mm/kstackwatch/watch.c
 create mode 100644 tools/kstackwatch/kstackwatch_test.sh

-- 
2.43.0

[RFC PATCH 01/13] mm: Add kstackwatch build infrastructure

[Jinchao Wang]

Introduce the build system for kstackwatch, a new kernel stack
corruption debugging tool. This patch adds the necessary Kconfig
and Makefile infrastructure to support the kstackwatch subsystem.

kstackwatch uses hardware write breakpoints to detect stack
corruption in real-time, providing precise identification of
the instruction that overwrites stack canaries or local variables.
This is a significant improvement over traditional stack protection
mechanisms that only detect corruption at function exit.

The implementation is placed in mm/kstackwatch/ alongside other
memory debugging tools like KASAN, KFENCE, and KMSAN. The tool
requires STACKPROTECTOR, hardware breakpoint support, and kprobes
functionality to operate.

The modular design splits functionality across:
- kernel.c: Main logic and module lifecycle
- stack.c: Stack canary detection and probing
- watch.c: Hardware breakpoint management

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/Kconfig.debug             | 12 ++++++++++++
 mm/Makefile                  |  1 +
 mm/kstackwatch/Makefile      |  3 +++
 mm/kstackwatch/kernel.c      |  0
 mm/kstackwatch/kstackwatch.h |  0
 mm/kstackwatch/stack.c       |  0
 mm/kstackwatch/watch.c       |  0
 7 files changed, 16 insertions(+)
 create mode 100644 mm/kstackwatch/Makefile
 create mode 100644 mm/kstackwatch/kernel.c
 create mode 100644 mm/kstackwatch/kstackwatch.h
 create mode 100644 mm/kstackwatch/stack.c
 create mode 100644 mm/kstackwatch/watch.c

diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
index 32b65073d0cc..dd9c1bb7f549 100644
--- a/mm/Kconfig.debug
+++ b/mm/Kconfig.debug
@@ -309,3 +309,15 @@ config PER_VMA_LOCK_STATS
 	  overhead in the page fault path.
 
 	  If in doubt, say N.
+
+
+config KSTACK_WATCH
+	tristate "Kernel Stack Watch"
+	depends on STACKPROTECTOR && HAVE_HW_BREAKPOINT && KPROBES && HAVE_KRETPROBES
+	help
+	  This debugging tool monitors kernel stack usage. When enabled,
+	  it can detect potential stack corruption by watching the remaining
+	  stack space. This provides real-time warnings before a crash occurs,
+	  which is useful for debugging stability issues.
+
+	  If unsure, say N.
diff --git a/mm/Makefile b/mm/Makefile
index ef54aa615d9d..665c9f2bf987 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_KSTACK_WATCH)	+= kstackwatch/
 obj-$(CONFIG_FAILSLAB) += failslab.o
 obj-$(CONFIG_FAIL_PAGE_ALLOC) += fail_page_alloc.o
 obj-$(CONFIG_MEMTEST)		+= memtest.o
diff --git a/mm/kstackwatch/Makefile b/mm/kstackwatch/Makefile
new file mode 100644
index 000000000000..076822eb7661
--- /dev/null
+++ b/mm/kstackwatch/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_KSTACK_WATCH)	+= kstackwatch.o
+
+kstackwatch-y := kernel.o stack.o watch.o
diff --git a/mm/kstackwatch/kernel.c b/mm/kstackwatch/kernel.c
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/mm/kstackwatch/kstackwatch.h b/mm/kstackwatch/kstackwatch.h
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/mm/kstackwatch/stack.c b/mm/kstackwatch/stack.c
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/mm/kstackwatch/watch.c b/mm/kstackwatch/watch.c
new file mode 100644
index 000000000000..e69de29bb2d1
-- 
2.43.0

[RFC PATCH 02/13] x86/HWBP: Add arch_reinstall_hw_breakpoint() for atomic updates

[Jinchao Wang]

Add arch_reinstall_hw_breakpoint() to enable atomic context modification
of hardware breakpoint parameters without deallocating and reallocating
the breakpoint slot.

The existing arch_install_hw_breakpoint() allocates a new debug register
slot, while arch_uninstall_hw_breakpoint() deallocates it. However, some
use cases require modifying breakpoint parameters (address, length, type)
atomically without losing the allocated slot, particularly when operating
in atomic contexts where allocation might fail or be unavailable.

This is particularly useful for debugging tools like kstackwatch that
need to dynamically update breakpoint targets in atomic contexts while
maintaining consistent hardware state.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 arch/x86/include/asm/hw_breakpoint.h |  1 +
 arch/x86/kernel/hw_breakpoint.c      | 50 ++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/arch/x86/include/asm/hw_breakpoint.h b/arch/x86/include/asm/hw_breakpoint.h
index 0bc931cd0698..bb7c70ad22fe 100644
--- a/arch/x86/include/asm/hw_breakpoint.h
+++ b/arch/x86/include/asm/hw_breakpoint.h
@@ -59,6 +59,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 b01644c949b2..89135229ed21 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -132,6 +132,56 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
 	return 0;
 }
 
+/*
+ * Reinstall a hardware breakpoint on the current CPU.
+ *
+ * This function is used to re-establish a perf counter hardware breakpoint.
+ * It finds the debug address register slot previously allocated for the
+ * breakpoint and re-enables it by writing the address to the debug register
+ * and setting the corresponding bits in the debug control register (DR7).
+ *
+ * It is expected that the breakpoint's event context lock is already held
+ * and interrupts are disabled, ensuring atomicity and safety from other
+ * event handlers.
+ */
+int arch_reinstall_hw_breakpoint(struct perf_event *bp)
+{
+	struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+	unsigned long *dr7;
+	int i;
+
+	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)
+			break;
+	}
+
+	if (WARN_ONCE(i == HBP_NUM, "Can't find a matching breakpoint slot"))
+		return -EINVAL;
+
+	set_debugreg(info->address, i);
+	__this_cpu_write(cpu_debugreg[i], info->address);
+
+	dr7 = this_cpu_ptr(&cpu_dr7);
+	*dr7 |= encode_dr7(i, 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.
+	 */
+	barrier();
+
+	set_debugreg(*dr7, 7);
+	if (info->mask)
+		amd_set_dr_addr_mask(info->mask, i);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(arch_reinstall_hw_breakpoint);
+
 /*
  * Uninstall the breakpoint contained in the given counter.
  *
-- 
2.43.0

[RFC PATCH 03/13] mm/kstackwatch: Add module core and configuration interface

[Jinchao Wang]

Implement the main module infrastructure for kstackwatch, providing
a proc-based configuration interface and basic module lifecycle
management.

This patch introduces:

1. Module initialization and cleanup with proper resource management
2. Configuration parsing for the flexible watch specification format:
   "function+ip_offset[+depth] [local_var_offset:local_var_len]"
3. Proc interface (/proc/kstackwatch) for runtime configuration
4. Support for both watch types through unified configuration syntax

The configuration parser handles:
- Function name and instruction pointer offset (mandatory)
- Optional recursion depth filtering
- Optional local variable specification (offset:length)
- Automatic detection of watch type based on presence of stack parameters

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/kstackwatch/kernel.c      | 205 +++++++++++++++++++++++++++++++++++
 mm/kstackwatch/kstackwatch.h |  39 +++++++
 2 files changed, 244 insertions(+)

diff --git a/mm/kstackwatch/kernel.c b/mm/kstackwatch/kernel.c
index e69de29bb2d1..726cf3f25888 100644
--- a/mm/kstackwatch/kernel.c
+++ b/mm/kstackwatch/kernel.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/kern_levels.h>
+#include <linux/kstrtox.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/proc_fs.h>
+#include <linux/uaccess.h>
+#include <linux/string.h>
+#include <linux/utsname.h>
+#include <linux/seq_file.h>
+
+#include "kstackwatch.h"
+
+MODULE_AUTHOR("Jinchao Wang");
+MODULE_DESCRIPTION("Kernel Stack Watch");
+MODULE_LICENSE("GPL");
+
+struct ksw_config *ksw_config;
+bool watching_active;
+
+/* Module parameters */
+bool panic_on_catch;
+module_param(panic_on_catch, bool, 0644);
+MODULE_PARM_DESC(panic_on_catch,
+		 "Trigger a kernel panic immediately on corruption catch");
+
+static int start_watching(void)
+{
+	if (strlen(ksw_config->function) == 0) {
+		pr_err("KSW: No target function specified\n");
+		return -EINVAL;
+	}
+
+	watching_active = true;
+
+	pr_info("KSW: start watching %s\n", ksw_config->config_str);
+	return 0;
+}
+
+static void stop_watching(void)
+{
+	watching_active = false;
+
+	pr_info("KSW: stop watching %s\n", ksw_config->config_str);
+}
+
+/* Parse watch configuration:
+ *    function+ip_offset[+depth] [local_var_offset:local_var_len]
+ */
+static int parse_config(char *buf, struct ksw_config *config)
+{
+	char *func_part, *stack_part = NULL;
+	char *token;
+
+	/* Initialize with default values */
+	memset(config, 0, sizeof(*config));
+	config->type = WATCH_CANARY;
+
+	/* strim() removes leading/trailing whitespace */
+	func_part = strim(buf);
+	strscpy(config->config_str, func_part, MAX_CONFIG_STR_LEN);
+
+	stack_part = strchr(func_part, ' ');
+	if (stack_part) {
+		*stack_part = '\0'; // Terminate the function part
+		stack_part = strim(stack_part + 1);
+	}
+
+	/* 1. Parse the function part: function+ip_offset[+depth] */
+	token = strsep(&func_part, "+");
+	if (!token)
+		return -EINVAL;
+
+	strscpy(config->function, token, MAX_FUNC_NAME_LEN - 1);
+
+	token = strsep(&func_part, "+");
+	if (!token || kstrtou16(token, 0, &config->ip_offset)) {
+		pr_err("KSW: Failed to parse instruction offset\n");
+		return -EINVAL;
+	}
+
+	token = strsep(&func_part, "+");
+	if (token && kstrtou16(token, 0, &config->depth)) {
+		pr_err("KSW: Failed to parse depth\n");
+		return -EINVAL;
+	}
+	if (!stack_part || !(*stack_part))
+		return 0;
+
+	/* 2. Parse the optional stack part: offset:len */
+	config->type = WATCH_LOCAL_VAR;
+	token = strsep(&stack_part, ":");
+	if (!token || kstrtou16(token, 0, &config->local_var_offset)) {
+		pr_err("KSW: Failed to parse stack variable offset\n");
+		return -EINVAL;
+	}
+
+	if (!stack_part || kstrtou16(stack_part, 0, &config->local_var_len)) {
+		pr_err("KSW: Failed to parse stack variable length\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/* Proc interface for configuration */
+static ssize_t kstackwatch_proc_write(struct file *file,
+				      const char __user *buffer, size_t count,
+				      loff_t *pos)
+{
+	char input[256];
+	int ret;
+
+	if (count == 0 || count >= sizeof(input))
+		return -EINVAL;
+
+	if (copy_from_user(input, buffer, count))
+		return -EFAULT;
+
+	input[count] = '\0';
+	strim(input);
+
+	/* Stop current watching */
+	if (watching_active)
+		stop_watching();
+
+	ret = parse_config(input, ksw_config);
+	if (ret)
+		return ret;
+
+	/* Start watching */
+	ret = start_watching();
+	if (ret < 0) {
+		pr_err("KSW: Failed to start watching with %d\n", ret);
+		return ret;
+	}
+
+	return count;
+}
+
+static int kstackwatch_proc_show(struct seq_file *m, void *v)
+{
+	struct ksw_config *config = ksw_config;
+
+	if (watching_active) {
+		seq_printf(m, "KSW: watch config %s\n", config->config_str);
+	} else {
+		seq_puts(m, "Not watching\n");
+		seq_puts(m, "\nUsage:\n");
+		seq_puts(
+			m,
+			"  echo 'function+ip_offset[+depth] [local_var_offset:local_var_len]' > /proc/kstackwatch\n");
+		seq_puts(m, "  if ignore the stack part, watch the canary");
+	}
+
+	return 0;
+}
+
+static int kstackwatch_proc_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, kstackwatch_proc_show, NULL);
+}
+
+static const struct proc_ops kstackwatch_proc_ops = {
+	.proc_open = kstackwatch_proc_open,
+	.proc_read = seq_read,
+	.proc_write = kstackwatch_proc_write,
+	.proc_lseek = seq_lseek,
+	.proc_release = single_release,
+};
+
+static int __init kstackwatch_init(void)
+{
+	ksw_config = kmalloc(sizeof(*ksw_config), GFP_KERNEL);
+	if (!ksw_config)
+		return -ENOMEM;
+
+	/* Create proc interface */
+	if (!proc_create("kstackwatch", 0644, NULL, &kstackwatch_proc_ops)) {
+		pr_err("KSW: create proc kstackwatch fail");
+		return -ENOMEM;
+	}
+
+	pr_info("KSW: Module loaded\n");
+	pr_info("KSW: Usage:\n");
+	pr_info("KSW: echo 'function+ip_offset[+depth] [local_var_offset:local_var_len]' > /proc/kstackwatch\n");
+
+	return 0;
+}
+
+static void __exit kstackwatch_exit(void)
+{
+	/* Cleanup active watching */
+	if (watching_active)
+		stop_watching();
+
+	/* Remove proc interface */
+	remove_proc_entry("kstackwatch", NULL);
+	kfree(ksw_config);
+
+	pr_info("KSW: Module unloaded\n");
+}
+
+module_init(kstackwatch_init);
+module_exit(kstackwatch_exit);
diff --git a/mm/kstackwatch/kstackwatch.h b/mm/kstackwatch/kstackwatch.h
index e69de29bb2d1..f58af36e64a7 100644
--- a/mm/kstackwatch/kstackwatch.h
+++ b/mm/kstackwatch/kstackwatch.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _KSTACKWATCH_H
+#define _KSTACKWATCH_H
+
+#include <linux/types.h>
+
+#define MAX_FUNC_NAME_LEN 64
+#define MAX_CONFIG_STR_LEN 128
+
+/* Watch target types */
+enum watch_type {
+	WATCH_CANARY = 0, /* canary placed by compiler */
+	WATCH_LOCAL_VAR, /* local var defined by code */
+};
+
+struct ksw_config {
+	/* function part */
+	char function[MAX_FUNC_NAME_LEN];
+	u16 ip_offset;
+	u16 depth;
+
+	/* stack part, useless for canary watch */
+	/* offset from rsp at function+ip_offset */
+	u16 local_var_offset;
+
+	/*
+	 * local var size (1,2,4,8 bytes)
+	 * it will be the watching len
+	 */
+	u16 local_var_len;
+
+	/* easy for understand*/
+	enum watch_type type;
+
+	/* save to show */
+	char config_str[MAX_CONFIG_STR_LEN];
+};
+
+#endif /* _KSTACKWATCH_H */
-- 
2.43.0

[RFC PATCH 04/13] mm/kstackwatch: Add HWBP pre-allocation infrastructure

[Jinchao Wang]

Implement the core HWBP management for kstackwatch with
pre-allocation strategy to enable atomic context operation.

This patch introduces the fundamental breakthrough that allows kstackwatch
to work in atomic contexts: pre-allocating hardware breakpoints across
all CPUs during initialization, then using arch_reinstall_hw_breakpoint()
to atomically update breakpoint targets without allocation overhead.

Key features:
- Pre-allocate per-CPU hardware breakpoints using register_wide_hw_breakpoint()
- Initialize with a dummy marker address that will be dynamically updated
- Comprehensive corruption detection handler with register dumps and
  optional panic trigger
- Clean resource management on module exit

The pre-allocation approach is critical because:
1. Hardware breakpoint allocation can fail or sleep in atomic contexts
2. kprobes run in atomic context where allocation is not permitted
3. Pre-allocated breakpoints can be instantly retargeted using
   arch_reinstall_hw_breakpoint() without any blocking operations

This foundation enables the subsequent kprobe integration to atomically
arm/disarm breakpoints on function entry/exit, providing real-time
stack corruption detection without the limitations of traditional
allocation-based approaches.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/kstackwatch/kstackwatch.h |  6 ++++
 mm/kstackwatch/watch.c       | 65 ++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/mm/kstackwatch/kstackwatch.h b/mm/kstackwatch/kstackwatch.h
index f58af36e64a7..256574cd9cb2 100644
--- a/mm/kstackwatch/kstackwatch.h
+++ b/mm/kstackwatch/kstackwatch.h
@@ -36,4 +36,10 @@ struct ksw_config {
 	char config_str[MAX_CONFIG_STR_LEN];
 };
 
+extern bool panic_on_catch;
+
+/* watch management */
+int ksw_watch_init(struct ksw_config *config);
+void ksw_watch_exit(void);
+
 #endif /* _KSTACKWATCH_H */
diff --git a/mm/kstackwatch/watch.c b/mm/kstackwatch/watch.c
index e69de29bb2d1..5cc2dfef140b 100644
--- a/mm/kstackwatch/watch.c
+++ b/mm/kstackwatch/watch.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/kprobes.h>
+#include <linux/hw_breakpoint.h>
+#include <linux/perf_event.h>
+#include <linux/sched/debug.h>
+#include <linux/smp.h>
+#include <linux/slab.h>
+#include <asm/hw_breakpoint.h>
+#include <linux/stacktrace.h>
+#include <linux/delay.h>
+
+#include "kstackwatch.h"
+
+struct perf_event *__percpu *watch_events;
+struct ksw_config *watch_config;
+
+static unsigned long long marker;
+
+/* Enhanced breakpoint handler with watch identification */
+static void ksw_watch_handler(struct perf_event *bp,
+			      struct perf_sample_data *data,
+			      struct pt_regs *regs)
+{
+	pr_emerg("========== KStackWatch: Caught stack corruption =======\n");
+	pr_emerg("KSW: config %s\n", watch_config->config_str);
+	show_regs(regs);
+	pr_emerg("========== KStackWatch End ==========\n");
+	mdelay(100);
+
+	if (panic_on_catch)
+		panic("KSW: Stack corruption detected");
+}
+
+/* Initialize hardware breakpoint  */
+int ksw_watch_init(struct ksw_config *config)
+{
+	struct perf_event_attr attr;
+
+	/* Initialize default breakpoint attributes */
+	hw_breakpoint_init(&attr);
+	attr.bp_addr = (unsigned long)&marker;
+	attr.bp_len = HW_BREAKPOINT_LEN_8;
+	attr.bp_type = HW_BREAKPOINT_W;
+	watch_events =
+		register_wide_hw_breakpoint(&attr, ksw_watch_handler, NULL);
+	if (IS_ERR((void *)watch_events)) {
+		int ret = PTR_ERR((void *)watch_events);
+
+		pr_err("KSW: Failed to register wide hw breakpoint: %d\n", ret);
+		return ret;
+	}
+
+	watch_config = config;
+	pr_info("KSW: HWBP  initialized\n");
+	return 0;
+}
+
+/* Cleanup hardware breakpoint  */
+void ksw_watch_exit(void)
+{
+	unregister_wide_hw_breakpoint(watch_events);
+	watch_events = NULL;
+
+	pr_info("KSW: HWBP  cleaned up\n");
+}
-- 
2.43.0

[RFC PATCH 05/13] mm/kstackwatch: Add atomic HWBP arm/disarm operations

[Jinchao Wang]

Implement the critical atomic operations for dynamically arming and
disarming hardware breakpoints across all CPUs without allocation
overhead.

This patch adds the core functionality that enables kstackwatch to
operate in atomic contexts (such as kprobe handlers):

Key features:
1. ksw_watch_on() - Atomically arm breakpoints on all CPUs with
   specified address and length
2. ksw_watch_off() - Disarm all breakpoints by resetting to dummy marker
3. HWBP updates using arch_reinstall_hw_breakpoint()
4. SMP-safe coordination using work queues and async function calls

The implementation uses a hybrid approach for SMP coordination:
- Current CPU: Direct function call for immediate effect
- Other CPUs: Asynchronous smp_call_function_single_async() for
  non-blocking operation in queue worker

This enables the kprobe handlers (added in subsequent patches) to
instantly arm breakpoints on function entry and disarm on exit,
providing real-time stack corruption detection without the performance
penalties or atomic context limitations of traditional approaches.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/kstackwatch/kstackwatch.h |   2 +
 mm/kstackwatch/watch.c       | 118 +++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+)

diff --git a/mm/kstackwatch/kstackwatch.h b/mm/kstackwatch/kstackwatch.h
index 256574cd9cb2..910f49014715 100644
--- a/mm/kstackwatch/kstackwatch.h
+++ b/mm/kstackwatch/kstackwatch.h
@@ -41,5 +41,7 @@ extern bool panic_on_catch;
 /* watch management */
 int ksw_watch_init(struct ksw_config *config);
 void ksw_watch_exit(void);
+int ksw_watch_on(u64 watch_addr, u64 watch_len);
+void ksw_watch_off(void);
 
 #endif /* _KSTACKWATCH_H */
diff --git a/mm/kstackwatch/watch.c b/mm/kstackwatch/watch.c
index 5cc2dfef140b..7ab247531961 100644
--- a/mm/kstackwatch/watch.c
+++ b/mm/kstackwatch/watch.c
@@ -1,4 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
+
+#include "linux/printk.h"
 #include <linux/kprobes.h>
 #include <linux/hw_breakpoint.h>
 #include <linux/perf_event.h>
@@ -11,11 +13,24 @@
 
 #include "kstackwatch.h"
 
+#define MAX_STACK_ENTRIES 64
+
 struct perf_event *__percpu *watch_events;
 struct ksw_config *watch_config;
+static DEFINE_SPINLOCK(watch_lock);
 
 static unsigned long long marker;
 
+struct watch_worker {
+	struct work_struct work;
+	int original_cpu;
+} myworker;
+
+static void ksw_watch_on_local_cpu(void *useless);
+
+static DEFINE_PER_CPU(call_single_data_t,
+		      hwbp_csd) = CSD_INIT(ksw_watch_on_local_cpu, NULL);
+
 /* Enhanced breakpoint handler with watch identification */
 static void ksw_watch_handler(struct perf_event *bp,
 			      struct perf_sample_data *data,
@@ -31,6 +46,59 @@ static void ksw_watch_handler(struct perf_event *bp,
 		panic("KSW: Stack corruption detected");
 }
 
+/* Setup single hardware breakpoint on current CPU */
+static void ksw_watch_on_local_cpu(void *useless)
+{
+	struct perf_event *bp;
+	int cpu = smp_processor_id();
+	int ret;
+
+	bp = *per_cpu_ptr(watch_events, cpu);
+	if (!bp)
+		return;
+
+	/* Update breakpoint address */
+	ret = hw_breakpoint_arch_parse(bp, &bp->attr, counter_arch_bp(bp));
+	if (ret) {
+		pr_err("KSW: Failed to parse HWBP for CPU %d ret %d\n", cpu,
+		       ret);
+		return;
+	}
+	ret = arch_reinstall_hw_breakpoint(bp);
+	if (ret) {
+		pr_err("KSW: Failed to install HWBP on CPU %d ret %d\n", cpu,
+		       ret);
+		return;
+	}
+
+	if (bp->attr.bp_addr == (unsigned long)&marker) {
+		pr_info("KSW: HWBP disarmed on CPU %d\n", cpu);
+	} else {
+		pr_info("KSW: HWBP armed on CPU %d at 0x%px (len %llu)\n", cpu,
+			(void *)bp->attr.bp_addr, bp->attr.bp_len);
+	}
+}
+
+static void ksw_watch_on_work_fn(struct work_struct *work)
+{
+	struct watch_worker *worker =
+		container_of(work, struct watch_worker, work);
+	int original_cpu = READ_ONCE(worker->original_cpu);
+	int local_cpu = smp_processor_id();
+	call_single_data_t *csd;
+	int cpu;
+
+	for_each_online_cpu(cpu) {
+		if (cpu == original_cpu)
+			continue;
+		if (cpu == local_cpu)
+			continue;
+		csd = &per_cpu(hwbp_csd, cpu);
+		smp_call_function_single_async(cpu, csd);
+	}
+	ksw_watch_on_local_cpu(NULL);
+}
+
 /* Initialize hardware breakpoint  */
 int ksw_watch_init(struct ksw_config *config)
 {
@@ -50,6 +118,8 @@ int ksw_watch_init(struct ksw_config *config)
 		return ret;
 	}
 
+	/* Initialize work structure */
+	INIT_WORK(&myworker.work, ksw_watch_on_work_fn);
 	watch_config = config;
 	pr_info("KSW: HWBP  initialized\n");
 	return 0;
@@ -63,3 +133,51 @@ void ksw_watch_exit(void)
 
 	pr_info("KSW: HWBP  cleaned up\n");
 }
+
+/* Legacy API: Arm single hardware breakpoint (backward compatibility) */
+int ksw_watch_on(u64 watch_addr, u64 watch_len)
+{
+	struct perf_event *bp;
+	unsigned long flags;
+	int cpu;
+
+	if (!watch_addr) {
+		pr_err("KSW: Invalid address for arming HWBP\n");
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&watch_lock, flags);
+
+	/* Check if already armed - only need to check one CPU since all share same addr */
+	bp = *this_cpu_ptr(watch_events);
+	if (bp->attr.bp_addr != 0 &&
+	    bp->attr.bp_addr != (unsigned long)&marker && // installted
+	    watch_addr != (unsigned long)&marker) { //restore
+		spin_unlock_irqrestore(&watch_lock, flags);
+		return -EBUSY;
+	}
+
+	/* Update address in all minimal breakpoint structures */
+	for_each_possible_cpu(cpu) {
+		bp = *per_cpu_ptr(watch_events, cpu);
+		WRITE_ONCE(bp->attr.bp_addr, watch_addr);
+		WRITE_ONCE(bp->attr.bp_len, watch_len);
+	}
+
+	WRITE_ONCE(myworker.original_cpu, smp_processor_id());
+
+	spin_unlock_irqrestore(&watch_lock, flags);
+
+	/* Then install on all CPUs */
+	/* Run on current CPU directly */
+	queue_work(system_highpri_wq, &myworker.work);
+	ksw_watch_on_local_cpu(NULL);
+	return 0;
+}
+
+void ksw_watch_off(void)
+{
+	pr_info("KSW: Disarming all HWBPs\n");
+	ksw_watch_on((unsigned long)&marker, sizeof(marker));
+	pr_info("KSW: All HWBPs disarmed\n");
+}
-- 
2.43.0

[RFC PATCH 06/13] mm/kstackwatch: Add stack address resolution functions

[Jinchao Wang]

Implement core stack analysis functionality for locating and validating
watch targets within kernel stack frames.

This patch provides the essential stack manipulation functions needed
to translate watch configurations into actual memory addresses:

Key functions:
1. ksw_stack_find_canary() - Locates compiler-generated stack canaries
   by scanning stack frames for the current task's canary value
2. ksw_stack_resolve_offset() - Converts relative stack offsets into
   absolute memory addresses using register state
3. ksw_stack_validate_addr() - Ensures target addresses fall within
   valid stack boundaries to prevent invalid memory access
4. ksw_stack_prepare_watch() - Unified interface that resolves watch
   configurations into concrete addresses and lengths for HWBP setup

Local variable resolution:
- Uses register state (pt_regs) to determine current stack pointer
- Calculates target addresses using SP + configured offset
- Validates addresses against THREAD_SIZE stack boundaries

All functions are marked __maybe_unused to avoid compiler warnings
during incremental development, as they will be integrated with
kprobe handlers in subsequent patches.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/kstackwatch/kstackwatch.h |   5 ++
 mm/kstackwatch/stack.c       | 112 +++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+)

diff --git a/mm/kstackwatch/kstackwatch.h b/mm/kstackwatch/kstackwatch.h
index 910f49014715..c24a651977c0 100644
--- a/mm/kstackwatch/kstackwatch.h
+++ b/mm/kstackwatch/kstackwatch.h
@@ -6,6 +6,7 @@
 
 #define MAX_FUNC_NAME_LEN 64
 #define MAX_CONFIG_STR_LEN 128
+#define MAX_FRAME_SEARCH 128
 
 /* Watch target types */
 enum watch_type {
@@ -38,6 +39,10 @@ struct ksw_config {
 
 extern bool panic_on_catch;
 
+/* stack management */
+int ksw_stack_init(struct ksw_config *config);
+void ksw_stack_exit(void);
+
 /* watch management */
 int ksw_watch_init(struct ksw_config *config);
 void ksw_watch_exit(void);
diff --git a/mm/kstackwatch/stack.c b/mm/kstackwatch/stack.c
index e69de29bb2d1..8b558cdbda97 100644
--- a/mm/kstackwatch/stack.c
+++ b/mm/kstackwatch/stack.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/stackprotector.h>
+#include <linux/kprobes.h>
+#include <asm/stacktrace.h>
+
+#include "kstackwatch.h"
+
+/* Find canary address in current stack frame */
+static unsigned long __maybe_unused ksw_stack_find_canary(struct pt_regs *regs)
+{
+	unsigned long *stack_ptr, *stack_end;
+	unsigned long expected_canary;
+	unsigned int i;
+
+	if (!regs || !regs->sp)
+		return 0;
+
+	stack_ptr = (unsigned long *)regs->sp;
+	stack_end =
+		(unsigned long *)current->stack + THREAD_SIZE / sizeof(long);
+	expected_canary = current->stack_canary;
+
+	for (i = 0; i < MAX_FRAME_SEARCH && &stack_ptr[i] < stack_end; i++) {
+		if (stack_ptr[i] == expected_canary) {
+			pr_info("KSW: Canary found i:%d 0x%px\n", i,
+				&stack_ptr[i]);
+			return (unsigned long)&stack_ptr[i];
+		}
+	}
+
+	return 0;
+}
+
+/* Resolve stack offset to actual address */
+static unsigned long __maybe_unused ksw_stack_resolve_offset(struct pt_regs *regs,
+					      s64 local_var_offset)
+{
+	unsigned long stack_base;
+	unsigned long target_addr;
+
+	if (!regs)
+		return 0;
+
+	/* Use stack pointer as base for offset calculation */
+	stack_base = regs->sp;
+	target_addr = stack_base + local_var_offset;
+
+	pr_info("KSW: %s sp:0x%lx offset: %llx, target: 0x%lx\n", __func__,
+		stack_base, local_var_offset, target_addr);
+
+	return target_addr;
+}
+
+/* Validate that address is within current stack bounds */
+static int __maybe_unused ksw_stack_validate_addr(unsigned long addr, size_t size)
+{
+	unsigned long stack_start, stack_end;
+
+	if (!addr || !size)
+		return -EINVAL;
+
+	stack_start = (unsigned long)current->stack;
+	stack_end = stack_start + THREAD_SIZE;
+
+	if (addr < stack_start || (addr + size) > stack_end) {
+		pr_warn("KSW: Address 0x%lx (size %zu) outside stack bounds [0x%lx-0x%lx]\n",
+			addr, size, stack_start, stack_end);
+		return -ERANGE;
+	}
+
+	return 0;
+}
+
+/* Setup hardware breakpoints for active watches */
+static int __maybe_unused ksw_stack_prepare_watch(struct pt_regs *regs,
+				   struct ksw_config *config, u64 *watch_addr,
+				   u64 *watch_len)
+{
+	u64 addr;
+	u64 len;
+
+	/* Resolve addresses for all active watches */
+	switch (config->type) {
+	case WATCH_CANARY:
+		addr = ksw_stack_find_canary(regs);
+		len = 8;
+		break;
+
+	case WATCH_LOCAL_VAR:
+		addr = ksw_stack_resolve_offset(regs, config->local_var_offset);
+		if (!addr) {
+			pr_err("KSW: Invalid stack var offset %u\n",
+			       config->local_var_offset);
+			return -EINVAL;
+		}
+		if (ksw_stack_validate_addr(addr, config->local_var_len)) {
+			pr_err("KSW: Invalid stack var len %u\n",
+			       config->local_var_len);
+		}
+		len = config->local_var_len;
+		break;
+
+	default:
+		pr_warn("KSW: Unknown watch type %d\n", config->type);
+		return -EINVAL;
+	}
+
+	*watch_addr = addr;
+	*watch_len = len;
+	return 0;
+}
-- 
2.43.0

[RFC PATCH 07/13] mm/kstackwatch: Add kprobe and stack watch control

[Jinchao Wang]

This patch adds the core logic for controlling the kernel
stack watch mechanism using a combination of kprobes and kretprobes.

A kprobe is registered at function + ip_offset to turn on
the hardware breakpoint that monitors the stack. This allows
the tool to begin watching from a specific instruction within
the function. At the same time, a kretprobe is registered to be
triggered when the function returns. Its handler is responsible
for turning off the hardware breakpoint.

By using these two probes, the tool can precisely watch a function's
stack frame for its entire duration. This makes sure the HWBP is active
only when needed, which reduces overhead and avoids accidental triggers.
This also provides a clear and reliable way to manage the HWBP.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/kstackwatch/stack.c | 86 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 82 insertions(+), 4 deletions(-)

diff --git a/mm/kstackwatch/stack.c b/mm/kstackwatch/stack.c
index 8b558cdbda97..ba5280787e8f 100644
--- a/mm/kstackwatch/stack.c
+++ b/mm/kstackwatch/stack.c
@@ -6,8 +6,10 @@
 
 #include "kstackwatch.h"
 
+struct ksw_config *probe_config;
+
 /* Find canary address in current stack frame */
-static unsigned long __maybe_unused ksw_stack_find_canary(struct pt_regs *regs)
+static unsigned long ksw_stack_find_canary(struct pt_regs *regs)
 {
 	unsigned long *stack_ptr, *stack_end;
 	unsigned long expected_canary;
@@ -33,7 +35,7 @@ static unsigned long __maybe_unused ksw_stack_find_canary(struct pt_regs *regs)
 }
 
 /* Resolve stack offset to actual address */
-static unsigned long __maybe_unused ksw_stack_resolve_offset(struct pt_regs *regs,
+static unsigned long ksw_stack_resolve_offset(struct pt_regs *regs,
 					      s64 local_var_offset)
 {
 	unsigned long stack_base;
@@ -53,7 +55,7 @@ static unsigned long __maybe_unused ksw_stack_resolve_offset(struct pt_regs *reg
 }
 
 /* Validate that address is within current stack bounds */
-static int __maybe_unused ksw_stack_validate_addr(unsigned long addr, size_t size)
+static int ksw_stack_validate_addr(unsigned long addr, size_t size)
 {
 	unsigned long stack_start, stack_end;
 
@@ -73,7 +75,7 @@ static int __maybe_unused ksw_stack_validate_addr(unsigned long addr, size_t siz
 }
 
 /* Setup hardware breakpoints for active watches */
-static int __maybe_unused ksw_stack_prepare_watch(struct pt_regs *regs,
+static int ksw_stack_prepare_watch(struct pt_regs *regs,
 				   struct ksw_config *config, u64 *watch_addr,
 				   u64 *watch_len)
 {
@@ -110,3 +112,79 @@ static int __maybe_unused ksw_stack_prepare_watch(struct pt_regs *regs,
 	*watch_len = len;
 	return 0;
 }
+
+/* Kprobe handlers */
+static struct kprobe entry_probe;
+static struct kretprobe exit_probe;
+
+static void ksw_stack_entry_handler(struct kprobe *p, struct pt_regs *regs,
+				    unsigned long flags)
+{
+	int ret;
+	u64 watch_addr;
+	u64 watch_len;
+
+	/* Setup breakpoints for all active watches */
+	ret = ksw_stack_prepare_watch(regs, probe_config, &watch_addr,
+				      &watch_len);
+	if (ret) {
+		pr_err("KSW: Failed to parse watch info: %d\n", ret);
+		return;
+	}
+	ret = ksw_watch_on(watch_addr, watch_len);
+	if (ret) {
+		pr_err("KSW: Failed to arm hwbp: %d\n", ret);
+		return;
+	}
+	pr_info("KSW: Armed for %s at addr:0x%llx len:%llu\n",
+		probe_config->function, watch_addr, watch_len);
+}
+
+/* Function exit handler */
+static int ksw_stack_exit_handler(struct kretprobe_instance *ri,
+				  struct pt_regs *regs)
+{
+	ksw_watch_off();
+	pr_info("KSW: Disarmed for %s\n", probe_config->function);
+
+	return 0;
+}
+
+int ksw_stack_init(struct ksw_config *config)
+{
+	int ret;
+
+	/* Setup entry probe */
+	memset(&entry_probe, 0, sizeof(entry_probe));
+	entry_probe.symbol_name = config->function;
+	entry_probe.offset = config->ip_offset;
+	entry_probe.post_handler = ksw_stack_entry_handler;
+	probe_config = config;
+	ret = register_kprobe(&entry_probe);
+	if (ret < 0) {
+		pr_err("KSW: Failed to register kprobe ret %d\n", ret);
+		return ret;
+	}
+
+	/* Setup exit probe */
+	memset(&exit_probe, 0, sizeof(exit_probe));
+	exit_probe.kp.symbol_name = config->function;
+	exit_probe.handler = ksw_stack_exit_handler;
+	exit_probe.maxactive = 20;
+
+	ret = register_kretprobe(&exit_probe);
+	if (ret < 0) {
+		pr_err("KSW: Failed to register exit probe for %s: %d\n",
+		       probe_config->function, ret);
+		unregister_kprobe(&entry_probe);
+		return ret;
+	}
+
+	return 0;
+}
+
+void ksw_stack_exit(void)
+{
+	unregister_kretprobe(&exit_probe);
+	unregister_kprobe(&entry_probe);
+}
-- 
2.43.0

[RFC PATCH 08/13] mm/kstackwatch: Wire up watch and stack subsystems in module core

[Jinchao Wang]

Connect the watch and stack functions to complete
the kstackwatch initialization sequence with proper ordering and
error handling.

This patch integrates the previously implemented components:

Initialization sequence:
1. ksw_watch_init() - Pre-allocate HWBP on all CPUs (must be first)
2. ksw_stack_init() - Register kprobes for function entry/exit
3. Set watching_active flag to enable operation

The ordering is critical because:
- HWBP must be pre-allocated before kprobes are registered
- Kprobe handlers depend on pre-existing HWBP infrastructure

Cleanup sequence:
1. ksw_stack_exit() - Unregister kprobes (stops new activations)
2. ksw_watch_exit() - Release pre-allocated HWBP resources
3. Clear watching_active flag

This completes the functional kstackwatch implementation, enabling
real-time stack corruption detection through the proc interface
with automatic HWBP management.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/kstackwatch/kernel.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/mm/kstackwatch/kernel.c b/mm/kstackwatch/kernel.c
index 726cf3f25888..b6366808e891 100644
--- a/mm/kstackwatch/kernel.c
+++ b/mm/kstackwatch/kernel.c
@@ -26,11 +26,29 @@ MODULE_PARM_DESC(panic_on_catch,
 
 static int start_watching(void)
 {
+	int ret;
+
 	if (strlen(ksw_config->function) == 0) {
 		pr_err("KSW: No target function specified\n");
 		return -EINVAL;
 	}
 
+	/*
+	 * watch init will prealloc HWBP
+	 * so it must be before stack init
+	 */
+	ret = ksw_watch_init(ksw_config);
+	if (ret) {
+		pr_err("KSW: ksw_watch_init ret: %d\n", ret);
+		return ret;
+	}
+
+	ret = ksw_stack_init(ksw_config);
+	if (ret) {
+		pr_err("KSW: ksw_stack_init ret: %d\n", ret);
+		ksw_watch_exit();
+		return ret;
+	}
 	watching_active = true;
 
 	pr_info("KSW: start watching %s\n", ksw_config->config_str);
@@ -39,6 +57,8 @@ static int start_watching(void)
 
 static void stop_watching(void)
 {
+	ksw_stack_exit();
+	ksw_watch_exit();
 	watching_active = false;
 
 	pr_info("KSW: stop watching %s\n", ksw_config->config_str);
-- 
2.43.0

[RFC PATCH 09/13] mm/kstackwatch: Add architecture support validation

[Jinchao Wang]

Add architecture compatibility checking during module initialization
to ensure kstackwatch operates only on supported platforms.

Currently supported architectures:
- x86_64: Full hardware breakpoint support with arch_reinstall_hw_breakpoint()

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/kstackwatch/kernel.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/mm/kstackwatch/kernel.c b/mm/kstackwatch/kernel.c
index b6366808e891..9f01e0222210 100644
--- a/mm/kstackwatch/kernel.c
+++ b/mm/kstackwatch/kernel.c
@@ -189,8 +189,31 @@ static const struct proc_ops kstackwatch_proc_ops = {
 	.proc_release = single_release,
 };
 
+static int is_ksw_supported(void)
+{
+	static const char *const supported_archs[] = { "x86_64", NULL };
+
+	const char *current_arch = utsname()->machine;
+	int i;
+
+	for (i = 0; supported_archs[i] != NULL; i++) {
+		if (strcmp(current_arch, supported_archs[i]) == 0) {
+			pr_info("KSW: Architecture %s supports hardware breakpoints\n",
+				current_arch);
+			return 1;
+		}
+	}
+
+	pr_warn("KSW: Architecture %s may not support hardware breakpoints\n",
+		current_arch);
+	return 1;
+}
+
 static int __init kstackwatch_init(void)
 {
+	if (!is_ksw_supported())
+		return -EOPNOTSUPP;
+
 	ksw_config = kmalloc(sizeof(*ksw_config), GFP_KERNEL);
 	if (!ksw_config)
 		return -ENOMEM;
-- 
2.43.0

[RFC PATCH 10/13] mm/kstackwatch: Handle nested function calls

[Jinchao Wang]

This patch adds support for watching specific depths of nested function calls.
The Kernel Stack Watch tool currently watchs the first entry of a probed
function, but this is insufficient for functions that are called recursively or
that call themselves indirectly.

To address this, a per-CPU variable `monitor_depth` is introduced to track
the nesting level of the function being watched. When a function's entry
handler is triggered, the depth is incremented, and the tool only arms the
hardware breakpoint if the current depth matches the configured `depth`.
The exit handler decrements the depth, disarming the breakpoint on the
correct return.

This ensures the watch mechanism only activates at the desired nesting level,
making the tool more precise and versatile for debugging complex stack issues.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/kstackwatch/stack.c | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/mm/kstackwatch/stack.c b/mm/kstackwatch/stack.c
index ba5280787e8f..86847c2e4506 100644
--- a/mm/kstackwatch/stack.c
+++ b/mm/kstackwatch/stack.c
@@ -6,6 +6,8 @@
 
 #include "kstackwatch.h"
 
+/* Per-CPU watching state */
+static DEFINE_PER_CPU(int, monitor_depth);
 struct ksw_config *probe_config;
 
 /* Find canary address in current stack frame */
@@ -120,10 +122,22 @@ static struct kretprobe exit_probe;
 static void ksw_stack_entry_handler(struct kprobe *p, struct pt_regs *regs,
 				    unsigned long flags)
 {
+	int *depth, cur_depth;
 	int ret;
 	u64 watch_addr;
 	u64 watch_len;
 
+	/* Handle nested calls - only monitor outermost */
+	depth = this_cpu_ptr(&monitor_depth);
+	cur_depth = (*depth)++;
+
+	if (cur_depth != probe_config->depth) {
+		/* depth start from 0 */
+		pr_info("KSW: config_depth:%u cur_depth:%d skipping %s\n",
+			probe_config->depth, cur_depth, __func__);
+		return;
+	}
+
 	/* Setup breakpoints for all active watches */
 	ret = ksw_stack_prepare_watch(regs, probe_config, &watch_addr,
 				      &watch_len);
@@ -136,14 +150,25 @@ static void ksw_stack_entry_handler(struct kprobe *p, struct pt_regs *regs,
 		pr_err("KSW: Failed to arm hwbp: %d\n", ret);
 		return;
 	}
-	pr_info("KSW: Armed for %s at addr:0x%llx len:%llu\n",
-		probe_config->function, watch_addr, watch_len);
+	pr_info("KSW: Armed for %s at depth %d addr:0x%llx len:%llu\n",
+		probe_config->function, cur_depth, watch_addr, watch_len);
 }
 
 /* Function exit handler */
 static int ksw_stack_exit_handler(struct kretprobe_instance *ri,
 				  struct pt_regs *regs)
 {
+	int *depth, cur_depth;
+
+	depth = this_cpu_ptr(&monitor_depth);
+	cur_depth = --(*depth);
+	if (cur_depth != probe_config->depth) {
+		/* depth start from 0 */
+		pr_info("KSW: %s config depth:%u cur_depth:%d skipping\n",
+			__func__, probe_config->depth, cur_depth);
+		return 0;
+	}
+
 	ksw_watch_off();
 	pr_info("KSW: Disarmed for %s\n", probe_config->function);
 
@@ -153,6 +178,13 @@ static int ksw_stack_exit_handler(struct kretprobe_instance *ri,
 int ksw_stack_init(struct ksw_config *config)
 {
 	int ret;
+	int cpu;
+	int *depth;
+
+	for_each_possible_cpu(cpu) {
+		depth = per_cpu_ptr(&monitor_depth, cpu);
+		WRITE_ONCE(*depth, 0);
+	}
 
 	/* Setup entry probe */
 	memset(&entry_probe, 0, sizeof(entry_probe));
-- 
2.43.0

[RFC PATCH 11/13] mm/kstackwatch: Ignore corruption in kretprobe trampolines

[Jinchao Wang]

This patch adds logic to prevent the KStackWatch tool from
triggering on events that occur inside `kretprobe` trampolines.

The `kretprobe` mechanism temporarily diverts execution through a
trampoline to handle function returns. When a HWBP is set.
It can be triggered within this trampoline after the watched function
exit, and before the exit_handler called, leading to a false positive.

The patch resolves the address range of the `arch_rethook_trampoline`
and checks the call stack within the breakpoint handler to see if the
event's instruction pointer (IP) is within this range. If it is, the
event is ignored.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/kstackwatch/watch.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/mm/kstackwatch/watch.c b/mm/kstackwatch/watch.c
index 7ab247531961..1b4cf5d42db9 100644
--- a/mm/kstackwatch/watch.c
+++ b/mm/kstackwatch/watch.c
@@ -31,11 +31,48 @@ static void ksw_watch_on_local_cpu(void *useless);
 static DEFINE_PER_CPU(call_single_data_t,
 		      hwbp_csd) = CSD_INIT(ksw_watch_on_local_cpu, NULL);
 
+/* Resolved once, then reused */
+static unsigned long tramp_start, tramp_end;
+
+static void ksw_watch_resolve_trampolines(void)
+{
+	unsigned long sz, off;
+
+	if (likely(tramp_start && tramp_end))
+		return;
+
+	tramp_start = kallsyms_lookup_name("arch_rethook_trampoline");
+	if (tramp_start && kallsyms_lookup_size_offset(tramp_start, &sz, &off))
+		tramp_end = tramp_start + sz;
+}
+
+static bool ksw_watch_in_trampoline(unsigned long ip)
+{
+	if (tramp_start && tramp_end && ip >= tramp_start && ip < tramp_end)
+		return true;
+	return false;
+}
+
 /* Enhanced breakpoint handler with watch identification */
 static void ksw_watch_handler(struct perf_event *bp,
 			      struct perf_sample_data *data,
 			      struct pt_regs *regs)
 {
+	unsigned long entries[MAX_STACK_ENTRIES];
+	int i, nr = 0;
+
+	ksw_watch_resolve_trampolines();
+
+#if IS_ENABLED(CONFIG_STACKTRACE)
+	nr = stack_trace_save_regs(regs, entries, MAX_STACK_ENTRIES, 0);
+	for (i = 0; i < nr; i++) {
+		if (ksw_watch_in_trampoline(entries[i])) {
+			pr_info("KSW: Found rethook trampolines, ignoring hit\n");
+			return;
+		}
+	}
+#endif
+
 	pr_emerg("========== KStackWatch: Caught stack corruption =======\n");
 	pr_emerg("KSW: config %s\n", watch_config->config_str);
 	show_regs(regs);
-- 
2.43.0

[RFC PATCH 12/13] mm/kstackwatch: Add debug and test functions

[Jinchao Wang]

This patch adds new helper functions to the kstackwatch module to aid in
debugging and testing.

- `ksw_watch_show()`: Displays information about the currently active hardware
  breakpoint, such as its address and length. This is useful for verifying
  that a breakpoint has been correctly installed.
- `ksw_watch_fire()`: Manually triggers the active hardware breakpoint. This
  function is for testing the breakpoint handler to ensure it correctly
  identifies and reports the stack corruption.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/kstackwatch/kstackwatch.h |  4 ++++
 mm/kstackwatch/watch.c       | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/mm/kstackwatch/kstackwatch.h b/mm/kstackwatch/kstackwatch.h
index c24a651977c0..27c71a9391ac 100644
--- a/mm/kstackwatch/kstackwatch.h
+++ b/mm/kstackwatch/kstackwatch.h
@@ -2,6 +2,8 @@
 #ifndef _KSTACKWATCH_H
 #define _KSTACKWATCH_H
 
+#include <linux/kprobes.h>
+#include <linux/perf_event.h>
 #include <linux/types.h>
 
 #define MAX_FUNC_NAME_LEN 64
@@ -48,5 +50,7 @@ int ksw_watch_init(struct ksw_config *config);
 void ksw_watch_exit(void);
 int ksw_watch_on(u64 watch_addr, u64 watch_len);
 void ksw_watch_off(void);
+void ksw_watch_show(void);
+void ksw_watch_fire(void);
 
 #endif /* _KSTACKWATCH_H */
diff --git a/mm/kstackwatch/watch.c b/mm/kstackwatch/watch.c
index 1b4cf5d42db9..28ea24e4ae3a 100644
--- a/mm/kstackwatch/watch.c
+++ b/mm/kstackwatch/watch.c
@@ -218,3 +218,23 @@ void ksw_watch_off(void)
 	ksw_watch_on((unsigned long)&marker, sizeof(marker));
 	pr_info("KSW: All HWBPs disarmed\n");
 }
+
+/* Debug functions */
+void ksw_watch_show(void)
+{
+	struct perf_event *bp;
+
+	bp = *this_cpu_ptr(watch_events);
+	pr_info("KSW: HWBP info test - bp_addr: 0x%px len:%llu\n",
+		(void *)bp->attr.bp_addr, bp->attr.bp_len);
+}
+
+void ksw_watch_fire(void)
+{
+	struct perf_event *bp;
+	char *ptr;
+
+	bp = *this_cpu_ptr(watch_events);
+	ptr = (char *)READ_ONCE(bp->attr.bp_addr);
+	*ptr = 0x42; // This should trigger immediately
+}
-- 
2.43.0

[RFC PATCH 13/13] mm/kstackwatch: Add a test module and script

[Jinchao Wang]

This patch introduces a test module and a helper script to verify the
functionality of the KStackWatch debugging tool.

The `kstackwatch_test.c` module provides a set of controlled scenarios for
stack-related issues, including:
- Direct write using ksw_watch_fire().
- Intentional stack overflow.
- Corruption of a local variable from another thread.
- Recursive function calls to test nested stack frames.

The accompanying `kstackwatch_test.sh` script automates the process of
loading the test module, configuring the KStackWatch tool via its debugfs
interface, and triggering each test case. This script also includes
instructions for users to find the necessary offsets for their specific
kernel build.

This dedicated test module and script significantly aid in the development,
validation, and long-term maintenance of the KStackWatch tool.

Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
 mm/Kconfig.debug                      |  11 ++
 mm/kstackwatch/Makefile               |   8 +
 mm/kstackwatch/kstackwatch_test.c     | 237 ++++++++++++++++++++++++++
 tools/kstackwatch/kstackwatch_test.sh | 122 +++++++++++++
 4 files changed, 378 insertions(+)
 create mode 100644 mm/kstackwatch/kstackwatch_test.c
 create mode 100644 tools/kstackwatch/kstackwatch_test.sh

diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
index dd9c1bb7f549..5e9402e39001 100644
--- a/mm/Kconfig.debug
+++ b/mm/Kconfig.debug
@@ -321,3 +321,14 @@ config KSTACK_WATCH
 	  which is useful for debugging stability issues.
 
 	  If unsure, say N.
+
+
+config KSTACK_WATCH_TEST
+	tristate "KStackWatch Test Module"
+	depends on KSTACK_WATCH
+	help
+	  This module provides controlled stack exhaustion and overflow scenarios
+	  to verify the functionality of KStackWatch. It is particularly useful
+	  for development and validation of the KStachWatch mechanism.
+
+	  If unsure, say N.
diff --git a/mm/kstackwatch/Makefile b/mm/kstackwatch/Makefile
index 076822eb7661..5e63e8c2e7c0 100644
--- a/mm/kstackwatch/Makefile
+++ b/mm/kstackwatch/Makefile
@@ -1,3 +1,11 @@
 obj-$(CONFIG_KSTACK_WATCH)	+= kstackwatch.o
 
 kstackwatch-y := kernel.o stack.o watch.o
+
+obj-$(CONFIG_KSTACK_WATCH_TEST)	+= kstackwatch_test.o
+
+kstackwatch-y := kernel.o stack.o watch.o
+CFLAGS_kstackwatch_test.o := -fno-ipa-sra -fno-inline \
+                             -fno-optimize-sibling-calls \
+                             -fno-section-anchors \
+                             -fno-pic -fno-pie
diff --git a/mm/kstackwatch/kstackwatch_test.c b/mm/kstackwatch/kstackwatch_test.c
new file mode 100644
index 000000000000..c21618ca0c8b
--- /dev/null
+++ b/mm/kstackwatch/kstackwatch_test.c
@@ -0,0 +1,237 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/proc_fs.h>
+#include <linux/uaccess.h>
+#include <linux/string.h>
+#include <linux/kthread.h>
+#include <linux/delay.h>
+#include <linux/sched.h>
+
+#include "kstackwatch.h"
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jinchao Wang");
+MODULE_DESCRIPTION("Simplified KStackWatch Test Module ");
+
+static struct proc_dir_entry *test_proc;
+#define BUFFER_SIZE 4
+#define MAX_DEPTH 4
+
+/* Global variables for multi-thread test synchronization */
+static u64 *g_corrupt_ptr;
+static struct completion g_wait_for_init;
+
+/*
+ * Test Case 0: Write to the canary position directly (Canary Test)
+ * This function uses a u64 buffer
+ * 64-bit write, corrupting the stack canary with a single operation.
+ */
+static void canary_test_write(void)
+{
+	u64 buffer[BUFFER_SIZE];
+
+	pr_info("KStackWatch Test: Starting %s with u64 write\n", __func__);
+	ksw_watch_show();
+	ksw_watch_fire();
+
+	buffer[0] = 0;
+	/* make sure the compiler do not drop assign action */
+	barrier_data(buffer);
+	pr_info("KStackWatch Test: Canary write test completed\n");
+}
+
+/*
+ * Test Case 1: Stack Overflow (Canary Test)
+ * This function uses a u64 buffer
+ * 64-bit write, corrupting the stack canary with a single operation.
+ */
+static void canary_test_overflow(void)
+{
+	u64 buffer[BUFFER_SIZE];
+
+	pr_info("KStackWatch Test: Starting %s with u64 write\n", __func__);
+	pr_info("KStackWatch Test: buffer 0x%px\n", buffer);
+
+	/* Intentionally overflow the u64 buffer. */
+	buffer[BUFFER_SIZE] = 0xdeadbeefdeadbeef;
+
+	/* make sure the compiler do not drop assign action */
+	barrier_data(buffer);
+
+	pr_info("KStackWatch Test: Canary overflow test completed\n");
+}
+
+/*
+ * Corrupts the variable on multi_thread_corruption_test's stack
+ */
+static int multi_thread_corruption_thread2(void *data)
+{
+	pr_info("KStackWatch Test: Starting %s\n", __func__);
+
+	wait_for_completion(&g_wait_for_init);
+
+	pr_info("KStackWatch Test: Thread2  woke up. Corrupting variable at 0x%px\n",
+		g_corrupt_ptr);
+	if (g_corrupt_ptr)
+		*g_corrupt_ptr = 0xdeadbeefdeadbeef;
+
+	pr_info("KStackWatch Test: Thread2 finished corruption\n");
+
+	return 0;
+}
+
+/*
+ * Test Case 2: Multi-threaded Local Variable Corruption
+ * multi_thread_corruption_test initializes a local variable
+ * makes its address globally available,
+ * and then sleeps. Thread B corrupts it.
+ */
+static void multi_thread_corruption_thread1(void)
+{
+	u64 local_var = 0x1234567887654321;
+
+	pr_info("KStackWatch Test: Starting %s\n", __func__);
+
+	pr_info("KStackWatch Test: Thread1 local_var address: 0x%px, value: 0x%llx\n",
+		&local_var, local_var);
+	WRITE_ONCE(g_corrupt_ptr, &local_var);
+
+	/* Signal Thread 2 that the pointer is ready, then sleep */
+	complete(&g_wait_for_init);
+	msleep(1000);
+
+	pr_info("KStackWatch Test: Thread1 woke up. Final local_var value: 0x%llx\n",
+		local_var);
+}
+
+static void multi_thread_corruption_test(void)
+{
+	struct task_struct *corrupt_thread;
+
+	pr_info("KStackWatch Test: Starting %s\n", __func__);
+
+	/* Reset completion */
+	init_completion(&g_wait_for_init);
+
+	corrupt_thread = kthread_run(multi_thread_corruption_thread2, NULL,
+				     "corruption_b");
+	if (IS_ERR(corrupt_thread)) {
+		pr_err("KStackWatch Test: Failed to create corruption thread\n");
+		return;
+	}
+	multi_thread_corruption_thread1();
+}
+
+/*
+ * Test Case 3: Recursive Call Corruption
+ * This function calls itself recursively and corrupts the stack at a specific depth.
+ * This tests whether KStackWatch can handle dynamic stack frames.
+ */
+static void recursive_corruption_test(int depth)
+{
+	u64 buffer[BUFFER_SIZE];
+
+	pr_info("KStackWatch Test: Recursive call at depth %d\n", depth);
+	pr_info("KStackWatch Test: buffer 0x%px\n", buffer);
+	if (depth <= MAX_DEPTH)
+		recursive_corruption_test(depth + 1);
+
+	buffer[0] = depth;
+
+	/* make sure the compiler do not drop assign action */
+	barrier_data(buffer);
+
+	pr_info("KStackWatch Test: Returning from depth %d\n", depth);
+}
+
+static ssize_t test_proc_write(struct file *file, const char __user *buffer,
+			       size_t count, loff_t *pos)
+{
+	char cmd[256];
+	int test_num;
+
+	if (count >= sizeof(cmd))
+		return -EINVAL;
+
+	if (copy_from_user(cmd, buffer, count))
+		return -EFAULT;
+
+	cmd[count] = '\0';
+	strim(cmd);
+
+	pr_info("KStackWatch Test: Received command: %s\n", cmd);
+
+	if (sscanf(cmd, "test%d", &test_num) == 1) {
+		switch (test_num) {
+		case 0:
+			pr_info("KStackWatch Test: Triggering canary write test\n");
+			canary_test_write();
+			break;
+		case 1:
+			pr_info("KStackWatch Test: Triggering canary overflow test\n");
+			canary_test_overflow();
+			break;
+		case 2:
+			pr_info("KStackWatch Test: Triggering local variable corruption test\n");
+			multi_thread_corruption_test();
+			break;
+		case 3:
+			pr_info("KStackWatch Test: Triggering recursive corruption test\n");
+			/* depth start with 0 */
+			recursive_corruption_test(0);
+			break;
+		default:
+			pr_err("KStackWatch Test: Unknown test number %d\n",
+			       test_num);
+			return -EINVAL;
+		}
+	} else {
+		pr_err("KStackWatch Test: Invalid command format. Use 'test1', 'test2', or 'test3'.\n");
+		return -EINVAL;
+	}
+
+	return count;
+}
+
+static ssize_t test_proc_read(struct file *file, char __user *buffer,
+			      size_t count, loff_t *pos)
+{
+	static const char usage[] =
+		"KStackWatch Simplified Test Module\n"
+		"==================================\n"
+		"Usage:\n"
+		"  echo 'test0' > /proc/kstackwatch_test  - Canary test test\n"
+		"  echo 'test1' > /proc/kstackwatch_test  - Canary overflow test\n"
+		"  echo 'test2' > /proc/kstackwatch_test  - Multi-threaded local variable corruption test\n"
+		"  echo 'test3' > /proc/kstackwatch_test  - Recursive corruption test\n";
+
+	return simple_read_from_buffer(buffer, count, pos, usage,
+				       strlen(usage));
+}
+
+static const struct proc_ops test_proc_ops = {
+	.proc_read = test_proc_read,
+	.proc_write = test_proc_write,
+};
+
+static int __init kstackwatch_test_init(void)
+{
+	test_proc = proc_create("kstackwatch_test", 0644, NULL, &test_proc_ops);
+	if (!test_proc) {
+		pr_err("KStackWatch Test: Failed to create proc entry\n");
+		return -ENOMEM;
+	}
+	pr_info("KStackWatch Test: Module loaded, use 'cat /proc/kstackwatch_test' for usage\n");
+	return 0;
+}
+
+static void __exit kstackwatch_test_exit(void)
+{
+	if (test_proc)
+		remove_proc_entry("kstackwatch_test", NULL);
+	pr_info("KStackWatch Test: Module unloaded\n");
+}
+
+module_init(kstackwatch_test_init);
+module_exit(kstackwatch_test_exit);
diff --git a/tools/kstackwatch/kstackwatch_test.sh b/tools/kstackwatch/kstackwatch_test.sh
new file mode 100644
index 000000000000..33ca590b7374
--- /dev/null
+++ b/tools/kstackwatch/kstackwatch_test.sh
@@ -0,0 +1,122 @@
+#!/bin/bash
+
+# --- Usage function ---
+usage() {
+    echo "======================================"
+    echo "  KStackWatch Test Script Usage"
+    echo "======================================"
+    echo ""
+    echo "IMPORTANT: Before running, make sure you have updated the offset values!"
+    echo ""
+    echo "To find your offsets, use objdump:"
+    echo "  objdump -S --disassemble=canary_test_write vmlinux"
+    echo ""
+    echo "Then search for your function names to find the instruction addresses."
+    echo "- Instruction offset: address relative to function's start"
+    echo "- Stack var offset: distance from stack base (%rbp) to the variable"
+    echo ""
+    echo "Usage: $0 [test_case_number]"
+    echo ""
+    echo "Available test cases:"
+    echo "  0  - Canary Write Test"
+    echo "  1  - Canary Overflow Test"
+    echo "  2  - Multi-threaded Local Variable Corruption Test"
+    echo "  3  - Recursive Corruption Test"
+    echo ""
+    echo "======================================"
+    echo ""
+}
+
+# --- Interactive menu ---
+show_menu() {
+    echo "Select a test case to run:"
+    echo "  0) Canary Write Test"
+    echo "  1) Canary Overflow Test"
+    echo "  2) Multi-threaded Local Variable Corruption Test"
+    echo "  3) Recursive Corruption Test"
+    echo "  q) Quit"
+    echo ""
+    echo "WARNING: Each test may cause system crash/hang!"
+    echo ""
+    read -p "Enter your choice [0-3/q]: " choice
+    echo ""
+
+    case "$choice" in
+        0) test0 ;;
+        1) test1 ;;
+        2) test2 ;;
+        3) test3 ;;
+        q|Q) echo "Exiting..."; exit 0 ;;
+        *) echo "Invalid choice. Please try again."; echo ""; show_menu ;;
+    esac
+}
+
+# --- Test Case 0: Canary Write ---
+test0() {
+    echo "=== Running Test Case 0: Canary Write ==="
+    # function+instruction_off[+depth] [local_var_offset:local_var_len]
+    echo "canary_test_write+0x19" > /proc/kstackwatch
+    echo "test0" > /proc/kstackwatch_test
+    echo ""
+    echo "-------------------------------------"
+    echo ""
+}
+
+# --- Test Case 1: Canary Overflow ---
+test1() {
+    echo "=== Running Test Case 1: Canary Overflow ==="
+    # function+instruction_off[+depth] [local_var_offset:local_var_len]
+    echo "canary_test_overflow+0x19" > /proc/kstackwatch
+    echo "test1" > /proc/kstackwatch_test
+    echo ""
+    echo "-------------------------------------"
+    echo ""
+}
+
+# --- Test Case 2: Multi-threaded Local Variable Corruption ---
+test2() {
+    echo "=== Running Test Case 2: Multi-threaded Corruption ==="
+    # function+instruction_off[+depth] [local_var_offset:local_var_len]
+    echo "multi_thread_corruption_thread1+0x2b 0:8" > /proc/kstackwatch
+    echo "test2" > /proc/kstackwatch_test
+    echo ""
+    echo "-------------------------------------"
+    echo ""
+}
+
+# --- Test Case 3: Recursive Corruption ---
+test3() {
+    echo "=== Running Test Case 3: Recursive Corruption ==="
+    # function+instruction_off[+depth] [local_var_offset:local_var_len]
+    echo "recursive_corruption_test+0x2b+3 0:8"  > /proc/kstackwatch
+    echo "test3" > /proc/kstackwatch_test
+    echo ""
+    echo "-------------------------------------"
+    echo ""
+}
+
+# --- Run all tests ---
+# Removed: These tests cause system crashes and should not be run sequentially
+
+echo 7 > /proc/sys/kernel/printk
+
+# --- Main ---
+if [ -z "$1" ]; then
+    usage
+    echo ""
+    show_menu
+else
+    case "$1" in
+        0) test0 ;;
+        1) test1 ;;
+        2) test2 ;;
+        3) test3 ;;
+        help|--help|-h) usage ;;
+        *)
+            echo "Error: Invalid argument '$1'"
+            echo ""
+            usage
+            exit 1
+            ;;
+    esac
+fi
-- 
2.43.0

Re: [RFC PATCH 07/13] mm/kstackwatch: Add kprobe and stack watch control

[Masami Hiramatsu]

On Mon, 18 Aug 2025 20:26:12 +0800
Jinchao Wang <wangjinchao600@gmail.com> wrote:

> This patch adds the core logic for controlling the kernel
> stack watch mechanism using a combination of kprobes and kretprobes.
> 
> A kprobe is registered at function + ip_offset to turn on
> the hardware breakpoint that monitors the stack. This allows
> the tool to begin watching from a specific instruction within
> the function. At the same time, a kretprobe is registered to be
> triggered when the function returns. Its handler is responsible
> for turning off the hardware breakpoint.

Could you use fprobe instead of kretprobe for hooking function exit?
kretprobe is an old feature and will be replaced by fprobe.

You can find an example in samples/fprobe/fprobe_example.c

Thank you,

> 
> By using these two probes, the tool can precisely watch a function's
> stack frame for its entire duration. This makes sure the HWBP is active
> only when needed, which reduces overhead and avoids accidental triggers.
> This also provides a clear and reliable way to manage the HWBP.
> 
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
> ---
>  mm/kstackwatch/stack.c | 86 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 82 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/kstackwatch/stack.c b/mm/kstackwatch/stack.c
> index 8b558cdbda97..ba5280787e8f 100644
> --- a/mm/kstackwatch/stack.c
> +++ b/mm/kstackwatch/stack.c
> @@ -6,8 +6,10 @@
>  
>  #include "kstackwatch.h"
>  
> +struct ksw_config *probe_config;
> +
>  /* Find canary address in current stack frame */
> -static unsigned long __maybe_unused ksw_stack_find_canary(struct pt_regs *regs)
> +static unsigned long ksw_stack_find_canary(struct pt_regs *regs)
>  {
>  	unsigned long *stack_ptr, *stack_end;
>  	unsigned long expected_canary;
> @@ -33,7 +35,7 @@ static unsigned long __maybe_unused ksw_stack_find_canary(struct pt_regs *regs)
>  }
>  
>  /* Resolve stack offset to actual address */
> -static unsigned long __maybe_unused ksw_stack_resolve_offset(struct pt_regs *regs,
> +static unsigned long ksw_stack_resolve_offset(struct pt_regs *regs,
>  					      s64 local_var_offset)
>  {
>  	unsigned long stack_base;
> @@ -53,7 +55,7 @@ static unsigned long __maybe_unused ksw_stack_resolve_offset(struct pt_regs *reg
>  }
>  
>  /* Validate that address is within current stack bounds */
> -static int __maybe_unused ksw_stack_validate_addr(unsigned long addr, size_t size)
> +static int ksw_stack_validate_addr(unsigned long addr, size_t size)
>  {
>  	unsigned long stack_start, stack_end;
>  
> @@ -73,7 +75,7 @@ static int __maybe_unused ksw_stack_validate_addr(unsigned long addr, size_t siz
>  }
>  
>  /* Setup hardware breakpoints for active watches */
> -static int __maybe_unused ksw_stack_prepare_watch(struct pt_regs *regs,
> +static int ksw_stack_prepare_watch(struct pt_regs *regs,
>  				   struct ksw_config *config, u64 *watch_addr,
>  				   u64 *watch_len)
>  {
> @@ -110,3 +112,79 @@ static int __maybe_unused ksw_stack_prepare_watch(struct pt_regs *regs,
>  	*watch_len = len;
>  	return 0;
>  }
> +
> +/* Kprobe handlers */
> +static struct kprobe entry_probe;
> +static struct kretprobe exit_probe;
> +
> +static void ksw_stack_entry_handler(struct kprobe *p, struct pt_regs *regs,
> +				    unsigned long flags)
> +{
> +	int ret;
> +	u64 watch_addr;
> +	u64 watch_len;
> +
> +	/* Setup breakpoints for all active watches */
> +	ret = ksw_stack_prepare_watch(regs, probe_config, &watch_addr,
> +				      &watch_len);
> +	if (ret) {
> +		pr_err("KSW: Failed to parse watch info: %d\n", ret);
> +		return;
> +	}
> +	ret = ksw_watch_on(watch_addr, watch_len);
> +	if (ret) {
> +		pr_err("KSW: Failed to arm hwbp: %d\n", ret);
> +		return;
> +	}
> +	pr_info("KSW: Armed for %s at addr:0x%llx len:%llu\n",
> +		probe_config->function, watch_addr, watch_len);
> +}
> +
> +/* Function exit handler */
> +static int ksw_stack_exit_handler(struct kretprobe_instance *ri,
> +				  struct pt_regs *regs)
> +{
> +	ksw_watch_off();
> +	pr_info("KSW: Disarmed for %s\n", probe_config->function);
> +
> +	return 0;
> +}
> +
> +int ksw_stack_init(struct ksw_config *config)
> +{
> +	int ret;
> +
> +	/* Setup entry probe */
> +	memset(&entry_probe, 0, sizeof(entry_probe));
> +	entry_probe.symbol_name = config->function;
> +	entry_probe.offset = config->ip_offset;
> +	entry_probe.post_handler = ksw_stack_entry_handler;
> +	probe_config = config;
> +	ret = register_kprobe(&entry_probe);
> +	if (ret < 0) {
> +		pr_err("KSW: Failed to register kprobe ret %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Setup exit probe */
> +	memset(&exit_probe, 0, sizeof(exit_probe));
> +	exit_probe.kp.symbol_name = config->function;
> +	exit_probe.handler = ksw_stack_exit_handler;
> +	exit_probe.maxactive = 20;
> +
> +	ret = register_kretprobe(&exit_probe);
> +	if (ret < 0) {
> +		pr_err("KSW: Failed to register exit probe for %s: %d\n",
> +		       probe_config->function, ret);
> +		unregister_kprobe(&entry_probe);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +void ksw_stack_exit(void)
> +{
> +	unregister_kretprobe(&exit_probe);
> +	unregister_kprobe(&entry_probe);
> +}
> -- 
> 2.43.0
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [RFC PATCH 07/13] mm/kstackwatch: Add kprobe and stack watch control

[Jinchao Wang]

On 8/25/25 18:31, Masami Hiramatsu (Google) wrote:
> On Mon, 18 Aug 2025 20:26:12 +0800
> Jinchao Wang <wangjinchao600@gmail.com> wrote:
> 
>> This patch adds the core logic for controlling the kernel
>> stack watch mechanism using a combination of kprobes and kretprobes.
>>
>> A kprobe is registered at function + ip_offset to turn on
>> the hardware breakpoint that monitors the stack. This allows
>> the tool to begin watching from a specific instruction within
>> the function. At the same time, a kretprobe is registered to be
>> triggered when the function returns. Its handler is responsible
>> for turning off the hardware breakpoint.
> 
> Could you use fprobe instead of kretprobe for hooking function exit?
> kretprobe is an old feature and will be replaced by fprobe.
> 
> You can find an example in samples/fprobe/fprobe_example.c
> 
> Thank you,
Hi Masami,
Thanks for the feedback.
I will respin the patchset and replace the kretprobe with fprobe, as you
advised.

-- 
Best regards,
Jinchao

Re: [RFC PATCH 02/13] x86/HWBP: Add arch_reinstall_hw_breakpoint() for atomic updates

[Masami Hiramatsu]

Hi Jinchao,

On Mon, 18 Aug 2025 20:26:07 +0800
Jinchao Wang <wangjinchao600@gmail.com> wrote:

> Add arch_reinstall_hw_breakpoint() to enable atomic context modification
> of hardware breakpoint parameters without deallocating and reallocating
> the breakpoint slot.
> 
> The existing arch_install_hw_breakpoint() allocates a new debug register
> slot, while arch_uninstall_hw_breakpoint() deallocates it. However, some
> use cases require modifying breakpoint parameters (address, length, type)
> atomically without losing the allocated slot, particularly when operating
> in atomic contexts where allocation might fail or be unavailable.
> 
> This is particularly useful for debugging tools like kstackwatch that
> need to dynamically update breakpoint targets in atomic contexts while
> maintaining consistent hardware state.
> 

I'm also trying to find this interface for my wprobe. So the idea is good.
But this looks hacky and only for x86. I think the interface should be
more generic and do not use this arch internal function directly.

It seems that the slot is allocated by "type", thus, if this reinstall
hwbp without deallocate/allocate slot, it must NOT change the type.
See __modify_bp_slot. Also, provide CONFIG_HAVE_... option for checking
whether the architecture support that interface.

Let me share a patch to wrap it.
I'll send my series with this patch.

From 9813e9d003f4691b5fe408094a02bd9e6a5fa249 Mon Sep 17 00:00:00 2001
From: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Date: Mon, 25 Aug 2025 19:37:24 +0900
Subject: [PATCH] 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.
Note that this can not change the type because it does not release and
reserve the hwbp slot based on type.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 arch/Kconfig                         | 10 ++++++++
 arch/x86/Kconfig                     |  1 +
 arch/x86/include/asm/hw_breakpoint.h |  2 ++
 arch/x86/kernel/hw_breakpoint.c      | 11 +++++++++
 include/linux/hw_breakpoint.h        |  6 +++++
 kernel/events/hw_breakpoint.c        | 36 ++++++++++++++++++++++++++++
 6 files changed, 66 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index 8e3fd723bd74..bec164bea0ec 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -409,6 +409,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
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ad7a6d139006..ca8331522c07 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -249,6 +249,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 bb7c70ad22fe..b3db25eb613f 100644
--- a/arch/x86/include/asm/hw_breakpoint.h
+++ b/arch/x86/include/asm/hw_breakpoint.h
@@ -64,6 +64,8 @@ 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);
 
+bool hw_breakpoint_arch_same_type(struct arch_hw_breakpoint *hw,
+				  struct perf_event_attr *attr);
 extern void
 arch_fill_perf_breakpoint(struct perf_event *bp);
 
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 89135229ed21..7dfc88ff6cd9 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -278,6 +278,17 @@ int arch_bp_generic_fields(int x86_len, int x86_type,
 	return 0;
 }
 
+bool hw_breakpoint_arch_same_type(struct arch_hw_breakpoint *hw,
+				  struct perf_event_attr *attr)
+{
+	int glen, gtype;
+
+	if (arch_bp_generic_fields(hw->len, hw->type, &glen, &gtype) < 0)
+		return false;
+
+	return gtype == attr->bp_type;
+}
+
 /*
  * Check for virtual address in kernel space.
  */
diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index db199d653dd1..eb14f9ab5bdb 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 update_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
+update_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 8ec2cb688903..473a5b76941d 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -887,6 +887,42 @@ 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 HWBP, just reuse 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 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 (!hw_breakpoint_arch_same_type(counter_arch_bp(bp), attr))
+		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.43.0


Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [RFC PATCH 02/13] x86/HWBP: Add arch_reinstall_hw_breakpoint() for atomic updates

[Jinchao Wang]

On 9/1/25 15:06, Masami Hiramatsu (Google) wrote:
> Hi Jinchao,
> 
Hi Masami,

> On Mon, 18 Aug 2025 20:26:07 +0800
> Jinchao Wang <wangjinchao600@gmail.com> wrote:
> 
>> Add arch_reinstall_hw_breakpoint() to enable atomic context modification
>> of hardware breakpoint parameters without deallocating and reallocating
>> the breakpoint slot.
>>
>> The existing arch_install_hw_breakpoint() allocates a new debug register
>> slot, while arch_uninstall_hw_breakpoint() deallocates it. However, some
>> use cases require modifying breakpoint parameters (address, length, type)
>> atomically without losing the allocated slot, particularly when operating
>> in atomic contexts where allocation might fail or be unavailable.
>>
>> This is particularly useful for debugging tools like kstackwatch that
>> need to dynamically update breakpoint targets in atomic contexts while
>> maintaining consistent hardware state.
>>
> 
> I'm also trying to find this interface for my wprobe. So the idea is good.
> But this looks hacky and only for x86. I think the interface should be
> more generic and do not use this arch internal function directly.
>

I agree with your point about the architectural dependency. I have been
considering this problem not only for the hardware breakpoint 
reinstallation,
but also for other related parts of the series, such as canary finding and
stack address resolving. These parts also rely on arch-specific code.


> It seems that the slot is allocated by "type", thus, if this reinstall
> hwbp without deallocate/allocate slot, it must NOT change the type.
> See __modify_bp_slot. Also, provide CONFIG_HAVE_... option for checking
> whether the architecture support that interface.
> 
Regarding the slot allocation, I would like to clarify my point. I 
believe the
event->attr.type should not be changed when reinstalling a hardware
breakpoint, as this defines the fundamental nature of the event. The type
must always be PERF_TYPE_BREAKPOINT.

The event->attr.bp_type, however, can be changed. For example, from a
HW_BREAKPOINT_W to a HW_BREAKPOINT_RW without needing to deallocate and
reallocate the slot. This is useful for future applications, even though the
current use case for KStackWatch only requires HW_BREAKPOINT_W.

By the way, I have sent an updated series.
https://lore.kernel.org/all/20250828073311.1116593-1-wangjinchao600@gmail.com/

Thank you again for your valuable review.
-- 
Best regards,
Jinchao

Re: [RFC PATCH 02/13] x86/HWBP: Add arch_reinstall_hw_breakpoint() for atomic updates

[Masami Hiramatsu]

On Mon, 1 Sep 2025 18:23:44 +0800
Jinchao Wang <wangjinchao600@gmail.com> wrote:

> On 9/1/25 15:06, Masami Hiramatsu (Google) wrote:
> > Hi Jinchao,
> > 
> Hi Masami,
> 
> > On Mon, 18 Aug 2025 20:26:07 +0800
> > Jinchao Wang <wangjinchao600@gmail.com> wrote:
> > 
> >> Add arch_reinstall_hw_breakpoint() to enable atomic context modification
> >> of hardware breakpoint parameters without deallocating and reallocating
> >> the breakpoint slot.
> >>
> >> The existing arch_install_hw_breakpoint() allocates a new debug register
> >> slot, while arch_uninstall_hw_breakpoint() deallocates it. However, some
> >> use cases require modifying breakpoint parameters (address, length, type)
> >> atomically without losing the allocated slot, particularly when operating
> >> in atomic contexts where allocation might fail or be unavailable.
> >>
> >> This is particularly useful for debugging tools like kstackwatch that
> >> need to dynamically update breakpoint targets in atomic contexts while
> >> maintaining consistent hardware state.
> >>
> > 
> > I'm also trying to find this interface for my wprobe. So the idea is good.
> > But this looks hacky and only for x86. I think the interface should be
> > more generic and do not use this arch internal function directly.
> >
> 
> I agree with your point about the architectural dependency. I have been
> considering this problem not only for the hardware breakpoint 
> reinstallation,
> but also for other related parts of the series, such as canary finding and
> stack address resolving. These parts also rely on arch-specific code.

Yes, even though, the hw-breakpoint is an independent feature.
Directly using arch_*() functions (which are expected to be used
internally) introduces a hidden dependency between these two
components and looses maintainability.

> > It seems that the slot is allocated by "type", thus, if this reinstall
> > hwbp without deallocate/allocate slot, it must NOT change the type.
> > See __modify_bp_slot. Also, provide CONFIG_HAVE_... option for checking
> > whether the architecture support that interface.
> > 
> Regarding the slot allocation, I would like to clarify my point. I 
> believe the
> event->attr.type should not be changed when reinstalling a hardware
> breakpoint, as this defines the fundamental nature of the event. The type
> must always be PERF_TYPE_BREAKPOINT.
> 
> The event->attr.bp_type, however, can be changed. For example, from a
> HW_BREAKPOINT_W to a HW_BREAKPOINT_RW without needing to deallocate and
> reallocate the slot. This is useful for future applications, even though the
> current use case for KStackWatch only requires HW_BREAKPOINT_W.

I understand your point, so it also needs another wrapper which checks
the type is compatible on the architecture.

> 
> By the way, I have sent an updated series.
> https://lore.kernel.org/all/20250828073311.1116593-1-wangjinchao600@gmail.com/

Yeah, OK, let me review the series. Thanks for update!

> 
> Thank you again for your valuable review.
> -- 
> Best regards,
> Jinchao


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [RFC PATCH 02/13] x86/HWBP: Add arch_reinstall_hw_breakpoint() for atomic updates

[Jinchao Wang]

On 9/2/25 22:11, Masami Hiramatsu (Google) wrote:
> On Mon, 1 Sep 2025 18:23:44 +0800
> Jinchao Wang <wangjinchao600@gmail.com> wrote:
> 
>> On 9/1/25 15:06, Masami Hiramatsu (Google) wrote:
>>> Hi Jinchao,
>>>
>> Hi Masami,
>>
>>> On Mon, 18 Aug 2025 20:26:07 +0800
>>> Jinchao Wang <wangjinchao600@gmail.com> wrote:
>>>
>>>> Add arch_reinstall_hw_breakpoint() to enable atomic context modification
>>>> of hardware breakpoint parameters without deallocating and reallocating
>>>> the breakpoint slot.
>>>>
>>>> The existing arch_install_hw_breakpoint() allocates a new debug register
>>>> slot, while arch_uninstall_hw_breakpoint() deallocates it. However, some
>>>> use cases require modifying breakpoint parameters (address, length, type)
>>>> atomically without losing the allocated slot, particularly when operating
>>>> in atomic contexts where allocation might fail or be unavailable.
>>>>
>>>> This is particularly useful for debugging tools like kstackwatch that
>>>> need to dynamically update breakpoint targets in atomic contexts while
>>>> maintaining consistent hardware state.
>>>>
>>>
>>> I'm also trying to find this interface for my wprobe. So the idea is good.
>>> But this looks hacky and only for x86. I think the interface should be
>>> more generic and do not use this arch internal function directly.
>>>
>>
>> I agree with your point about the architectural dependency. I have been
>> considering this problem not only for the hardware breakpoint
>> reinstallation,
>> but also for other related parts of the series, such as canary finding and
>> stack address resolving. These parts also rely on arch-specific code.
> 
> Yes, even though, the hw-breakpoint is an independent feature.
> Directly using arch_*() functions (which are expected to be used
> internally) introduces a hidden dependency between these two
> components and looses maintainability.

Yes, I am trying to improve this in the v3 series.

> 
>>> It seems that the slot is allocated by "type", thus, if this reinstall
>>> hwbp without deallocate/allocate slot, it must NOT change the type.
>>> See __modify_bp_slot. Also, provide CONFIG_HAVE_... option for checking
>>> whether the architecture support that interface.
>>>
>> Regarding the slot allocation, I would like to clarify my point. I
>> believe the
>> event->attr.type should not be changed when reinstalling a hardware
>> breakpoint, as this defines the fundamental nature of the event. The type
>> must always be PERF_TYPE_BREAKPOINT.
>>
>> The event->attr.bp_type, however, can be changed. For example, from a
>> HW_BREAKPOINT_W to a HW_BREAKPOINT_RW without needing to deallocate and
>> reallocate the slot. This is useful for future applications, even though the
>> current use case for KStackWatch only requires HW_BREAKPOINT_W.
> 
> I understand your point, so it also needs another wrapper which checks
> the type is compatible on the architecture.
> 

I think the wrapper should handle the type by type_slot, something like[1]:

diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 1db2c5e24d0e..6fed9521baf2 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -752,6 +752,7 @@ modify_user_hw_breakpoint_check(struct perf_event 
*bp, struct perf_event_attr *a
  {
  	struct arch_hw_breakpoint hw = { };
  	int err;
+	enum bp_type_idx old_type_idx, new_type_idx;

  	err = hw_breakpoint_parse(bp, attr, &hw);
  	if (err)
@@ -766,7 +767,9 @@ modify_user_hw_breakpoint_check(struct perf_event 
*bp, struct perf_event_attr *a
  			return -EINVAL;
  	}

-	if (bp->attr.bp_type != attr->bp_type) {
+	old_type_idx = find_slot_idx(bp->attr.bp_type);
+	new_type_idx = find_slot_idx(attr->bp_type);
+	if (old_type_idx != new_type_idx) {
  		err = modify_bp_slot(bp, bp->attr.bp_type, attr->bp_type);
  		if (err)
  			return err;

For kernel breakpoints, we might also consider introducing a
modify_kernel_hw_breakpoint() helper, similar to
modify_user_hw_breakpoint(), to encapsulate the kernel-specific case.

[1]https://lore.kernel.org/all/20250903075144.3722848-3-wangjinchao600@gmail.com/

>>
>> By the way, I have sent an updated series.
>> https://lore.kernel.org/all/20250828073311.1116593-1-wangjinchao600@gmail.com/
> 
> Yeah, OK, let me review the series. Thanks for update!
> 
>>
>> Thank you again for your valuable review.
>> -- 
>> Best regards,
>> Jinchao
> 
> 


-- 
Best regards,
Jinchao

Re: [RFC PATCH 02/13] x86/HWBP: Add arch_reinstall_hw_breakpoint() for atomic updates

[Jinchao Wang]

On 9/3/25 15:58, Jinchao Wang wrote:
> On 9/2/25 22:11, Masami Hiramatsu (Google) wrote:
>> On Mon, 1 Sep 2025 18:23:44 +0800
>> Jinchao Wang <wangjinchao600@gmail.com> wrote:
>>
>>> On 9/1/25 15:06, Masami Hiramatsu (Google) wrote:
>>>> Hi Jinchao,
>>>>
>>> Hi Masami,
>>>
>>>> On Mon, 18 Aug 2025 20:26:07 +0800
>>>> Jinchao Wang <wangjinchao600@gmail.com> wrote:
>>>>
>>>>> Add arch_reinstall_hw_breakpoint() to enable atomic context 
>>>>> modification
>>>>> of hardware breakpoint parameters without deallocating and 
>>>>> reallocating
>>>>> the breakpoint slot.
>>>>>
>>>>> The existing arch_install_hw_breakpoint() allocates a new debug 
>>>>> register
>>>>> slot, while arch_uninstall_hw_breakpoint() deallocates it. However, 
>>>>> some
>>>>> use cases require modifying breakpoint parameters (address, length, 
>>>>> type)
>>>>> atomically without losing the allocated slot, particularly when 
>>>>> operating
>>>>> in atomic contexts where allocation might fail or be unavailable.
>>>>>
>>>>> This is particularly useful for debugging tools like kstackwatch that
>>>>> need to dynamically update breakpoint targets in atomic contexts while
>>>>> maintaining consistent hardware state.
>>>>>
>>>>
>>>> I'm also trying to find this interface for my wprobe. So the idea is 
>>>> good.
>>>> But this looks hacky and only for x86. I think the interface should be
>>>> more generic and do not use this arch internal function directly.
>>>>
>>>
>>> I agree with your point about the architectural dependency. I have been
>>> considering this problem not only for the hardware breakpoint
>>> reinstallation,
>>> but also for other related parts of the series, such as canary 
>>> finding and
>>> stack address resolving. These parts also rely on arch-specific code.
>>
>> Yes, even though, the hw-breakpoint is an independent feature.
>> Directly using arch_*() functions (which are expected to be used
>> internally) introduces a hidden dependency between these two
>> components and looses maintainability.
> 
> Yes, I am trying to improve this in the v3 series.
> 
>>
>>>> It seems that the slot is allocated by "type", thus, if this reinstall
>>>> hwbp without deallocate/allocate slot, it must NOT change the type.
>>>> See __modify_bp_slot. Also, provide CONFIG_HAVE_... option for checking
>>>> whether the architecture support that interface.
>>>>
>>> Regarding the slot allocation, I would like to clarify my point. I
>>> believe the
>>> event->attr.type should not be changed when reinstalling a hardware
>>> breakpoint, as this defines the fundamental nature of the event. The 
>>> type
>>> must always be PERF_TYPE_BREAKPOINT.
>>>
>>> The event->attr.bp_type, however, can be changed. For example, from a
>>> HW_BREAKPOINT_W to a HW_BREAKPOINT_RW without needing to deallocate and
>>> reallocate the slot. This is useful for future applications, even 
>>> though the
>>> current use case for KStackWatch only requires HW_BREAKPOINT_W.
>>
>> I understand your point, so it also needs another wrapper which checks
>> the type is compatible on the architecture.
>>
> 

> I think the wrapper should handle the type by type_slot, something like[1]:
> ...
> 
> For kernel breakpoints, we might also consider introducing a
> modify_kernel_hw_breakpoint() helper, similar to
> modify_user_hw_breakpoint(), to encapsulate the kernel-specific case.
> 

Hi Masami,
I have a new idea, indroducing hw_breakpoint_modify_local():
   - call hw_breakpoint_arch_parse to validate bp_{type,addr,len},
     eliminating the need for a separate type-only check
   - use a__weak arch_reinstall_hw_breakpoint for other archs

If other archs require additional checks, they could add that logic to 
hw_breakpoint_modify_local or their hw_breakpoint_arch_parse().

Seems clear and simple enough.

Regarding the arch dependency problems you mentioned, I have solved them 
and sent a v2 series:
https://lore.kernel.org/all/20250904002126.1514566-7-wangjinchao600@gmail.com/

>>
>>>
>>> Thank you again for your valuable review.
>>> -- 
>>> Best regards,
>>> Jinchao
>>
>>
> 
> 


-- 
Best regards,
Jinchao

Re: [RFC PATCH 02/13] x86/HWBP: Add arch_reinstall_hw_breakpoint() for atomic updates

[Masami Hiramatsu]

On Wed, 3 Sep 2025 15:58:44 +0800
Jinchao Wang <wangjinchao600@gmail.com> wrote:

> On 9/2/25 22:11, Masami Hiramatsu (Google) wrote:
> > On Mon, 1 Sep 2025 18:23:44 +0800
> > Jinchao Wang <wangjinchao600@gmail.com> wrote:
> > 
> >> On 9/1/25 15:06, Masami Hiramatsu (Google) wrote:
> >>> Hi Jinchao,
> >>>
> >> Hi Masami,
> >>
> >>> On Mon, 18 Aug 2025 20:26:07 +0800
> >>> Jinchao Wang <wangjinchao600@gmail.com> wrote:
> >>>
> >>>> Add arch_reinstall_hw_breakpoint() to enable atomic context modification
> >>>> of hardware breakpoint parameters without deallocating and reallocating
> >>>> the breakpoint slot.
> >>>>
> >>>> The existing arch_install_hw_breakpoint() allocates a new debug register
> >>>> slot, while arch_uninstall_hw_breakpoint() deallocates it. However, some
> >>>> use cases require modifying breakpoint parameters (address, length, type)
> >>>> atomically without losing the allocated slot, particularly when operating
> >>>> in atomic contexts where allocation might fail or be unavailable.
> >>>>
> >>>> This is particularly useful for debugging tools like kstackwatch that
> >>>> need to dynamically update breakpoint targets in atomic contexts while
> >>>> maintaining consistent hardware state.
> >>>>
> >>>
> >>> I'm also trying to find this interface for my wprobe. So the idea is good.
> >>> But this looks hacky and only for x86. I think the interface should be
> >>> more generic and do not use this arch internal function directly.
> >>>
> >>
> >> I agree with your point about the architectural dependency. I have been
> >> considering this problem not only for the hardware breakpoint
> >> reinstallation,
> >> but also for other related parts of the series, such as canary finding and
> >> stack address resolving. These parts also rely on arch-specific code.
> > 
> > Yes, even though, the hw-breakpoint is an independent feature.
> > Directly using arch_*() functions (which are expected to be used
> > internally) introduces a hidden dependency between these two
> > components and looses maintainability.
> 
> Yes, I am trying to improve this in the v3 series.
> 
> > 
> >>> It seems that the slot is allocated by "type", thus, if this reinstall
> >>> hwbp without deallocate/allocate slot, it must NOT change the type.
> >>> See __modify_bp_slot. Also, provide CONFIG_HAVE_... option for checking
> >>> whether the architecture support that interface.
> >>>
> >> Regarding the slot allocation, I would like to clarify my point. I
> >> believe the
> >> event->attr.type should not be changed when reinstalling a hardware
> >> breakpoint, as this defines the fundamental nature of the event. The type
> >> must always be PERF_TYPE_BREAKPOINT.
> >>
> >> The event->attr.bp_type, however, can be changed. For example, from a
> >> HW_BREAKPOINT_W to a HW_BREAKPOINT_RW without needing to deallocate and
> >> reallocate the slot. This is useful for future applications, even though the
> >> current use case for KStackWatch only requires HW_BREAKPOINT_W.
> > 
> > I understand your point, so it also needs another wrapper which checks
> > the type is compatible on the architecture.
> > 
> 
> I think the wrapper should handle the type by type_slot, something like[1]:
> 

Ah, that's a good idea!

> diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
> index 1db2c5e24d0e..6fed9521baf2 100644
> --- a/kernel/events/hw_breakpoint.c
> +++ b/kernel/events/hw_breakpoint.c
> @@ -752,6 +752,7 @@ modify_user_hw_breakpoint_check(struct perf_event 
> *bp, struct perf_event_attr *a
>   {
>   	struct arch_hw_breakpoint hw = { };
>   	int err;
> +	enum bp_type_idx old_type_idx, new_type_idx;
> 
>   	err = hw_breakpoint_parse(bp, attr, &hw);
>   	if (err)
> @@ -766,7 +767,9 @@ modify_user_hw_breakpoint_check(struct perf_event 
> *bp, struct perf_event_attr *a
>   			return -EINVAL;
>   	}
> 
> -	if (bp->attr.bp_type != attr->bp_type) {
> +	old_type_idx = find_slot_idx(bp->attr.bp_type);
> +	new_type_idx = find_slot_idx(attr->bp_type);
> +	if (old_type_idx != new_type_idx) {
>   		err = modify_bp_slot(bp, bp->attr.bp_type, attr->bp_type);
>   		if (err)
>   			return err;
> 
> For kernel breakpoints, we might also consider introducing a
> modify_kernel_hw_breakpoint() helper, similar to
> modify_user_hw_breakpoint(), to encapsulate the kernel-specific case.
> 
> [1]https://lore.kernel.org/all/20250903075144.3722848-3-wangjinchao600@gmail.com/

Hmm, it seems that there is *user_hw_breakpoint() and *wide_hw_breakpoint()
(maybe it means system-wide) so it is better to call modify_wide_hw_breakpoint().
(anyway it is only for kernel address space...)

Thank you!

> 
> >>
> >> By the way, I have sent an updated series.
> >> https://lore.kernel.org/all/20250828073311.1116593-1-wangjinchao600@gmail.com/
> > 
> > Yeah, OK, let me review the series. Thanks for update!
> > 
> >>
> >> Thank you again for your valuable review.
> >> -- 
> >> Best regards,
> >> Jinchao
> > 
> > 
> 
> 
> -- 
> Best regards,
> Jinchao


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [RFC PATCH 02/13] x86/HWBP: Add arch_reinstall_hw_breakpoint() for atomic updates

[Jinchao Wang]

On 9/4/25 09:02, Masami Hiramatsu (Google) wrote:
> On Wed, 3 Sep 2025 15:58:44 +0800
> Jinchao Wang <wangjinchao600@gmail.com> wrote:
> 
>> On 9/2/25 22:11, Masami Hiramatsu (Google) wrote:
>>> On Mon, 1 Sep 2025 18:23:44 +0800
>>> Jinchao Wang <wangjinchao600@gmail.com> wrote:
>>>
>>>> On 9/1/25 15:06, Masami Hiramatsu (Google) wrote:
>>>>> Hi Jinchao,
>>>>>
>>>> Hi Masami,
>>>>
>>>>> On Mon, 18 Aug 2025 20:26:07 +0800
>>>>> Jinchao Wang <wangjinchao600@gmail.com> wrote:
>>>>>
>>>>>> Add arch_reinstall_hw_breakpoint() to enable atomic context modification
>>>>>> of hardware breakpoint parameters without deallocating and reallocating
>>>>>> the breakpoint slot.
>>>>>>
>>>>>> The existing arch_install_hw_breakpoint() allocates a new debug register
>>>>>> slot, while arch_uninstall_hw_breakpoint() deallocates it. However, some
>>>>>> use cases require modifying breakpoint parameters (address, length, type)
>>>>>> atomically without losing the allocated slot, particularly when operating
>>>>>> in atomic contexts where allocation might fail or be unavailable.
>>>>>>
>>>>>> This is particularly useful for debugging tools like kstackwatch that
>>>>>> need to dynamically update breakpoint targets in atomic contexts while
>>>>>> maintaining consistent hardware state.
>>>>>>
>>>>>
>>>>> I'm also trying to find this interface for my wprobe. So the idea is good.
>>>>> But this looks hacky and only for x86. I think the interface should be
>>>>> more generic and do not use this arch internal function directly.
>>>>>
>>>>
>>>> I agree with your point about the architectural dependency. I have been
>>>> considering this problem not only for the hardware breakpoint
>>>> reinstallation,
>>>> but also for other related parts of the series, such as canary finding and
>>>> stack address resolving. These parts also rely on arch-specific code.
>>>
>>> Yes, even though, the hw-breakpoint is an independent feature.
>>> Directly using arch_*() functions (which are expected to be used
>>> internally) introduces a hidden dependency between these two
>>> components and looses maintainability.
>>
>> Yes, I am trying to improve this in the v3 series.
>>
>>>
>>>>> It seems that the slot is allocated by "type", thus, if this reinstall
>>>>> hwbp without deallocate/allocate slot, it must NOT change the type.
>>>>> See __modify_bp_slot. Also, provide CONFIG_HAVE_... option for checking
>>>>> whether the architecture support that interface.
>>>>>
>>>> Regarding the slot allocation, I would like to clarify my point. I
>>>> believe the
>>>> event->attr.type should not be changed when reinstalling a hardware
>>>> breakpoint, as this defines the fundamental nature of the event. The type
>>>> must always be PERF_TYPE_BREAKPOINT.
>>>>
>>>> The event->attr.bp_type, however, can be changed. For example, from a
>>>> HW_BREAKPOINT_W to a HW_BREAKPOINT_RW without needing to deallocate and
>>>> reallocate the slot. This is useful for future applications, even though the
>>>> current use case for KStackWatch only requires HW_BREAKPOINT_W.
>>>
>>> I understand your point, so it also needs another wrapper which checks
>>> the type is compatible on the architecture.
>>>
>>
>> I think the wrapper should handle the type by type_slot, something like[1]:
>>
> 
> Ah, that's a good idea!
> 
>> diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
>> index 1db2c5e24d0e..6fed9521baf2 100644
>> --- a/kernel/events/hw_breakpoint.c
>> +++ b/kernel/events/hw_breakpoint.c
>> @@ -752,6 +752,7 @@ modify_user_hw_breakpoint_check(struct perf_event
>> *bp, struct perf_event_attr *a
>>    {
>>    	struct arch_hw_breakpoint hw = { };
>>    	int err;
>> +	enum bp_type_idx old_type_idx, new_type_idx;
>>
>>    	err = hw_breakpoint_parse(bp, attr, &hw);
>>    	if (err)
>> @@ -766,7 +767,9 @@ modify_user_hw_breakpoint_check(struct perf_event
>> *bp, struct perf_event_attr *a
>>    			return -EINVAL;
>>    	}
>>
>> -	if (bp->attr.bp_type != attr->bp_type) {
>> +	old_type_idx = find_slot_idx(bp->attr.bp_type);
>> +	new_type_idx = find_slot_idx(attr->bp_type);
>> +	if (old_type_idx != new_type_idx) {
>>    		err = modify_bp_slot(bp, bp->attr.bp_type, attr->bp_type);
>>    		if (err)
>>    			return err;
>>
>> For kernel breakpoints, we might also consider introducing a
>> modify_kernel_hw_breakpoint() helper, similar to
>> modify_user_hw_breakpoint(), to encapsulate the kernel-specific case.
>>
>> [1]https://lore.kernel.org/all/20250903075144.3722848-3-wangjinchao600@gmail.com/
> 
> Hmm, it seems that there is *user_hw_breakpoint() and *wide_hw_breakpoint()
> (maybe it means system-wide) so it is better to call modify_wide_hw_breakpoint().
> (anyway it is only for kernel address space...)
> 
> Thank you!

The counterpart to user might be kernel, and wide likely means "for all 
CPUs".

We might have missed each other's emails due to the time zone 
difference. Thank you so much for your feedback.>
>>
>>>>
>>>> By the way, I have sent an updated series.
>>>> https://lore.kernel.org/all/20250828073311.1116593-1-wangjinchao600@gmail.com/
>>>
>>> Yeah, OK, let me review the series. Thanks for update!
>>>
>>>>
>>>> Thank you again for your valuable review.
>>>> -- 
>>>> Best regards,
>>>> Jinchao
>>>
>>>
>>
>>
>> -- 
>> Best regards,
>> Jinchao
> 
> 
-- 
Best regards,
Jinchao