Re: [PATCH v15 03/12] x86/hw_breakpoints: Add arch_modify_local_hw_breakpoint_addr() API

On Mon, 07 Sep 2026 04:02:03 +0000
sashiko-bot@kernel.org wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Modifying a debug address register while the breakpoint is enabled in DR7 violates the Intel SDM. This can occur due to a race condition with an NMI because the `cpu_dr7` shadow state is not temporarily updated.
--

commit ca38cc4e9ed0533dfe16175c78dba60903420a8b
Author: Jinchao Wang [off-list ref]

x86/hw_breakpoints: Add arch_modify_local_hw_breakpoint_addr() API

Wprobe needs to move an active per-CPU watchpoint without releasing and
reserving its hardware slot.

Add arch_modify_local_hw_breakpoint_addr() as the architecture backend for
modify_local_hw_breakpoint_addr(). It validates the new address, finds the
installed local slot, and updates the hardware debug register and per-CPU
shadow register.

Link: https://lore.kernel.org/all/59637b96946653393a7ad3c7de094094796b39c2.1785067572.git.wangjinchao600@gmail.com/ (local)

quoted

diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 312550be3ad2b..99f0926298a85 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c

[ ... ]

quoted

@@ -129,6 +129,62 @@ int arch_install_hw_breakpoint(struct perf_event *bp)

[ ... ]

quoted

+	do {
+		seq = this_cpu_inc_return(cpu_dr7_seq);
+
+		/* Disable breakpoint in DR7 before updating address register */
+		dr7 = this_cpu_read(cpu_dr7);
+		set_debugreg((dr7 & ~__encode_dr7(i, info->len, info->type)) | DR7_FIXED_1, 7);

[Severity: High]
Does arch_modify_local_hw_breakpoint_addr() need to clear the breakpoint in
the shadow cpu_dr7 state as well?

Ah, OK, let's update cpu_dr7 too.

Since the breakpoint isn't cleared in the cpu_dr7 shadow state, could an NMI
that interrupts execution here inadvertently re-enable the breakpoint
prematurely?

If an NMI handler (such as kgdb or perf) modifies breakpoints, it increments
cpu_dr7_seq and invokes local_db_save() and local_db_restore(). When the NMI
completes, local_db_restore() detects the sequence mismatch and restores
hardware DR7 using the cpu_dr7 shadow cache. Because cpu_dr7 still has this
breakpoint enabled, the target breakpoint gets re-enabled in hardware DR7
before the execution below resumes.

quoted

+		barrier();
+
+		this_cpu_write(cpu_debugreg[i], addr);
+		barrier();
+		set_debugreg(addr, i);

[Severity: High]
If an NMI prematurely restored DR7 as described above, won't this modify the
debug address register while the breakpoint is enabled in DR7?

This also needs to update cpu_dr7.

Thank you,

Modifying a debug address register while its corresponding DR7 enable bit is
set violates the Intel SDM and causes spurious #DB exceptions.

While the sequence loop retry mechanism will catch the mismatch, it only does
so after this instruction has already executed, which is too late to prevent
the spurious exception.

quoted

+		barrier();
+
+		/* Re-enable breakpoint in DR7 */
+		set_debugreg(this_cpu_read(cpu_dr7) | DR7_FIXED_1, 7);
+		barrier();
+	} while (seq != this_cpu_read(cpu_dr7_seq));
+
+	return 0;
+}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178875277830.93794.14247844688761142429.stgit@devnote2?part=3

-- 
Masami Hiramatsu (Google) [off-list ref]