Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot

[Jike Song <jike.song@intel.com>]

On 10/21/2016 01:06 AM, Paolo Bonzini wrote:
> On 20/10/2016 03:48, Xiao Guangrong wrote:
>> I understood that KVM side is safe, however, vfio side is independent with
>> kvm and the user of usrdata can fetch kvm struct at any time, consider
>> this scenario:
>>
>> CPU 0                         CPU 1
>> KVM:                         VFIO/userdata user
>>   kvm_ioctl_create_device
>>      get_kvm()
>>                             vfio_group_get_usrdata(vfio_group)
>>   kvm_device_release
>>     put_kvm()
>>                             !!! kvm refcount has gone
>>                             use KVM struct
>>
>> Then, the user of userdata have fetched kvm struct but the refcount has
>> already gone.
> 
> vfio_group_set_usrdata (actually) kvm_vfio_group_set_kvm has called
> kvm_get_kvm too, however.  What you need is a mutex that is taken by
> vfio_group_set_usrdata and by the callers of vfio_group_get_usrdata.

Hi Paolo & Guangrong,

I walked the whole thread and became a little nervous: I don't want
to introduce a global mutex.

The problem is, as I understand, vfio_group_get_usrdata() returns a
KVM pointer but it may be stale. To make the pointer always valid,
it can call kvm_get_kvm() *before* return the pointer.

I would apologize in advance if this idea turns out totally
nonsense, but hey, please kindly help fix my whim :-)


[vfio.h]

	struct vfio_usrdata {
		void *data;
		void (*get)(void *data);
		void (*put)(void *data)
	};

	vfio_group {
		...
		vfio_usrdata *usrdata;

[kvm.ko]

	struvt vfio_usrdata kvmdata = {
		.data = kvm,
		.get = kvm_get_kvm,
		.put = kvm_put_kvm,
	};

	fn = symbol_get(vfio_group_set_usrdata)
	fn(vfio_group, &kvmdata)


[vfio.ko]

	vfio_group_set_usrdata
		lock
		vfio_group->d = kvmdata
		unlock

	void *vfio_group_get_usrdata
		lock
		struct vfio_usrdata *d = vfio_group->usrdata;
		d->get(d->data);
		unlock
		return d->data;

	void vfio_group_put_usrdata
		lock
		struct vfio_usrdata *d = vfio_group->usrdata;
		d->put(d->data)
		unlock

[kvmgt.ko]

	call vfio_group_get_usrdata to get kvm,
	call vfio_group_put_usrdata to release it
	*never* call kvm_get_kvm/kvm_put_kvm

--
Thanks,
Jike