netdev - Re: [PATCH v4] vhost: disable for OABI

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, 27 Apr 2020 08:45:22 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     "Michael S. Tsirkin" <mst@...hat.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Ard Biesheuvel <ardb@...nel.org>,
        Richard Earnshaw <Richard.Earnshaw@....com>,
        Sudeep Dutt <sudeep.dutt@...el.com>,
        Ashutosh Dixit <ashutosh.dixit@...el.com>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jason Wang <jasowang@...hat.com>,
        netdev <netdev@...r.kernel.org>,
        virtualization@...ts.linux-foundation.org,
        KVM list <kvm@...r.kernel.org>,
        Christoph Hellwig <hch@...radead.org>
Subject: Re: [PATCH v4] vhost: disable for OABI

Hi Michael,

Thanks for your patch!

On Mon, Apr 20, 2020 at 5:13 PM Michael S. Tsirkin <mst@...hat.com> wrote:
> vhost is currently broken on the some ARM configs.
>
> The reason is that the ring element addresses are passed between
> components with different alignments assumptions. Thus, if
> guest selects a pointer and host then gets and dereferences
> it, then alignment assumed by the host's compiler might be
> greater than the actual alignment of the pointer.
> compiler on the host from assuming pointer is aligned.
>
> This actually triggers on ARM with -mabi=apcs-gnu - which is a
> deprecated configuration. With this OABI, compiler assumes that
> all structures are 4 byte aligned - which is stronger than
> virtio guarantees for available and used rings, which are
> merely 2 bytes. Thus a guest without -mabi=apcs-gnu running
> on top of host with -mabi=apcs-gnu will be broken.
>
> The correct fix is to force alignment of structures - however
> that is an intrusive fix that's best deferred until the next release.
>
> We didn't previously support such ancient systems at all - this surfaced
> after vdpa support prompted removing dependency of vhost on
> VIRTULIZATION. So for now, let's just add something along the lines of
>
>         depends on !ARM || AEABI
>
> to the virtio Kconfig declaration, and add a comment that it has to do
> with struct member alignment.
>
> Note: we can't make VHOST and VHOST_RING themselves have
> a dependency since these are selected. Add a new symbol for that.

Adding the dependencies to VHOST and VHOST_RING themselves is indeed not
sufficient.  But IMHO you should still add VHOST_DPN dependencies t
 these two symbols, so any driver selecting them without fulfilling the
VHOST_DPN dependency will trigger a Kconfig warning.  Else the
issue will be ignored silently.

> We should be able to drop this dependency down the road.
>
> Fixes: 20c384f1ea1a0bc7 ("vhost: refine vhost and vringh kconfig")
> Suggested-by: Ard Biesheuvel <ardb@...nel.org>
> Suggested-by: Richard Earnshaw <Richard.Earnshaw@....com>
> Signed-off-by: Michael S. Tsirkin <mst@...hat.com>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds