Re: [PATCH 2/2] accel: kvm: Add aligment check for kvm_log_clear_one_slot

[Andrew Jones]

On Tue, Dec 15, 2020 at 03:19:48PM +0800, Keqian Zhu wrote:
> The parameters start and size are transfered from QEMU memory
> emulation layer. It can promise that they are TARGET_PAGE_SIZE
> aligned. However, KVM needs they are qemu_real_page_size aligned.
> 
> Though no caller breaks this aligned requirement currently, we'd
> better add an explicit check to avoid future breaking.
> 
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> ---
>  accel/kvm/kvm-all.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index c5e06288eb..3d0e3aa872 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -701,6 +701,11 @@ static int kvm_log_clear_one_slot(KVMSlot *mem, int 
> as_id, uint64_t start,
>      unsigned long *bmap_clear = NULL, psize = qemu_real_host_page_size;
>      int ret;
>  
> +    /* Make sure start and size are psize aligned */
> +    if (!QEMU_IS_ALIGNED(start, psize) || !QEMU_IS_ALIGNED(size, psize)) {
> +        return -EINVAL;
> +    }
> +
>      /*
>       * We need to extend either the start or the size or both to
>       * satisfy the KVM interface requirement.  Firstly, do the start
> -- 
> 2.23.0
> 
>

It's not clear to me that this function has any restrictions on start
and size. If it does, then please document those restrictions in the
function's header and assert rather than return.

Thanks,
drew