Re: [PATCH] Enable 32bit dirty log pointers on 64bit host

[Jan Kiszka <jan.kiszka@siemens.com>]

Alexander Graf wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> With big endian userspace, we can't quite figure out if a pointer
> is 32 bit (shifted >> 32) or 64 bit when we read a 64 bit pointer.
> 
> This is what happens with dirty logging. To get the pointer interpreted
> correctly, we thus need Arnd's patch to implement a compat layer for
> the ioctl:
> 
> A better way to do this is to add a separate compat_ioctl() method that
> converts this for you.
> 
> From: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Alexander Graf <agraf@suse.de>
> 
> ---
> 
> Changes from Arnd's example version:
> 
>   - s/log.log/log/ (Avi)
>   - use sizeof(compat_log) (Avi)
>   - compile fixes
> ---
>  virt/kvm/kvm_main.c |   49 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 48 insertions(+), 1 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index cac69c4..54a272f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -43,6 +43,7 @@
>  #include <linux/swap.h>
>  #include <linux/bitops.h>
>  #include <linux/spinlock.h>
> +#include <linux/compat.h>
>  
>  #include <asm/processor.h>
>  #include <asm/io.h>
> @@ -1542,6 +1543,52 @@ out:
>  	return r;
>  }
>  
> +#ifdef CONFIG_COMPAT
> +struct compat_kvm_dirty_log {
> +	__u32 slot;
> +	__u32 padding1;
> +	union {
> +		compat_uptr_t dirty_bitmap; /* one bit per page */
> +		__u64 padding2;
> +	};
> +};
> +
> +static long kvm_vm_compat_ioctl(struct file *filp,
> +			   unsigned int ioctl, unsigned long arg)
> +{
> +	struct kvm *kvm = filp->private_data;
> +	int r;
> +
> +	if (kvm->mm != current->mm)
> +		return -EIO;
> +	switch (ioctl) {
> +	case KVM_GET_DIRTY_LOG: {
> +		struct compat_kvm_dirty_log compat_log;
> +		struct kvm_dirty_log log;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&compat_log, (void __user *)arg,
> +				   sizeof(compat_log)))
> +			goto out;
> +		log.slot	 = compat_log.slot;
> +		log.padding1	 = compat_log.padding1;
> +		log.padding2	 = compat_log.padding2;
> +		log.dirty_bitmap = compat_ptr(compat_log.dirty_bitmap);
> +
> +		r = kvm_vm_ioctl_get_dirty_log(kvm, &log);
> +		if (r)
> +			goto out;
> +		break;
> +	}
> +	default:
> +		r = kvm_vm_ioctl(filp, ioctl, arg);
> +	}
> +
> +out:
> +	return r;
> +}
> +#endif
> +
>  static int kvm_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  {
>  	struct page *page[1];
> @@ -1576,7 +1623,7 @@ static int kvm_vm_mmap(struct file *file, struct vm_area_struct *vma)
>  static struct file_operations kvm_vm_fops = {
>  	.release        = kvm_vm_release,
>  	.unlocked_ioctl = kvm_vm_ioctl,
> -	.compat_ioctl   = kvm_vm_ioctl,
> +	.compat_ioctl   = kvm_vm_compat_ioctl,

This fails in the absence of CONFIG_COMPAT.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux