| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <e1ff9918-a1ab-31b1-0b32-a3980770d2ac@linux.ibm.com>
Date: Fri, 21 Sep 2018 19:28:20 -0400
From: Tony Krowiak <akrowiak@...ux.ibm.com>
To: Tony Krowiak <akrowiak@...ux.vnet.ibm.com>,
linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
kvm@...r.kernel.org
Cc: freude@...ibm.com, schwidefsky@...ibm.com,
heiko.carstens@...ibm.com, borntraeger@...ibm.com,
cohuck@...hat.com, kwankhede@...dia.com,
bjsdjshi@...ux.vnet.ibm.com, pbonzini@...hat.com,
alex.williamson@...hat.com, pmorel@...ux.vnet.ibm.com,
alifm@...ux.vnet.ibm.com, mjrosato@...ux.vnet.ibm.com,
jjherne@...ux.vnet.ibm.com, thuth@...hat.com,
pasic@...ux.vnet.ibm.com, berrange@...hat.com,
fiuczy@...ux.vnet.ibm.com, buendgen@...ibm.com,
frankja@...ux.ibm.com
Subject: Re: [PATCH v10 11/26] s390: vfio-ap: implement mediated device open
callback
On 09/12/2018 03:43 PM, Tony Krowiak wrote:
> From: Tony Krowiak <akrowiak@...ux.ibm.com>
>
> Implements the open callback on the mediated matrix device.
> The function registers a group notifier to receive notification
> of the VFIO_GROUP_NOTIFY_SET_KVM event. When notified,
> the vfio_ap device driver will get access to the guest's
> kvm structure. The open callback must ensure that only one
> mediated device shall be opened per guest.
>
> Signed-off-by: Tony Krowiak <akrowiak@...ux.ibm.com>
> Acked-by: Halil Pasic <pasic@...ux.ibm.com>
> Tested-by: Michael Mueller <mimu@...ux.ibm.com>
> Tested-by: Farhan Ali <alifm@...ux.ibm.com>
> Tested-by: Pierre Morel <pmorel@...ux.ibm.com>
> Acked-by: Pierre Morel <pmorel@...ux.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@...ibm.com>
> ---
> arch/s390/include/asm/kvm_host.h | 1 +
> drivers/s390/crypto/vfio_ap_ops.c | 168 +++++++++++++++++++++++++++++++++
> drivers/s390/crypto/vfio_ap_private.h | 5 +
> 3 files changed, 174 insertions(+), 0 deletions(-)
>
(...)
> @@ -699,12 +730,149 @@ static ssize_t matrix_show(struct device *dev, struct device_attribute *attr,
> NULL
> };
>
> +/**
> + * Verify that the AP instructions are available on the guest. This is indicated
> + * via the KVM_S390_VM_CPU_FEAT_AP CPU model feature.
> + */
> +static int kvm_ap_validate_crypto_setup(struct kvm *kvm)
> +{
> + if (test_bit_inv(KVM_S390_VM_CPU_FEAT_AP, kvm->arch.cpu_feat))
> + return 0;
> +
> + return -EOPNOTSUPP;
> +}
> +
(...)
> +
> +/**
> + * 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);
> + 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;
> +}
> +
> +static int vfio_ap_mdev_open(struct mdev_device *mdev)
> +{
> + struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
> + unsigned long events;
> + int ret;
> +
> +
> + if (!try_module_get(THIS_MODULE))
> + return -ENODEV;
> +
> + matrix_mdev->group_notifier.notifier_call = vfio_ap_mdev_group_notifier;
> + events = VFIO_GROUP_NOTIFY_SET_KVM;
> +
> + ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
> + &events, &matrix_mdev->group_notifier);
> + if (ret) {
> + module_put(THIS_MODULE);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
(...)
The fixup! patch below modifies this patch (11/26) to illustrate how
David's recommendation will be implemented for v11 of the series. It
is one of three fixup! patches (the other two are in responses to
03/26 and 25/26) included to generate discussion in v10 rather than
waiting until v11 for comments.
-----------------------------------8<-----------------------------------
From: Tony Krowiak <akrowiak@...ux.ibm.com>
Date: Thu, 20 Sep 2018 12:01:53 -0400
Subject: [FIXUP v10] fixup!: s390: vfio-ap: implement mediated device
open callback
* Fix race condition in KVM notifier
* Remove test for KVM_S390_VM_CPU_FEAT_AP
---
drivers/s390/crypto/vfio_ap_ops.c | 26 +++++++++++++++-----------
1 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c
b/drivers/s390/crypto/vfio_ap_ops.c
index 8bc0cdd..573a5cc 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -731,12 +731,12 @@ static ssize_t matrix_show(struct device *dev,
struct device_attribute *attr,
};
/**
- * Verify that the AP instructions are available on the guest. This is
indicated
- * via the KVM_S390_VM_CPU_FEAT_AP CPU model feature.
+ * Verify that the AP instructions are being interpreted by firmware
for the
+ * guest. This is indicated by the kvm->arch.crypto.apie flag.
*/
static int kvm_ap_validate_crypto_setup(struct kvm *kvm)
{
- if (test_bit_inv(KVM_S390_VM_CPU_FEAT_AP, kvm->arch.cpu_feat))
+ if (kvm->arch.crypto.apie)
return 0;
return -EOPNOTSUPP;
@@ -772,15 +772,19 @@ static void vfio_ap_mdev_copy_masks(struct
ap_matrix_mdev *matrix_mdev)
}
/**
- * vfio_ap_mdev_open_once
+ * vfio_ap_mdev_set_kvm
*
* @matrix_mdev: a mediated matrix device
+ * @kvm: reference to KVM instance
*
- * Return 0 if no other mediated matrix device has been opened for the
- * KVM guest assigned to @matrix_mdev; otherwise, returns an error.
+ * Verifies no other mediated matrix device has a reference to @kvm and
sets a
+ * reference to it in @matrix_mdev->kvm.
+ *
+ * Return 0 if no other mediated matrix device has a reference to @kvm;
+ * otherwise, returns -EPERM.
*/
-static int vfio_ap_mdev_open_once(struct ap_matrix_mdev *matrix_mdev,
- struct kvm *kvm)
+static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
+ struct kvm *kvm)
{
struct ap_matrix_mdev *m;
@@ -793,6 +797,7 @@ static int vfio_ap_mdev_open_once(struct
ap_matrix_mdev *matrix_mdev,
}
}
+ matrix_mdev->kvm = kvm;
mutex_unlock(&matrix_dev->lock);
return 0;
@@ -814,16 +819,15 @@ static int vfio_ap_mdev_group_notifier(struct
notifier_block *nb,
return NOTIFY_OK;
}
- ret = vfio_ap_mdev_open_once(matrix_mdev, data);
+ ret = vfio_ap_mdev_set_kvm(matrix_mdev, data);
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;
--
1.7.1
Powered by blists - more mailing lists