From patchwork Thu Dec 24 06:00:58 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 69679 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter.kernel.org (8.14.3/8.14.2) with ESMTP id nBO63v8L022432 for ; Thu, 24 Dec 2009 06:03:57 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753564AbZLXGDy (ORCPT ); Thu, 24 Dec 2009 01:03:54 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753484AbZLXGDy (ORCPT ); Thu, 24 Dec 2009 01:03:54 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57615 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753161AbZLXGDx (ORCPT ); Thu, 24 Dec 2009 01:03:53 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id nBO63let021557 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 24 Dec 2009 01:03:47 -0500 Received: from redhat.com (vpn-6-219.tlv.redhat.com [10.35.6.219]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with SMTP id nBO63iEZ016206; Thu, 24 Dec 2009 01:03:45 -0500 Date: Thu, 24 Dec 2009 08:00:58 +0200 From: "Michael S. Tsirkin" To: "Michael S. Tsirkin" , rusty@rustcorp.com.au, Al Viro , kvm@vger.kernel.org, virtualization@lists.osdl.org Subject: [PATCH] vhost: access check thinko fixes Message-ID: <20091224060058.GA26101@redhat.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.19 (2009-01-05) X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 2b65d9b..c8c25db 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -230,7 +230,7 @@ static int log_access_ok(void __user *log_base, u64 addr, unsigned long sz) } /* Caller should have vq mutex and device mutex. */ -static int vq_memory_access_ok(struct vhost_virtqueue *vq, struct vhost_memory *mem, +static int vq_memory_access_ok(void __user *log_base, struct vhost_memory *mem, int log_all) { int i; @@ -242,7 +242,7 @@ static int vq_memory_access_ok(struct vhost_virtqueue *vq, struct vhost_memory * else if (!access_ok(VERIFY_WRITE, (void __user *)a, m->memory_size)) return 0; - else if (log_all && !log_access_ok(vq->log_base, + else if (log_all && !log_access_ok(log_base, m->guest_phys_addr, m->memory_size)) return 0; @@ -259,9 +259,10 @@ static int memory_access_ok(struct vhost_dev *d, struct vhost_memory *mem, 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); + /* If ring is inactive, will check when it's enabled. */ + if (d->vqs[i].private_data) + ok = vq_memory_access_ok(d->vqs[i].log_base, mem, + log_all); else ok = 1; mutex_unlock(&d->vqs[i].mutex); @@ -290,18 +291,25 @@ int vhost_log_access_ok(struct vhost_dev *dev) return memory_access_ok(dev, dev->memory, 1); } -/* Can we start vq? */ +/* Verify access for write logging. */ /* Caller should have vq mutex and device mutex */ -int vhost_vq_access_ok(struct vhost_virtqueue *vq) +static int vq_log_access_ok(struct vhost_virtqueue *vq, void __user *log_base) { - return vq_access_ok(vq->num, vq->desc, vq->avail, vq->used) && - vq_memory_access_ok(vq, vq->dev->memory, + return vq_memory_access_ok(log_base, vq->dev->memory, vhost_has_feature(vq->dev, VHOST_F_LOG_ALL)) && - (!vq->log_used || log_access_ok(vq->log_base, vq->log_addr, + (!vq->log_used || log_access_ok(log_base, vq->log_addr, sizeof *vq->used + vq->num * sizeof *vq->used->ring)); } +/* 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_log_access_ok(vq, vq->log_base); +} + static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m) { struct vhost_memory mem, *newmem, *oldmem; @@ -564,15 +572,14 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, unsigned long arg) } for (i = 0; i < d->nvqs; ++i) { struct vhost_virtqueue *vq; + void __user *base = (void __user *)(unsigned long)p; vq = d->vqs + i; mutex_lock(&vq->mutex); - /* Moving log base with an active backend? - * You don't want to do that. */ - if (vq->private_data && (vq->log_used || - vhost_has_feature(d, VHOST_F_LOG_ALL))) - r = -EBUSY; + /* If ring is inactive, will check when it's enabled. */ + if (vq->private_data && !vq_log_access_ok(vq, base)) + r = -EFAULT; else - vq->log_base = (void __user *)(unsigned long)p; + vq->log_base = base; mutex_unlock(&vq->mutex); } break;