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
