netdev - Re: [PATCH][VHOST] fix race with guest on multi-buffer used buffer updates

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 24 May 2010 19:42:05 +0300
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	David Stevens <dlstevens@...ibm.com>
Cc:	kvm@...r.kernel.org, netdev@...r.kernel.org,
	netdev-owner@...r.kernel.org
Subject: Re: [PATCH][VHOST] fix race with guest on multi-buffer used buffer
	updates

On Mon, May 24, 2010 at 09:27:15AM -0700, David Stevens wrote:
> netdev-owner@...r.kernel.org wrote on 05/24/2010 09:13:51 AM:
> 
> > On Mon, May 24, 2010 at 08:52:40AM -0700, David Stevens wrote:
> > > "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
> > 
> > so this happens always - what does wrap-around refer to?
> 
>         The 2-part update only happens when a packet spans
> the end/beginning of the vring (the wrap). The framing error only
> happens if the guest sees the vring-wrapping packets
> before the second used-index write (the race).
>         So, the framing error doesn't happen always--it's
> pretty rare. But with the patch, it never happens.
> 
>                                                         +-DLS

I see. The logging is still bugg though I think.
--
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