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 for Android: free password hash cracker in your pocket

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

Message-ID: <87k1krh588.fsf@vitty.brq.redhat.com>
Date:   Mon, 03 Dec 2018 00:35:35 +0100
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Nadav Amit <nadav.amit@...il.com>
Cc:     kvm list <kvm@...r.kernel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Roman Kagan <rkagan@...tuozzo.com>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>, 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

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').

-- 
Vitaly