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]