From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [Qemu-devel] [PATCH 1/2] KVM: page track: add a new notifier type: track_flush_slot Date: Thu, 20 Oct 2016 19:06:48 +0200 Message-ID: References: <1259cdba-c137-c3da-abe2-ecf51aec6738@linux.intel.com> <523e1446-75f1-fe3a-d818-f7d238d57751@redhat.com> <5800B579.9000705@intel.com> <20161014084158.623087aa@t450s.home> <20161014084601.2a50ba87@t450s.home> <20161014163545.GA6121@nvidia.com> <20161014105124.42b438a6@t450s.home> <20161014221901.GA8865@nvidia.com> <20161017100229.1474ae33@t450s.home> <580617BD.8000300@intel.com> <20161018085918.61ec0e93@t450s.home> <5806DB2D.6090306@intel.com> <2f04a53d-261c-7fb5-6825-117da6a1307d@intel.com> <06340187-61d8-ed7a-e40d-264ca3eb4b37@linux.intel.com> <6067bf7d-42ba-0ffd-5131-da74f60296d4@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: "Tian, Kevin" , Neo Jia , kvm@vger.kernel.org, qemu-devel , Xiaoguang Chen , Kirti Wankhede To: Xiao Guangrong , Xiao Guangrong , Jike Song , Alex Williamson Return-path: Received: from mail-lf0-f66.google.com ([209.85.215.66]:33479 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933416AbcJTRGz (ORCPT ); Thu, 20 Oct 2016 13:06:55 -0400 Received: by mail-lf0-f66.google.com with SMTP id l131so1060870lfl.0 for ; Thu, 20 Oct 2016 10:06:54 -0700 (PDT) In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 20/10/2016 03:48, Xiao Guangrong wrote: > > > On 10/19/2016 10:14 PM, Paolo Bonzini wrote: >> >> >> On 19/10/2016 15:39, Xiao Guangrong wrote: >>> >>> >>> On 10/19/2016 07:56 PM, Paolo Bonzini wrote: >>>> >>>> >>>> On 19/10/2016 07:45, Xiao Guangrong wrote: >>>>> >>>>> >>>>> On 10/19/2016 10:32 AM, Jike Song wrote: >>>>> +EXPORT_SYMBOL_GPL(vfio_group_set_usrdata); >>>>>>>> + >>>>>>>> +void *vfio_group_get_usrdata(struct vfio_group *group) >>>>>>>> +{ >>>>>>>> + return group->usrdata; >>>>>>>> +} >>>>>>>> +EXPORT_SYMBOL_GPL(vfio_group_get_usrdata); >>>>>>>> + >>>>>>>> +void *vfio_group_get_usrdata_by_device(struct device *dev) >>>>>>>> +{ >>>>>>>> + struct vfio_group *vfio_group; >>>>>>>> + >>>>>>>> + vfio_group = __vfio_group_get_from_iommu(dev->iommu_group); >>>>>>> >>>>>>> We actually need to use iommu_group_get() here. Kirti adds a >>>>>>> vfio_group_get_from_dev() in v9 03/12 that does this properly. >>>>>>> >>>>>>>> + if (!vfio_group) >>>>>>>> + return NULL; >>>>>>>> + >>>>>>>> + return vfio_group_get_usrdata(vfio_group); >>>>> >>>>> I am worrying if the kvm instance got from group->usrdata is safe >>>>> enough? What happens if you get the instance after kvm released >>>>> kvm-vfio device? >>>> >>>> It shouldn't happen if you use kvm_get_kvm and kvm_put_kvm >>>> properly. It >>>> is almost okay in the patch, just: >>> >>> How about if KVM releases kvm-vfio device between >>> vfio_group_get_usrdata() >>> and get_kvm()? >> >> That cannot happen as long as there is a struct file* for the device >> (see kvm_ioctl_create_device and kvm_device_release). Since you're >> sending a ioctl to it, it's fine. > > 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. Paolo > What i missed? > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >