From: Christian Borntraeger <borntraeger@de.ibm.com>
To: Pierre Morel <pmorel@linux.ibm.com>
Cc: alex.williamson@redhat.com, cohuck@redhat.com,
linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
kvm@vger.kernel.org, frankja@linux.ibm.com,
akrowiak@linux.ibm.com, pasic@linux.ibm.com, david@redhat.com,
schwidefsky@de.ibm.com, heiko.carstens@de.ibm.com,
freude@linux.ibm.com, mimu@linux.ibm.com
Subject: Re: [PATCH v4 5/7] s390: ap: implement PAPQ AQIC interception in kernel
Date: Thu, 28 Feb 2019 21:20:49 +0100 [thread overview]
Message-ID: <a7be3d71-6392-4ff3-5bd5-60429b8a61c0@de.ibm.com> (raw)
In-Reply-To: <1550849400-27152-6-git-send-email-pmorel@linux.ibm.com>
On 22.02.2019 16:29, Pierre Morel wrote:
> We register the AP PQAP instruction hook during the open
> of the mediated device. And unregister it on release.
>
> In the AP PQAP instruction hook, if we receive a demand to
> enable IRQs,
> - we retrieve the vfio_ap_queue based on the APQN we receive
> in REG1,
> - we retrieve the page of the guest address, (NIB), from
> register REG2
> - we the mediated device to use the VFIO pinning infratrsucture
> to pin the page of the guest address,
> - we retrieve the pointer to KVM to register the guest ISC
> and retrieve the host ISC
> - finaly we activate GISA
>
> If we receive a demand to disable IRQs,
> - we deactivate GISA
> - unregister from the GIB
> - unping the NIB
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
> arch/s390/include/asm/kvm_host.h | 1 +
> drivers/s390/crypto/ap_bus.h | 1 +
> drivers/s390/crypto/vfio_ap_ops.c | 199 +++++++++++++++++++++++++++++++++-
> drivers/s390/crypto/vfio_ap_private.h | 1 +
> 4 files changed, 199 insertions(+), 3 deletions(-)
>
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 49cc8b0..5f3bb8c 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -720,6 +720,7 @@ struct kvm_s390_cpu_model {
> struct kvm_s390_crypto {
> struct kvm_s390_crypto_cb *crycb;
> int (*pqap_hook)(struct kvm_vcpu *vcpu);
> + void *vfio_private;
> __u32 crycbd;
> __u8 aes_kw;
> __u8 dea_kw;
> diff --git a/drivers/s390/crypto/ap_bus.h b/drivers/s390/crypto/ap_bus.h
> index bfc66e4..323f2aa 100644
> --- a/drivers/s390/crypto/ap_bus.h
> +++ b/drivers/s390/crypto/ap_bus.h
> @@ -43,6 +43,7 @@ static inline int ap_test_bit(unsigned int *ptr, unsigned int nr)
> #define AP_RESPONSE_BUSY 0x05
> #define AP_RESPONSE_INVALID_ADDRESS 0x06
> #define AP_RESPONSE_OTHERWISE_CHANGED 0x07
> +#define AP_RESPONSE_INVALID_GISA 0x08
> #define AP_RESPONSE_Q_FULL 0x10
> #define AP_RESPONSE_NO_PENDING_REPLY 0x10
> #define AP_RESPONSE_INDEX_TOO_BIG 0x11
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 1b5130a..0196065 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -43,7 +43,7 @@ struct vfio_ap_queue *vfio_ap_get_queue(int apqn, struct list_head *l)
> return NULL;
> }
>
> -static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q)
> +int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q)
> {
> struct ap_queue_status status;
> int retry = 20;
> @@ -75,6 +75,27 @@ static int vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q)
> return -EBUSY;
> }
>
> +/**
> + * vfio_ap_free_irq:
> + * @q: The vfio_ap_queue
> + *
> + * Unpin the guest NIB
> + * Unregister the ISC from the GIB alert
> + * Clear the vfio_ap_queue intern fields
> + */
> +static void vfio_ap_free_irq(struct vfio_ap_queue *q)
> +{
> + if (!q)
> + return;
> + if (q->g_pfn)
> + vfio_unpin_pages(mdev_dev(q->matrix_mdev->mdev), &q->g_pfn, 1);
> + if (q->isc)
> + kvm_s390_gisc_unregister(q->matrix_mdev->kvm, q->isc);
> + q->nib = 0;
> + q->isc = 0;
> + q->g_pfn = 0;
> +}
Pierre, unless there is some magic, I think we need to free the gisa stuff before kvm exit.
Imagine a malicious userspace that setups everything fine, but then closes all kvm file
descriptors but not the vfio file descriptor. This might result in random access to the
memory that contained the gisa potentially resulting in random memory overwrites.
the problem is that kvm_destroy_vm calls kvm_arch_destroy_vm(kvm) before it calls
kvm_destroy_devices(kvm); So we already free the gisa before we do the unregister call.
What about calling kvm_get_kvm/put from some of the callbacks in the right places.
Debugging random memory overwrites is a PITA, so we either should document why I cannot
happen (even with malicious userspace) or simply fix the refcounting.
next prev parent reply other threads:[~2019-02-28 20:21 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-22 15:29 [PATCH v4 0/7] vfio: ap: AP Queue Interrupt Control Pierre Morel
2019-02-22 15:29 ` [PATCH v4 1/7] s390: ap: kvm: add PQAP interception for AQIC Pierre Morel
2019-02-25 18:36 ` Tony Krowiak
2019-02-26 11:47 ` Pierre Morel
2019-02-26 15:47 ` Tony Krowiak
2019-02-27 8:09 ` Pierre Morel
2019-02-27 9:13 ` Cornelia Huck
2019-02-27 10:16 ` Pierre Morel
2019-02-27 18:00 ` Tony Krowiak
2019-02-28 9:42 ` Christian Borntraeger
2019-02-28 11:03 ` Christian Borntraeger
2019-02-28 11:22 ` Cornelia Huck
2019-02-28 13:16 ` Pierre Morel
2019-02-28 13:52 ` Cornelia Huck
2019-02-28 14:14 ` Pierre Morel
2019-03-01 12:03 ` Pierre Morel
2019-03-01 12:05 ` Christian Borntraeger
2019-03-01 12:36 ` Cornelia Huck
2019-03-01 15:32 ` Pierre Morel
2019-02-28 13:10 ` Pierre Morel
2019-02-28 15:36 ` Tony Krowiak
2019-02-28 12:39 ` Halil Pasic
2019-02-28 14:12 ` Pierre Morel
2019-02-28 16:51 ` Halil Pasic
2019-03-01 12:10 ` Pierre Morel
2019-02-28 15:43 ` Tony Krowiak
2019-02-28 13:23 ` Pierre Morel
2019-02-28 13:44 ` Christian Borntraeger
2019-02-28 13:47 ` Pierre Morel
2019-02-28 14:07 ` Halil Pasic
2019-02-28 14:13 ` Pierre Morel
2019-02-28 15:45 ` Tony Krowiak
2019-02-28 15:35 ` Tony Krowiak
2019-03-01 8:42 ` Christian Borntraeger
2019-02-28 8:31 ` Christian Borntraeger
2019-02-22 15:29 ` [PATCH v4 2/7] s390: ap: new vfio_ap_queue structure Pierre Morel
2019-02-26 16:10 ` Tony Krowiak
2019-02-27 8:40 ` Pierre Morel
2019-02-27 20:35 ` Tony Krowiak
2019-02-22 15:29 ` [PATCH v4 3/7] s390: ap: associate a ap_vfio_queue and a matrix mdev Pierre Morel
2019-02-26 18:14 ` Tony Krowiak
2019-02-27 9:29 ` Pierre Morel
2019-02-27 20:14 ` Tony Krowiak
2019-02-27 9:32 ` Cornelia Huck
2019-02-27 10:21 ` Pierre Morel
2019-02-27 10:44 ` Pierre Morel
2019-02-27 20:53 ` Tony Krowiak
2019-03-04 2:09 ` Halil Pasic
2019-03-04 10:19 ` Pierre Morel
2019-03-05 22:17 ` Tony Krowiak
2019-03-12 21:39 ` Tony Krowiak
2019-03-13 10:19 ` Pierre Morel
2019-02-22 15:29 ` [PATCH v4 4/7] vfio: ap: register IOMMU VFIO notifier Pierre Morel
2019-02-27 9:42 ` Cornelia Huck
2019-02-27 10:22 ` Pierre Morel
2019-02-28 8:23 ` Christian Borntraeger
2019-02-28 8:48 ` Pierre Morel
2019-02-28 16:55 ` Halil Pasic
2019-03-01 7:51 ` Christian Borntraeger
2019-02-22 15:29 ` [PATCH v4 5/7] s390: ap: implement PAPQ AQIC interception in kernel Pierre Morel
2019-02-26 18:23 ` Tony Krowiak
2019-02-27 9:54 ` Pierre Morel
2019-02-27 18:17 ` Tony Krowiak
2019-02-27 18:18 ` Tony Krowiak
2019-02-28 20:20 ` Christian Borntraeger [this message]
2019-03-01 9:35 ` Pierre Morel
2019-03-04 1:57 ` Halil Pasic
2019-03-04 9:47 ` Pierre Morel
2019-02-22 15:29 ` [PATCH v4 6/7] s390: ap: Cleanup on removing the AP device Pierre Morel
2019-02-26 18:27 ` Tony Krowiak
2019-02-27 9:58 ` Pierre Morel
2019-03-04 13:02 ` Cornelia Huck
2019-03-08 22:43 ` Tony Krowiak
2019-03-11 8:31 ` Pierre Morel
2019-03-12 21:53 ` Tony Krowiak
2019-03-13 10:15 ` Pierre Morel
2019-02-22 15:30 ` [PATCH v4 7/7] s390: ap: kvm: Enable PQAP/AQIC facility for the guest Pierre Morel
2019-02-28 15:08 ` [PATCH v4 0/7] vfio: ap: AP Queue Interrupt Control Halil Pasic
2019-03-01 9:40 ` Pierre Morel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a7be3d71-6392-4ff3-5bd5-60429b8a61c0@de.ibm.com \
--to=borntraeger@de.ibm.com \
--cc=akrowiak@linux.ibm.com \
--cc=alex.williamson@redhat.com \
--cc=cohuck@redhat.com \
--cc=david@redhat.com \
--cc=frankja@linux.ibm.com \
--cc=freude@linux.ibm.com \
--cc=heiko.carstens@de.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=mimu@linux.ibm.com \
--cc=pasic@linux.ibm.com \
--cc=pmorel@linux.ibm.com \
--cc=schwidefsky@de.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).