Re: [PATCH v17 02/13] perf/x86, KVM: Prevent host debug register leak into guest OS on NMI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] perf/x86: -EBUSY in arch_install_hw_breakpoint breaks KGDB and misses NMI clobbers
--

commit f10b90aff527e68c852cc3297fd95e985f01fa91
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

perf/x86, KVM: Prevent host debug register leak into guest OS on NMI

When KVM enters a guest OS, host hardware breakpoints are disabled
before running the guest. However, an NMI can occur while executing
in guest mode or during guest transition, where
arch_install_hw_breakpoint() can be invoked from NMI context.

If arch_install_hw_breakpoint() is executed while the CPU is in guest
mode or during the VM entry transition window, hardware debug registers
can be modified with host breakpoint settings, leaking host breakpoints
into the guest OS or clobbering the guest's debug registers.

> diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
> index f846c15f21ca1..0473a5c958566 100644
> --- a/arch/x86/kernel/hw_breakpoint.c
> +++ b/arch/x86/kernel/hw_breakpoint.c
> @@ -102,6 +102,9 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
>  
>  	lockdep_assert_irqs_disabled();
>  
> +	if (perf_guest_in_guest())
> +		return -EBUSY;
> +

[Severity: High]
Does returning -EBUSY here break KGDB?

If an NMI (e.g., KGDB entry) occurs while the CPU is in guest mode, this
will return -EBUSY. In arch/x86/kernel/kgdb.c:kgdb_correct_hw_break(), it
receives this and leaves the breakpoint permanently disabled, since it does
not retry:

	val = arch_install_hw_breakpoint(bp);
	if (!val)
		bp->attr.disabled = 0;

Also, since arch_uninstall_hw_breakpoint() and hw_breakpoint_restore() in
arch/x86/kernel/hw_breakpoint.c do not have this perf_guest_in_guest()
check, will they still unconditionally write host state to the physical DR7
register?

For example, if KGDB removes breakpoints or resumes the CPU during guest
mode, they might still clobber the guest's active DR7:

arch/x86/kernel/hw_breakpoint.c:arch_uninstall_hw_breakpoint() {
	...
	dr7 &= ~__encode_dr7(i, info->len, info->type);

	set_debugreg(dr7, 7);
	...
}

arch/x86/kernel/hw_breakpoint.c:hw_breakpoint_restore() {
	...
	set_debugreg(__this_cpu_read(cpu_dr7), 7);
	...
}

>  	for (i = 0; i < HBP_NUM; i++) {
>  		struct perf_event **slot = this_cpu_ptr(&bp_per_reg[i]);
>

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