On Wed, Sep 15, 2021 at 2:30 PM David Matlack <dmatlack@xxxxxxxxxx> wrote:
>
> The calculation to get the per-slot dirty bitmap was incorrect leading
> to a buffer overrun. Fix it by dividing the number of pages by
> BITS_PER_LONG, since each element of the bitmap is a long and there is
> one bit per page.
>
> Fixes: 609e6202ea5f ("KVM: selftests: Support multiple slots in dirty_log_perf_test")
> Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>
I was a little confused initially because we're allocating only one
dirty bitmap in userspace even when we have multiple slots, but that's
not a problem.
Reviewed-by: Ben Gardon <bgardon@xxxxxxxxxx>
>
> ---
> tools/testing/selftests/kvm/dirty_log_perf_test.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index 5ad9f2bc7369..0dd4626571e9 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -118,6 +118,12 @@ static inline void disable_dirty_logging(struct kvm_vm *vm, int slots)
> toggle_dirty_logging(vm, slots, false);
> }
>
> +static unsigned long *get_slot_bitmap(unsigned long *bitmap, uint64_t page_offset)
> +{
> + /* Guaranteed to be evenly divisible by the TEST_ASSERT in run_test. */
> + return &bitmap[page_offset / BITS_PER_LONG];
> +}
> +
> static void get_dirty_log(struct kvm_vm *vm, int slots, unsigned long *bitmap,
> uint64_t nr_pages)
> {
> @@ -126,7 +132,8 @@ static void get_dirty_log(struct kvm_vm *vm, int slots, unsigned long *bitmap,
>
> for (i = 0; i < slots; i++) {
> int slot = PERF_TEST_MEM_SLOT_INDEX + i;
> - unsigned long *slot_bitmap = bitmap + i * slot_pages;
> + uint64_t page_offset = slot_pages * i;
> + unsigned long *slot_bitmap = get_slot_bitmap(bitmap, page_offset);
>
> kvm_vm_get_dirty_log(vm, slot, slot_bitmap);
> }
> @@ -140,7 +147,8 @@ static void clear_dirty_log(struct kvm_vm *vm, int slots, unsigned long *bitmap,
>
> for (i = 0; i < slots; i++) {
> int slot = PERF_TEST_MEM_SLOT_INDEX + i;
> - unsigned long *slot_bitmap = bitmap + i * slot_pages;
> + uint64_t page_offset = slot_pages * i;
> + unsigned long *slot_bitmap = get_slot_bitmap(bitmap, page_offset);
>
> kvm_vm_clear_dirty_log(vm, slot, slot_bitmap, 0, slot_pages);
> }
> @@ -172,6 +180,9 @@ static void run_test(enum vm_guest_mode mode, void *arg)
> guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
> host_num_pages = vm_num_host_pages(mode, guest_num_pages);
> bmap = bitmap_alloc(host_num_pages);
> + TEST_ASSERT((host_num_pages / p->slots) % BITS_PER_LONG == 0,
> + "The number of pages per slot must be divisible by %d.",
> + BITS_PER_LONG);
>
> if (dirty_log_manual_caps) {
> cap.cap = KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2;
> --
> 2.33.0.309.g3052b89438-goog
>
