On biarch kernels, it is not safe to do copy from/to user, even with use_mm,
unless we checked the address range with access_ok previously.  Implement these
checks to enforce safe memory accesses.

Reported-by: Al Viro <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
---
 drivers/vhost/net.c   |   17 ++++++-
 drivers/vhost/vhost.c |  111 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/vhost/vhost.h |    2 +
 3 files changed, 127 insertions(+), 3 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 1b509a0..1aacd8c 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -493,6 +493,12 @@ static long vhost_net_set_backend(struct vhost_net *n, 
unsigned index, int fd)
        }
        vq = n->vqs + index;
        mutex_lock(&vq->mutex);
+
+       /* Verify that ring has been setup correctly. */
+       if (!vhost_vq_access_ok(vq)) {
+               r = -EFAULT;
+               goto err;
+       }
        sock = get_socket(fd);
        if (IS_ERR(sock)) {
                r = PTR_ERR(sock);
@@ -539,12 +545,17 @@ done:
        return err;
 }
 
-static void vhost_net_set_features(struct vhost_net *n, u64 features)
+static int vhost_net_set_features(struct vhost_net *n, u64 features)
 {
        size_t hdr_size = features & (1 << VHOST_NET_F_VIRTIO_NET_HDR) ?
                sizeof(struct virtio_net_hdr) : 0;
        int i;
        mutex_lock(&n->dev.mutex);
+       if ((features & (1 << VHOST_F_LOG_ALL)) &&
+           !vhost_log_access_ok(&n->dev)) {
+               mutex_unlock(&n->dev.mutex);
+               return -EFAULT;
+       }
        n->dev.acked_features = features;
        smp_wmb();
        for (i = 0; i < VHOST_NET_VQ_MAX; ++i) {
@@ -554,6 +565,7 @@ static void vhost_net_set_features(struct vhost_net *n, u64 
features)
        }
        vhost_net_flush(n);
        mutex_unlock(&n->dev.mutex);
+       return 0;
 }
 
 static long vhost_net_ioctl(struct file *f, unsigned int ioctl,
@@ -580,8 +592,7 @@ static long vhost_net_ioctl(struct file *f, unsigned int 
ioctl,
                        return r;
                if (features & ~VHOST_FEATURES)
                        return -EOPNOTSUPP;
-               vhost_net_set_features(n, features);
-               return 0;
+               return vhost_net_set_features(n, features);
        case VHOST_RESET_OWNER:
                return vhost_net_reset_owner(n);
        default:
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 29f1675..33e06bf 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -224,6 +224,91 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
        dev->mm = NULL;
 }
 
+static int log_access_ok(void __user *log_base, u64 addr, unsigned long sz)
+{
+       u64 a = addr / VHOST_PAGE_SIZE / 8;
+       /* Make sure 64 bit math will not overflow. */
+       if (a > ULONG_MAX - (unsigned long)log_base ||
+           a + (unsigned long)log_base > ULONG_MAX)
+               return -EFAULT;
+
+       return access_ok(VERIFY_WRITE, log_base + a,
+                        (sz + VHOST_PAGE_SIZE * 8 - 1) / VHOST_PAGE_SIZE / 8);
+}
+
+/* Caller should have vq mutex and device mutex. */
+static int vq_memory_access_ok(struct vhost_virtqueue *vq, struct vhost_memory 
*mem,
+                              int log_all)
+{
+       int i;
+       for (i = 0; i < mem->nregions; ++i) {
+               struct vhost_memory_region *m = mem->regions + i;
+               unsigned long a = m->userspace_addr;
+               if (m->memory_size > ULONG_MAX)
+                       return 0;
+               else if (!access_ok(VERIFY_WRITE, (void __user *)a,
+                                   m->memory_size))
+                       return 0;
+               else if (log_all && !log_access_ok(vq->log_base,
+                                                  m->guest_phys_addr,
+                                                  m->memory_size))
+                       return 0;
+       }
+       return 1;
+}
+
+/* Can we switch to this memory table? */
+/* Caller should have device mutex but not vq mutex */
+static int memory_access_ok(struct vhost_dev *d, struct vhost_memory *mem,
+                           int log_all)
+{
+       int i;
+       for (i = 0; i < d->nvqs; ++i) {
+               int ok;
+               mutex_lock(&d->vqs[i].mutex);
+               /* If ring is not running, will check when it's enabled. */
+               if (&d->vqs[i].private_data)
+                       ok = vq_memory_access_ok(&d->vqs[i], mem, log_all);
+               else
+                       ok = 1;
+               mutex_unlock(&d->vqs[i].mutex);
+               if (!ok)
+                       return 0;
+       }
+       return 1;
+}
+
+static int vq_access_ok(unsigned int num,
+                       struct vring_desc __user *desc,
+                       struct vring_avail __user *avail,
+                       struct vring_used __user *used)
+{
+       return access_ok(VERIFY_READ, desc, num * sizeof *desc) &&
+              access_ok(VERIFY_READ, avail,
+                        sizeof *avail + num * sizeof *avail->ring) &&
+              access_ok(VERIFY_WRITE, used,
+                       sizeof *used + num * sizeof *used->ring);
+}
+
+/* Can we log writes? */
+/* Caller should have device mutex but not vq mutex */
+int vhost_log_access_ok(struct vhost_dev *dev)
+{
+       return memory_access_ok(dev, dev->memory, 1);
+}
+
+/* Can we start vq? */
+/* Caller should have vq mutex and device mutex */
+int vhost_vq_access_ok(struct vhost_virtqueue *vq)
+{
+       return vq_access_ok(vq->num, vq->desc, vq->avail, vq->used) &&
+               vq_memory_access_ok(vq, vq->dev->memory,
+                           vhost_has_feature(vq->dev, VHOST_F_LOG_ALL)) &&
+               (!vq->log_used || log_access_ok(vq->log_base, vq->log_addr,
+                                       sizeof *vq->used +
+                                       vq->num * sizeof *vq->used->ring));
+}
+
 static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user 
*m)
 {
        struct vhost_memory mem, *newmem, *oldmem;
@@ -247,6 +332,9 @@ static long vhost_set_memory(struct vhost_dev *d, struct 
vhost_memory __user *m)
                kfree(newmem);
                return r;
        }
+
+       if (!memory_access_ok(d, newmem, vhost_has_feature(d, VHOST_F_LOG_ALL)))
+               return -EFAULT;
        oldmem = d->memory;
        rcu_assign_pointer(d->memory, newmem);
        synchronize_rcu();
@@ -348,6 +436,29 @@ static long vhost_set_vring(struct vhost_dev *d, int 
ioctl, void __user *argp)
                        r = -EINVAL;
                        break;
                }
+
+               /* We only verify access here if backend is configured.
+                * If it is not, we don't as size might not have been setup.
+                * We will verify when backend is configured. */
+               if (vq->private_data) {
+                       if (!vq_access_ok(vq->num,
+                               (void __user *)(unsigned long)a.desc_user_addr,
+                               (void __user *)(unsigned long)a.avail_user_addr,
+                               (void __user *)(unsigned 
long)a.used_user_addr)) {
+                               r = -EINVAL;
+                               break;
+                       }
+
+                       /* 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,
+                                          sizeof *vq->used +
+                                          vq->num * sizeof *vq->used->ring)) {
+                               r = -EINVAL;
+                               break;
+                       }
+               }
+
                r = init_used(vq, (struct vring_used __user *)(unsigned long)
                              a.used_user_addr);
                if (r)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index d1f0453..44591ba 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -117,6 +117,8 @@ long vhost_dev_check_owner(struct vhost_dev *);
 long vhost_dev_reset_owner(struct vhost_dev *);
 void vhost_dev_cleanup(struct vhost_dev *);
 long vhost_dev_ioctl(struct vhost_dev *, unsigned int ioctl, unsigned long 
arg);
+int vhost_vq_access_ok(struct vhost_virtqueue *vq);
+int vhost_log_access_ok(struct vhost_dev *);
 
 unsigned vhost_get_vq_desc(struct vhost_dev *, struct vhost_virtqueue *,
                           struct iovec iov[], unsigned int iov_count,
-- 
1.6.6.rc1.43.gf55cc

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization

Reply via email to