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
| ||
|
Date: Mon, 24 May 2010 08:52:40 -0700 From: David Stevens <dlstevens@...ibm.com> To: "Michael S. Tsirkin" <mst@...hat.com> Cc: kvm@...r.kernel.org, netdev@...r.kernel.org Subject: Re: [PATCH][VHOST] fix race with guest on multi-buffer used buffer updates "Michael S. Tsirkin" <mst@...hat.com> wrote on 05/24/2010 03:17:10 AM: > On Thu, May 20, 2010 at 09:58:06AM -0700, David L Stevens wrote: > > [for Michael Tsirkin's vhost development git tree] > > > > This patch fixes a race between guest and host when > > adding used buffers wraps the ring. Without it, guests > > can see partial packets before num_buffers is set in > > the vnet header. > > > > Signed-off-by: David L Stevens <dlstevens@...ibm.com> > > Could you please explain what the race is? Sure. The pre-patch code in the ring-wrap case does this: add part1 bufs update used index add part2 bufs update used index After we update the used index for part1, the part1 buffers are available to the guest. If the guest is consuming at that point, it can process the partial packet before the rest of the packet is there. In that case, num_buffers will be greater than the number of buffers available to the guest and it'll drop the packet with a framing error. I was seeing 2 or 3 framing errors every 100 million packets or so pre-patch, none post-patch. Actually, the second sentence is incorrect in the original description-- num_buffers is up to date when the guest sees it, but the used index is not. +-DLS > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > > index 7f2568d..74790ab 100644 > > --- a/drivers/vhost/vhost.c > > +++ b/drivers/vhost/vhost.c > > @@ -1065,14 +1065,6 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq, > > vq_err(vq, "Failed to write used"); > > return -EFAULT; > > } > > - /* Make sure buffer is written before we update index. */ > > - smp_wmb(); > > - if (put_user(vq->last_used_idx + count, &vq->used->idx)) { > > - vq_err(vq, "Failed to increment used idx"); > > - return -EFAULT; > > - } > > - if (unlikely(vq->log_used)) > > - vhost_log_used(vq, used); > > vq->last_used_idx += count; > > return 0; > > } > > @@ -1093,7 +1085,17 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, > struct vring_used_elem *heads, > > heads += n; > > count -= n; > > } > > - return __vhost_add_used_n(vq, heads, count); > > + r = __vhost_add_used_n(vq, heads, count); > > + > > + /* Make sure buffer is written before we update index. */ > > + smp_wmb(); > > + if (put_user(vq->last_used_idx, &vq->used->idx)) { > > + vq_err(vq, "Failed to increment used idx"); > > + return -EFAULT; > > + } > > + if (unlikely(vq->log_used)) > > + vhost_log_used(vq, vq->used->ring + start); > > + return r; > > } > > > > /* This actually signals the guest, using eventfd. */ > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists