Re: [PATCH RFC v2 6/6] KVM: introduce a new API for getting dirty bitmaps

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 22.04.2010, at 08:09, Fernando Luis Vázquez Cao wrote:

> On 04/22/2010 11:45 AM, Fernando Luis Vázquez Cao wrote:
>> On 04/21/2010 06:41 PM, Alexander Graf wrote:
>>> On 21.04.2010, at 10:29, Fernando Luis Vázquez Cao wrote:
>>> 
>>>> On 04/20/2010 08:03 PM, Takuya Yoshikawa wrote:
>>>>> @@ -318,7 +318,7 @@ struct kvm_dirty_log {
>>>>> 	__u32 padding1;
>>>>> 	union {
>>>>> 		void __user *dirty_bitmap; /* one bit per page */
>>>>> -		__u64 padding2;
>>>>> +		__u64 addr;
>>>> 
>>>> This can break on x86_32 and x86_64-compat. addr is a long not a __u64.
>>> 
>>> So the high 32 bits are zero. Where's the problem?
>> 
>> If we are careful enough to cast the addr appropriately we should be fine,
>> even if we keep the padding field in the union. I am not saying that it
>> breaks 32 architectures but that it can potentially be problematic.
>> 
>>>>> +	case KVM_SWITCH_DIRTY_LOG: {
>>>>> +		struct kvm_dirty_log log;
>>>>> +
>>>>> +		r = -EFAULT;
>>>>> +		if (copy_from_user(&log, argp, sizeof log))
>>>>> +			goto out;
>>>>> +		r = kvm_vm_ioctl_switch_dirty_log(kvm, &log);
>>>>> +		if (r)
>>>>> +			goto out;
>>>>> +		r = -EFAULT;
>>>>> +		if (copy_to_user(argp, &log, sizeof log))
>>>>> +			goto out;
>>>>> +		r = 0;
>>>>> +		break;
>>>>> +	}
>>>> 
>>>> In x86_64-compat mode we are handling 32bit user-space addresses
>>>> so we need the compat counterpart of KVM_SWITCH_DIRTY_LOG too.
>>> 
>>> The compat code just forwards everything to the generic ioctls.
>> 
>> The compat code uses struct compat_kvm_dirty_log instead of
>> struct kvm_dirty_log to communicate with user space so
>> the necessary conversions needs to be done before invoking
>> the generic ioctl (see KVM_GET_DIRTY_LOG in kvm_vm_compat_ioctl).
>> 
>> By the way we probable should move the definition of struct
>> compat_kvm_dirty_log to a header file.
> 
> It seems that it was you and Arnd who added the kvm_vm compat ioctl :-).
> Are you considering a different approach to tackle the issues that we
> have with a big-endian userspace?

IIRC the issue was a pointer inside of a nested structure, no?


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux