On 09/30/2009 02:04 PM, Arnd Bergmann wrote:
On Tuesday 29 September 2009, Avi Kivity wrote:
r = -EINVAL;
if (log->slot>= KVM_MEMORY_SLOTS)
@@ -718,8 +719,15 @@ int kvm_get_dirty_log(struct kvm *kvm,
for (i = 0; !any&& i< n/sizeof(long); ++i)
any = memslot->dirty_bitmap[i];
+#if defined(__BIG_ENDIAN)&& defined(CONFIG_64BIT)
+ /* Need to convert user pointers */
+ if (test_thread_flag(TIF_32BIT))
+ target_bm = (void*)((u64)log->dirty_bitmap>> 32);
+ else
+#endif
+ target_bm = log->dirty_bitmap;
r = -EFAULT;
- if (copy_to_user(log->dirty_bitmap, memslot->dirty_bitmap, n))
+ if (copy_to_user(target_bm, memslot->dirty_bitmap, n))
goto out;
if (any)
Ah, that's much better. Plus a mental note not to put pointers in
user-visible structures in the future. This can serve as a reminder :)
It's still broken on s390, which
1. uses TIF_31BIT instead of TIF_32BIT
2. needs to call compat_ptr() to do a real conversion instead of a cast
The TIF_32BIT method is also not reliable. E.g. on x86_64 you are supposed
to get the 32 bit ABI when calling through INT80 instead of syscall/sysenter,
independent of the value of TIF_32BIT.
A better way to do this is to add a separate compat_ioctl() method that
converts this for you.
The patch below is an example for the canonical way to do this. Not tested!
Signed-off-by: Arnd Bergmann<arnd@xxxxxxxx>
---
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 897bff3..20f88ad 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2297,6 +2297,49 @@ 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 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.log);
+ if (r)
+ goto out;
+ break;
+ default:
+ r = kvm_vm_ioctl(filp, ioctl, arg);
+ }
+
+ return r;
+}
+#endif
+
static int kvm_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
struct page *page[1];
@@ -2331,7 +2374,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,
.mmap = kvm_vm_mmap,
};
This is a bit painful - I tried to avoid compat_ioctl. Maybe it's
better to have dirty_bitmap_virt, given no existing users are impacted.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html