Re: [PATCH v2] KVM: selftests: Test partial KVM_CLEAR_DIRTY_LOG requests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: selftests: out-of-bounds memory read in dirty_log_test
--

commit 91a9082b8baef537794ce258865b745e98facf50
Author: Tharit Tangkijwanichakul <tharitt97@gmail.com>

KVM: selftests: Test partial KVM_CLEAR_DIRTY_LOG requests

This commit adds a test to verify that KVM correctly applies a clear log
bitmap relative to a nonzero first_page offset. It introduces
run_clear_log_partial_test() to ensure that pages not selected by a
partial request remain dirty.

> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> index af5eb0334a74f..3976748f39400 100644
> --- a/tools/testing/selftests/kvm/dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
[ ... ]
> @@ -815,6 +815,114 @@ static void run_test(enum vm_guest_mode mode, void *arg)
[ ... ]
> +static void run_clear_log_partial_test(enum vm_guest_mode mode, void *arg)
> +{
> +	unsigned long *bmap, *clear_bmap;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	u64 page;
[ ... ]
> +	bmap = bitmap_zalloc(host_num_pages);
> +	clear_bmap = bitmap_zalloc(BITS_PER_LONG);

[Severity: Medium]
Does this allocate an undersized buffer on 32-bit systems?

Using BITS_PER_LONG allocates 32 bits (4 bytes) on 32-bit architectures,
rather than the 64 bits mandated by the KVM API bit count for the request
below.

> +
> +	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
[ ... ]
> +	__set_bit_le(TEST_CLEAR_LOG_BIT, clear_bmap);
> +	kvm_vm_clear_dirty_log(vm, TEST_MEM_SLOT_INDEX, clear_bmap,
> +			       TEST_CLEAR_LOG_FIRST_PAGE,
> +			       TEST_CLEAR_LOG_NUM_PAGES);

[Severity: Medium]
Will this trigger an out-of-bounds read of userspace heap memory?

The test passes clear_bmap alongside TEST_CLEAR_LOG_NUM_PAGES (which is 64)
to kvm_vm_clear_dirty_log(). This instructs the kernel to read 64 bits (8
bytes) of dirty log data. 

Since clear_bmap is only 4 bytes long on 32-bit systems, this appears to cause
the kernel to read 4 bytes past the end of the allocated buffer.

> +
> +	kvm_vm_get_dirty_log(vm, TEST_MEM_SLOT_INDEX, bmap);
[ ... ]

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