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

[Jike Song]

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