Messages in this thread | | | Subject | Re: [PATCH v10 11/26] s390: vfio-ap: implement mediated device open callback | From | Tony Krowiak <> | Date | Tue, 18 Sep 2018 17:57:31 -0400 |
| |
On 09/18/2018 01:00 PM, Halil Pasic wrote: > > On 09/12/2018 09:43 PM, Tony Krowiak wrote: >> +/** >> + * vfio_ap_mdev_open_once >> + * >> + * @matrix_mdev: a mediated matrix device >> + * >> + * Return 0 if no other mediated matrix device has been opened for the >> + * KVM guest assigned to @matrix_mdev; otherwise, returns an error. >> + */ >> +static int vfio_ap_mdev_open_once(struct ap_matrix_mdev *matrix_mdev, >> + struct kvm *kvm) >> +{ >> + struct ap_matrix_mdev *m; >> + >> + mutex_lock(&matrix_dev->lock); >> + >> + list_for_each_entry(m, &matrix_dev->mdev_list, node) { >> + if ((m != matrix_mdev) && (m->kvm == kvm)) { >> + mutex_unlock(&matrix_dev->lock); >> + return -EPERM; >> + } >> + } >> + >> + mutex_unlock(&matrix_dev->lock); >> + >> + return 0; >> +} >> + >> +static int vfio_ap_mdev_group_notifier(struct notifier_block *nb, >> + unsigned long action, void *data) >> +{ >> + int ret; >> + struct ap_matrix_mdev *matrix_mdev; >> + >> + if (action != VFIO_GROUP_NOTIFY_SET_KVM) >> + return NOTIFY_OK; >> + >> + matrix_mdev = container_of(nb, struct ap_matrix_mdev, group_notifier); >> + >> + if (!data) { >> + matrix_mdev->kvm = NULL; >> + return NOTIFY_OK; >> + } >> + >> + ret = vfio_ap_mdev_open_once(matrix_mdev, data); > This could be racy. Two threads doing vfio_ap_mdev_group_notifier() > can first get 0 here in a sense that there is no such kvm in the list, > and then both set the very same kvm three lines below. Which would > result in what we are trying to prevent. > > Also vfio_ap_mdev_open_once() does not seem like an appropriate name > any more. If we were to do the matrix_mdev->kvm = kvm in there we could > call it something like vfio_ap_mdev_set_kvm().
I'm moving the matrix-mdev->kvm = kvm inside the mutex lock in vfio_ap_mdev_open_once() ... also renaming it to vfio_ap_mdev_set_kvm().
> >> + if (ret) >> + return NOTIFY_DONE; >> + >> + matrix_mdev->kvm = data; >> + >> + ret = kvm_ap_validate_crypto_setup(matrix_mdev->kvm); >> + if (ret) >> + return ret; >> + >> + vfio_ap_mdev_copy_masks(matrix_mdev); >> + >> + return NOTIFY_OK; >> +}
| |