linux-kernel - Re: [PATCH v2] x86/hyper-v: Mark TLFS structures packed

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]

Message-ID: <20181203071520.GA17278@rkaganb.sw.ru>
Date:   Mon, 3 Dec 2018 07:15:25 +0000
From:   Roman Kagan <rkagan@...tuozzo.com>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>
CC:     Nadav Amit <nadav.amit@...il.com>, kvm list <kvm@...r.kernel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "Michael Kelley (EOSG)" <Michael.H.Kelley@...rosoft.com>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v2] x86/hyper-v: Mark TLFS structures packed

On Mon, Dec 03, 2018 at 12:35:35AM +0100, Vitaly Kuznetsov wrote:
> Nadav Amit <nadav.amit@...il.com> writes:
> 
> [skip]
> 
> >
> > Having said that, something else is sort of strange in the TLFS definitions,
> > I think (I really know little about this whole protocol). Look at the
> > following definitions from hyperv-tlfs.h:
> >
> >> struct hv_vpset {
> >>         u64 format;
> >>         u64 valid_bank_mask;
> >>         u64 bank_contents[];
> >> };
> >> 
> >> struct hv_tlb_flush_ex {
> >>         u64 address_space;
> >>         u64 flags;
> >>         struct hv_vpset hv_vp_set;
> >>         u64 gva_list[];
> >> };
> >
> > It seems you have two flexible array members at the end of hv_tlb_flush_ex.
> > This causes bank_contents[x] and gva_list[x] to overlap. So unless they have
> > the same meaning, this asks for trouble IMHO.
> >
> 
> This is weird but intentional :-) We're just following Hyper-V spec
> here.
> 
> E.g. HvFlushVirtualAddressListEx hypercall has the following input ABI:
> 
> [Fixed len head][[Fixed len VP set spec]Var len VP set][Var len addr List]
> 
> "Fixed len VP set spec" defines the true length of "Var len VP set" and
> "Address List" starts right after that. The length of the whole
> structure is also known.
> 
> So bank_contents[] and gva_list[] do overlap (and have different
> meaning). We take special precautions when forming the structure
> (e.g. fill_gva_list() takes 'offset').

This basically means that the argument of this hypercall can't be
represented by a C struct.  gva_list just can't be used.  So I'd rather
remove it from the struct (but leave a comment to that end perhaps), and
construct the message in place (as is done now anyway).

Roman.