Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Lockless updates to `bp->attr.bp_addr` and `counter_arch_bp(bp)->address` lack `WRITE_ONCE()`, constituting a formal data race under the Linux Kernel Memory Model. --
commit 0c79300d09e43eac52a027f6cb480808e7f3902f Author: Masami Hiramatsu (Google) <[email protected]> HWBP: Add modify_local_hw_breakpoint_addr() API This commit adds the modify_local_hw_breakpoint_addr() API to update the watched address of an installed hardware breakpoint on the local CPU without releasing and reserving its hardware slot. Link: https://lore.kernel.org/all/f9c49dfa49bdc57ba8c0574bc9981c1e581acf92.1785067572.git.wangjinchao...@gmail.com/ > diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c > index 789add0c185a0..124074c3d3bc2 100644 > --- a/kernel/events/hw_breakpoint.c > +++ b/kernel/events/hw_breakpoint.c [ ... ] > +int modify_local_hw_breakpoint_addr(struct perf_event *bp, > + unsigned long addr) > +{ > + unsigned long old_addr; > + int ret; > + > + lockdep_assert_irqs_disabled(); > + > + old_addr = bp->attr.bp_addr; > + counter_arch_bp(bp)->address = addr; > + bp->attr.bp_addr = addr; [Severity: Low] The function intentionally updates bp->attr.bp_addr locklessly so NMI handlers can observe the new address during the transition. However, without WRITE_ONCE() annotations, does this constitute a formal data race under the Linux Kernel Memory Model? While a torn store on 32-bit architectures might only overwrite the upper 32 bits with zero (leaving the address functionally intact), could this still trigger KCSAN warnings when an NMI handler concurrently reads bp->attr.bp_addr via perf_bp_event()? > + > + ret = arch_modify_local_hw_breakpoint_addr(bp, addr); > + if (ret) { > + counter_arch_bp(bp)->address = old_addr; > + bp->attr.bp_addr = old_addr; [Severity: Low] Similarly, should the rollback path also use WRITE_ONCE() to ensure concurrent accesses by NMI handlers avoid data race warnings from KCSAN? > + return ret; > + } > + > + return 0; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/178810001186.64882.2161016469449127450.stgit@devnote2?part=6
