linux-kernel - Re: [PATCH v2 6/7] KVM: VMX: Check Intel PT related CPUID leaves

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]

Date:   Mon, 18 Oct 2021 21:56:12 +0800
From:   Xiaoyao Li <xiaoyao.li@...el.com>
To:     Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 6/7] KVM: VMX: Check Intel PT related CPUID leaves

On 10/18/2021 8:47 PM, Paolo Bonzini wrote:
> On 10/09/21 03:59, Xiaoyao Li wrote:
>>>
>>> Ugh, looking at the rest of the code, even this isn't sufficient
>>> because pt_desc.guest.addr_{a,b} are hardcoded at 4 entries, i.e.
>>> running KVM on hardware with >4 entries will lead to buffer
>>> overflows.
>>
>> it's hardcoded to 4 because there is a note of "no processors support
>>  more than 4 address ranges" in SDM vol.3 Chapter 31.3.1, table
>> 31-11
> 
> True, but I agree with Sean that it's not pretty.

Yes. We can add the check to validate the hardware is not bigger than 4 
for future processors. Intel folks are supposed to send new patches 
before silicon with more than 4 PT_ADDR_RANGE delivered.

>>> One option would be to bump that to the theoretical max of 15,
>>> which doesn't seem too horrible, especially if pt_desc as a whole
>>> is allocated on-demand, which it probably should be since it isn't
>>> exactly tiny (nor ubiquitous)
>>>
>>> A different option would be to let userspace define whatever it
>>> wants for guest CPUID, and instead cap nr_addr_ranges at
>>> min(host.cpuid, guest.cpuid, RTIT_ADDR_RANGE).
> 
> This is the safest option.

My concern was that change userspace's input silently is not good. If 
you prefer this, we certainly need to extend the userspace to query what 
value is finally accepted and set by KVM.

> Paolo
>