Re: [PATCH v8 6/9] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Modifying an active breakpoint via `arch_reinstall_hw_breakpoint()` updates the debug address register before disabling it in the control register, violating architectural requirements and creating a race condition that can trigger spurious `#DB` exceptions.
- [Low] The newly added function `arch_reinstall_hw_breakpoint()` has no callers in the kernel tree.
--

--- Patch [6]: [PATCH v8 6/9] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint ---
commit 26fd09a1abe10d2182564575b6e349d84a888d45
Author: Jinchao Wang <wangjinchao600@gmail.com>

x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint

The new arch_reinstall_hw_breakpoint() function can be used in an
atomic context to efficiently re-establish an existing breakpoint
without going through the more expensive free and re-allocation path.

> diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
> index c323c2aab2af9..5821bb4650cec 100644
> --- a/arch/x86/kernel/hw_breakpoint.c
> +++ b/arch/x86/kernel/hw_breakpoint.c

[ ... ]

> @@ -189,6 +193,11 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
>  	return arch_manage_bp(bp, BP_SLOT_ACTION_INSTALL);
>  }
>  
> +int arch_reinstall_hw_breakpoint(struct perf_event *bp)
> +{
> +	return arch_manage_bp(bp, BP_SLOT_ACTION_REINSTALL);
> +}

[Severity: High]
Does this sequence risk triggering spurious hardware debug exceptions?

When arch_reinstall_hw_breakpoint() modifies an active breakpoint, it calls
arch_manage_bp() which then invokes setup_hwbp(). In setup_hwbp(), the debug
address register is updated before the control register (dr7) is safely
cleared:

arch/x86/kernel/hw_breakpoint.c:setup_hwbp() {
    ...
    set_debugreg(info->address, slot);
    ...
    dr7 &= ~(__encode_dr7(slot, 0xc, 0x3) |
             (DR_LOCAL_ENABLE << (slot * DR_ENABLE_SIZE)));
    ...
}

Because BP_SLOT_ACTION_REINSTALL operates on a breakpoint that might already
be active, dr7 is currently enabled in hardware during this window. Can
writing to the address register while the slot is still enabled in dr7
trigger a spurious debug exception if a concurrent execution accesses the new
address?

[Severity: Low]
This isn't a bug, but are there any planned callers for this function?

A search of the tree indicates that arch_reinstall_hw_breakpoint() is not
currently invoked by any subsystem or exported to modules.

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