Re: [PATCH v2 06/28] KVM: selftests: test hypercall memory fault exits

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] kvm: selftests: host OOB write in test_readonly_input()
- [Medium] kvm: selftests: missing read-only memory map in test_clock_pairing_fault()
--

commit 363dccb0211d2629dd610b0d28c041e51635155e
Author: Paolo Bonzini <pbonzini@redhat.com>

KVM: selftests: test hypercall memory fault exits

This commit introduces selftests for KVM hypercall memory faults. It verifies
that hypercalls with invalid or read-only guest physical addresses exit to
userspace correctly.

> diff --git a/tools/testing/selftests/kvm/x86/hcall_fault.c b/tools/testing/selftests/kvm/x86/hcall_fault.c
> new file mode 100644
> index 0000000000000..63e38c87616f9
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/hcall_fault.c
[ ... ]
> +/* Read-only hypercalls, read-only inputs, valid/invalid argument */
> +static void test_readonly_input(void)
> +{
> +	struct hv_tlb_flush *flush;
> +	struct hcall_test *test;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	gpa_t hcall_page;
> +
> +	vm = create_vm(&vcpu, &test, &hcall_page);
> +	flush = (struct hv_tlb_flush *)&test->readonly_data;
> +	flush->flags = HV_FLUSH_ALL_VIRTUAL_ADDRESS_SPACES | HV_FLUSH_ALL_PROCESSORS;
> +	flush->processor_mask = 0;

[Severity: Medium]
Will this result in an out-of-bounds host memory write?

The test allocates two separate 1-page memslots for TEST_MEM_GPA and
READONLY_GPA. In the guest, they are mapped contiguously to test_gva. However,
create_vm() gets the host virtual address of only the first page and assigns
it to the test pointer.

By accessing test->readonly_data, does this write past the 1-page host
allocation of the first memslot?

> +	test->control = HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE;
> +	test->ingpa = READONLY_GPA;
> +	test->outgpa = UNMAPPED_OUT_GPA;
[ ... ]
> +static void test_clock_pairing_fault(bool readonly)
> +{
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	gpa_t gpa;
> +
> +	/* Unlike create_vm do not enable Hyper-V hypercalls. */
> +	vm = vm_create_with_one_vcpu(&vcpu, clock_pairing_guest);
> +	vm_enable_cap(vm, KVM_CAP_HCALL_FAULT_EXIT, 1);
> +	gpa = readonly ? READONLY_GPA : UNMAPPED_OUT_GPA;

[Severity: Medium]
When readonly is true, does this inadvertently test an unmapped write fault
instead of a read-only write fault?

Since test_clock_pairing_fault() creates a new VM with
vm_create_with_one_vcpu() instead of using create_vm(), it appears
READONLY_GPA is never actually mapped into the guest as a read-only memslot.

Will KVM treat READONLY_GPA as completely unmapped memory here, missing the
intended coverage for read-only memory faults?

> +	vcpu_args_set(vcpu, 1, gpa);
> +
> +	assert_memory_fault(vcpu, gpa, KVM_MEMORY_EXIT_FLAG_WRITE);
> +	kvm_vm_free(vm);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918081543.139871-1-pbonzini@redhat.com?part=6