From: Greg Kurz <groug@kaod.org> To: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com> Cc: kvm@vger.kernel.org, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, qemu-devel@nongnu.org, Laurent Vivier <laurent@vivier.eu>, David Gibson <david@gibson.dropbear.id.au> Subject: [PATCH v2 2/2] vhost: Don't call log_access_ok() when using IOTLB Date: Tue, 29 Sep 2020 18:30:44 +0200 [thread overview] Message-ID: <160139704424.162128.7839027287942194310.stgit@bahia.lan> (raw) In-Reply-To: <160139701999.162128.2399875915342200263.stgit@bahia.lan> When the IOTLB device is enabled, the log_guest_addr that is passed by userspace to the VHOST_SET_VRING_ADDR ioctl, and which is then written to vq->log_addr, is a GIOVA. All writes to this address are translated by log_user() to writes to an HVA, and then ultimately logged through the corresponding GPAs in log_write_hva(). No logging will ever occur with vq->log_addr in this case. It is thus wrong to pass vq->log_addr and log_guest_addr to log_access_vq() which assumes they are actual GPAs. Introduce a new vq_log_used_access_ok() helper that only checks accesses to the log for the used structure when there isn't an IOTLB device around. Signed-off-by: Greg Kurz <groug@kaod.org> --- drivers/vhost/vhost.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index c3b49975dc28..5996e32fa818 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -1370,6 +1370,20 @@ bool vhost_log_access_ok(struct vhost_dev *dev) } EXPORT_SYMBOL_GPL(vhost_log_access_ok); +static bool vq_log_used_access_ok(struct vhost_virtqueue *vq, + void __user *log_base, + bool log_used, + u64 log_addr, + size_t log_size) +{ + /* If an IOTLB device is present, log_addr is a GIOVA that + * will never be logged by log_used(). */ + if (vq->iotlb) + return true; + + return !log_used || log_access_ok(log_base, log_addr, log_size); +} + /* Verify access for write logging. */ /* Caller should have vq mutex and device mutex */ static bool vq_log_access_ok(struct vhost_virtqueue *vq, @@ -1377,8 +1391,8 @@ static bool vq_log_access_ok(struct vhost_virtqueue *vq, { return vq_memory_access_ok(log_base, vq->umem, vhost_has_feature(vq, VHOST_F_LOG_ALL)) && - (!vq->log_used || log_access_ok(log_base, vq->log_addr, - vhost_get_used_size(vq, vq->num))); + vq_log_used_access_ok(vq, log_base, vq->log_used, vq->log_addr, + vhost_get_used_size(vq, vq->num)); } /* Can we start vq? */ @@ -1517,8 +1531,9 @@ static long vhost_vring_set_addr(struct vhost_dev *d, return -EINVAL; /* Also validate log access for used ring if enabled. */ - if ((a.flags & (0x1 << VHOST_VRING_F_LOG)) && - !log_access_ok(vq->log_base, a.log_guest_addr, + if (!vq_log_used_access_ok(vq, vq->log_base, + a.flags & (0x1 << VHOST_VRING_F_LOG), + a.log_guest_addr, sizeof *vq->used + vq->num * sizeof *vq->used->ring)) return -EINVAL;
WARNING: multiple messages have this Message-ID (diff)
From: Greg Kurz <groug@kaod.org> To: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com> Cc: kvm@vger.kernel.org, netdev@vger.kernel.org, Laurent Vivier <laurent@vivier.eu>, qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org, David Gibson <david@gibson.dropbear.id.au> Subject: [PATCH v2 2/2] vhost: Don't call log_access_ok() when using IOTLB Date: Tue, 29 Sep 2020 18:30:44 +0200 [thread overview] Message-ID: <160139704424.162128.7839027287942194310.stgit@bahia.lan> (raw) In-Reply-To: <160139701999.162128.2399875915342200263.stgit@bahia.lan> When the IOTLB device is enabled, the log_guest_addr that is passed by userspace to the VHOST_SET_VRING_ADDR ioctl, and which is then written to vq->log_addr, is a GIOVA. All writes to this address are translated by log_user() to writes to an HVA, and then ultimately logged through the corresponding GPAs in log_write_hva(). No logging will ever occur with vq->log_addr in this case. It is thus wrong to pass vq->log_addr and log_guest_addr to log_access_vq() which assumes they are actual GPAs. Introduce a new vq_log_used_access_ok() helper that only checks accesses to the log for the used structure when there isn't an IOTLB device around. Signed-off-by: Greg Kurz <groug@kaod.org> --- drivers/vhost/vhost.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index c3b49975dc28..5996e32fa818 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -1370,6 +1370,20 @@ bool vhost_log_access_ok(struct vhost_dev *dev) } EXPORT_SYMBOL_GPL(vhost_log_access_ok); +static bool vq_log_used_access_ok(struct vhost_virtqueue *vq, + void __user *log_base, + bool log_used, + u64 log_addr, + size_t log_size) +{ + /* If an IOTLB device is present, log_addr is a GIOVA that + * will never be logged by log_used(). */ + if (vq->iotlb) + return true; + + return !log_used || log_access_ok(log_base, log_addr, log_size); +} + /* Verify access for write logging. */ /* Caller should have vq mutex and device mutex */ static bool vq_log_access_ok(struct vhost_virtqueue *vq, @@ -1377,8 +1391,8 @@ static bool vq_log_access_ok(struct vhost_virtqueue *vq, { return vq_memory_access_ok(log_base, vq->umem, vhost_has_feature(vq, VHOST_F_LOG_ALL)) && - (!vq->log_used || log_access_ok(log_base, vq->log_addr, - vhost_get_used_size(vq, vq->num))); + vq_log_used_access_ok(vq, log_base, vq->log_used, vq->log_addr, + vhost_get_used_size(vq, vq->num)); } /* Can we start vq? */ @@ -1517,8 +1531,9 @@ static long vhost_vring_set_addr(struct vhost_dev *d, return -EINVAL; /* Also validate log access for used ring if enabled. */ - if ((a.flags & (0x1 << VHOST_VRING_F_LOG)) && - !log_access_ok(vq->log_base, a.log_guest_addr, + if (!vq_log_used_access_ok(vq, vq->log_base, + a.flags & (0x1 << VHOST_VRING_F_LOG), + a.log_guest_addr, sizeof *vq->used + vq->num * sizeof *vq->used->ring)) return -EINVAL;
next prev parent reply other threads:[~2020-09-29 16:30 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-29 16:30 [PATCH v2 0/2] vhost: Skip access checks on GIOVAs Greg Kurz 2020-09-29 16:30 ` Greg Kurz 2020-09-29 16:30 ` [PATCH v2 1/2] vhost: Don't call access_ok() when using IOTLB Greg Kurz 2020-09-29 16:30 ` Greg Kurz 2020-10-03 1:51 ` Jason Wang 2020-10-03 1:51 ` Jason Wang 2020-10-03 1:51 ` Jason Wang 2020-09-29 16:30 ` Greg Kurz [this message] 2020-09-29 16:30 ` [PATCH v2 2/2] vhost: Don't call log_access_ok() " Greg Kurz 2020-10-03 1:58 ` Jason Wang 2020-10-03 1:58 ` Jason Wang 2020-10-03 1:58 ` Jason Wang 2020-10-03 8:38 ` Greg Kurz 2020-10-03 8:38 ` Greg Kurz 2020-10-01 12:46 ` [PATCH v2 0/2] vhost: Skip access checks on GIOVAs Michael S. Tsirkin 2020-10-01 12:46 ` Michael S. Tsirkin 2020-10-01 12:46 ` Michael S. Tsirkin
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=160139704424.162128.7839027287942194310.stgit@bahia.lan \ --to=groug@kaod.org \ --cc=david@gibson.dropbear.id.au \ --cc=jasowang@redhat.com \ --cc=kvm@vger.kernel.org \ --cc=laurent@vivier.eu \ --cc=mst@redhat.com \ --cc=netdev@vger.kernel.org \ --cc=qemu-devel@nongnu.org \ --cc=virtualization@lists.linux-foundation.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.