linux-kernel - Re: [PATCH] mic: vop: Fix broken virtqueues

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] [day] [month] [year] [list]

Date:   Wed, 30 Jan 2019 17:27:23 +0100
From:   Vincent Whitchurch <vincent.whitchurch@...s.com>
To:     Sudeep Dutt <sudeep.dutt@...el.com>
Cc:     gregkh@...uxfoundation.org, arnd@...db.de,
        ashutosh.dixit@...el.com, linux-kernel@...r.kernel.org,
        virtualization@...ts.linux-foundation.org, tiwei.bie@...el.com,
        luto@...nel.org
Subject: Re: [PATCH] mic: vop: Fix broken virtqueues

On Wed, Jan 30, 2019 at 08:29:57AM -0800, Sudeep Dutt wrote:
> On Tue, 2019-01-29 at 11:22 +0100, Vincent Whitchurch wrote:
> > VOP is broken in mainline since commit 1ce9e6055fa0a9043 ("virtio_ring:
> > introduce packed ring support"); attempting to use the virtqueues leads
> > to various kernel crashes.  I'm testing it with my not-yet-merged
> > loopback patches, but even the in-tree MIC hardware cannot work.
> > 
> > The problem is not in the referenced commit per se, but is due to the
> > following hack in vop_find_vq() which depends on the layout of private
> > structures in other source files, which that commit happened to change:
> > 
> >   /*
> >    * To reassign the used ring here we are directly accessing
> >    * struct vring_virtqueue which is a private data structure
> >    * in virtio_ring.c. At the minimum, a BUILD_BUG_ON() in
> >    * vring_new_virtqueue() would ensure that
> >    *  (&vq->vring == (struct vring *) (&vq->vq + 1));
> >    */
> >   vr = (struct vring *)(vq + 1);
> >   vr->used = used;
> > 
> > Fix vop by using __vring_new_virtqueue() to create the needed vring
> > layout from the start, instead of attempting to patch in the used ring
> > later.  __vring_new_virtqueue() was added way back in commit
> > 2a2d1382fe9dcc ("virtio: Add improved queue allocation API") in order to
> > address mic's usecase, according to the commit message.
> > 
> 
> Thank you for fixing this up Vincent.
> 
> Reviewed-by: Sudeep Dutt <sudeep.dutt@...el.com>

Thanks, but I just noticed that the removal patch has the hack too
(without a comment), so that needs to be fixed.  I'll post a v2.

(The removal path also has an unrelated use-after-free, but that's a
 subject for a different patch.)