Re: [PATCH 1/2] KVM: selftests: Actually compare sregs and events in sync_regs_test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] KVM's `__get_sregs` does not zero-initialize the `interrupt_bitmap` array, leading to stale interrupt state remaining persistently visible to userspace in the memory-mapped `kvm_run` structure.
--

commit fa07ac80146018f01ced6cfdfcb90c42aec2c65c
Author: Hemanth Selam <hemanth.selam@gmail.com>

KVM: selftests: Actually compare sregs and events in sync_regs_test

This commit populates compare_sregs() and compare_vcpu_events() to verify
all fields of struct kvm_sregs and struct kvm_vcpu_events. Previously,
these functions were empty, meaning the tests only verified general purpose
registers after each KVM_RUN.

> diff --git a/tools/testing/selftests/kvm/x86/sync_regs_test.c b/tools/testing/selftests/kvm/x86/sync_regs_test.c
> index ed0c21b427c90..c7d1f3cca16e4 100644
> --- a/tools/testing/selftests/kvm/x86/sync_regs_test.c
> +++ b/tools/testing/selftests/kvm/x86/sync_regs_test.c
[ ... ]
> @@ -70,13 +70,102 @@ static void compare_regs(struct kvm_regs *left, struct kvm_regs *right)
[ ... ]
> +	SREG_COMPARE(efer);
> +	SREG_COMPARE(apic_base);
> +#undef SREG_COMPARE
> +	TEST_ASSERT(!memcmp(left->interrupt_bitmap, right->interrupt_bitmap,
> +			    sizeof(left->interrupt_bitmap)),
> +		    "interrupt_bitmap values did not match");

[Severity: High]
This is a pre-existing issue, but does this new assertion trigger test
failures due to how __get_sregs() handles the interrupt_bitmap?

Looking at __get_sregs() in arch/x86/kvm/regs.c:

	if (vcpu->arch.interrupt.injected && !vcpu->arch.interrupt.soft)
		set_bit(vcpu->arch.interrupt.nr,
			(unsigned long *)sregs->interrupt_bitmap);

This function sets the bit corresponding to the currently injected interrupt
but never clears the prior contents of the array.

If userspace relies on KVM_SYNC_X86_SREGS to capture guest state, KVM copies
the sregs state to run->s.regs.sregs via kvm_run_sync_regs_to_user().

Because run->s.regs.sregs is in the user-mapped kvm_run structure and is
not zero-initialized by the kernel on each sync, it continuously accumulates
injected interrupts or retains stale data placed there by userspace.

Could this lead to spurious interrupt injections or corrupted state during
live migration for any VMM using the KVM_SYNC_X86_SREGS API?

>  }
>  
>  static void compare_vcpu_events(struct kvm_vcpu_events *left,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902120235.18486-1-hemanth.selam@gmail.com?part=1