Re: [RFC PATCH 13/13] Documentation/dev-tools: document KWatch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Rate-limiting in kwatch_hwbp_arm() silently fails to arm the watchpoint locally and causes state mismatches.
- [High] Using calling convention registers (arg1..arg6) to resolve watch expressions at mid-function offsets leads to reading garbage data.
--

--- Patch [13]: [RFC PATCH 13/13] Documentation/dev-tools: document KWatch ---
commit da418fc283a833027fb7ccca30c188d2536f9a4a
Author: Jinchao Wang <wangjinchao600@gmail.com>

Documentation/dev-tools: document KWatch

Describe what KWatch is for, how it compares with KASAN and KFENCE,
the debugfs configuration interface, the watch expression syntax,
how to read hits from the trace buffer (including after a crash),
and the current limitations.

> diff --git a/mm/kwatch/deref.c b/mm/kwatch/deref.c
> new file mode 100644
> index 0000000000000..a93c76139e7cd
> --- /dev/null
> +++ b/mm/kwatch/deref.c
> @@ -0,0 +1,174 @@
> +// SPDX-License-Identifier: GPL-2.0

[ ... ]

> +int kwatch_deref_resolve(const struct kwatch_config *cfg, struct pt_regs *regs,
> +			 unsigned long *out_addr, u16 *out_len)
> +{
> +	unsigned long addr = 0;
> +	int i;
> +
> +	/* 1. Resolve the Base Anchor */
> +	if (cfg->base == KWATCH_BASE_STACK) {
> +		addr = kernel_stack_pointer(regs);
> +		if (unlikely(!addr))
> +			return -EINVAL;
> +	} else if (cfg->base >= KWATCH_BASE_ARG1 &&
> +		   cfg->base <= KWATCH_BASE_ARG6) {
> +		int arg_idx = cfg->base - KWATCH_BASE_ARG1;
> +
> +		addr = regs_get_kernel_argument(regs, arg_idx);

[Severity: High]
If a watchpoint is configured with a mid-function offset (func_offset > 0),
are the calling convention registers still guaranteed to hold the arguments?

Since the compiler is free to overwrite argument registers immediately after
function entry, could this read garbage data and cause KWatch to monitor an
incorrect memory address?

[ ... ]

> diff --git a/mm/kwatch/hwbp.c b/mm/kwatch/hwbp.c
> new file mode 100644
> index 0000000000000..19498ba038260
> --- /dev/null
> +++ b/mm/kwatch/hwbp.c
> @@ -0,0 +1,358 @@
> +// SPDX-License-Identifier: GPL-2.0

[ ... ]

> +void kwatch_hwbp_arm(struct kwatch_watchpoint *wp, unsigned long addr, u16 len)
> +{
> +	static DEFINE_PER_CPU(u64, last_ipi_time);
> +	int cur_cpu;
> +	call_single_data_t *csd;
> +	int cpu;
> +	bool is_disarm = (addr == (unsigned long)&kwatch_dummy_holder);
> +
> +	wp->attr.bp_addr = addr;
> +	wp->attr.bp_len = len;
> +
> +	if (!is_disarm)
> +		wp->arm_tsk = current;
> +
> +	/* ensure attr update visible to other cpu before sending IPI */
> +	smp_wmb();
> +
> +	atomic_set(&wp->pending_ipis, 1);
> +	cur_cpu = get_cpu();
> +
> +	if (!is_disarm) {
> +		u64 now = local_clock();
> +		u64 last = this_cpu_read(last_ipi_time);
> +
> +		if (now - last < 1000000ULL) {
> +			put_cpu();
> +			return;
> +		}

[Severity: High]
Does this early return skip both the global IPI broadcast and the
kwatch_hwbp_arm_local() call?

If a user configures KWatch on a frequently invoked function and this rate
limit is hit, it looks like it will silently fail to arm the watchpoint locally.

Could this also cause state mismatches when disarm operations later broadcast
IPIs for a watchpoint that was never actually armed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714182243.10687-1-wangjinchao600@gmail.com?part=13