| 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: <20091220171625.GC31713@redhat.com>
Date: Sun, 20 Dec 2009 19:16:25 +0200
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Rusty Russell <rusty@...tcorp.com.au>,
virtualization@...ts.linux-foundation.org,
linux-kernel@...r.kernel.org
Cc: Al Viro <viro@...iv.linux.org.uk>
Subject: [PATCH 2/3] vhost: add access_ok checks
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 <viro@...iv.linux.org.uk>
Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists