linux-kernel - Re: [PATCH] KVM: VMX: drop I/O permission bitmaps

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: <CALMp9eS=oC8UiCoj_+V2N6=i5ovwy5WFg9gcFj-PaATYXs+kqQ@mail.gmail.com>
Date:   Mon, 11 Dec 2017 10:08:29 -0800
From:   Jim Mattson <jmattson@...gle.com>
To:     Quan Xu <quan.xu0@...il.com>
Cc:     kvm list <kvm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        David Hildenbrand <david@...hat.com>
Subject: Re: [PATCH] KVM: VMX: drop I/O permission bitmaps

Removing these two lines from the initialization of
field_to_offset_table[] means that vmcs_field_to_offset() will return
-ENOENT for IO_BITMAP_A or IO_BITMAP_B. Hence, handle_vmread and
handle_vmwrite will incorrectly report these fields as unsupported
VMCS components if an L1 hypervisor tries to access them.

On Sun, Dec 10, 2017 at 9:37 PM, Quan Xu <quan.xu0@...il.com> wrote:
>
>
> On 2017/12/09 01:31, Jim Mattson wrote:
>>
>> On Fri, Dec 8, 2017 at 2:22 AM, Quan Xu <quan.xu0@...il.com> wrote:
>>>
>>> From: Quan Xu <quan.xu0@...il.com>
>>>
>>> Since KVM removes the only I/O port 0x80 bypass on Intel hosts,
>>> clear CPU_BASED_USE_IO_BITMAPS and set CPU_BASED_UNCOND_IO_EXITING
>>> bit. Then these I/O permission bitmaps are not used at all, so
>>> drop I/O permission bitmaps.
>>>
>>> Signed-off-by: Jim Mattson <jmattson@...gle.com>
>>> Signed-off-by: Radim Krčmář <rkrcmar@...hat.com>
>>> Signed-off-by: Quan Xu <quan.xu0@...il.com>
>>> ---
>>>   arch/x86/kvm/vmx.c |   17 +----------------
>>>   1 files changed, 1 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index 2fd9a8c..3e4f760 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -771,8 +771,6 @@ enum segment_cache_field {
>>>          FIELD(HOST_FS_SELECTOR, host_fs_selector),
>>>          FIELD(HOST_GS_SELECTOR, host_gs_selector),
>>>          FIELD(HOST_TR_SELECTOR, host_tr_selector),
>>> -       FIELD64(IO_BITMAP_A, io_bitmap_a),
>>> -       FIELD64(IO_BITMAP_B, io_bitmap_b),
>>
>> These two lines should stay.
>
> Jim, could you explain why these two lines should stay?
>
>
> IIUC, the main concern is from  nested virtualization, which still uses
> io_bitmap_a/io_bitmap_b..
> if so, we really need to further clean up these code, as
>
> CPU_BASED_USE_IO_BITMAPS is clear, and CPU_BASED_UNCOND_IO_EXITING is set
> for both L0/L2. after new patches which I mentioned
> in this thread.
>
> right?
>
> Alibaba Cloud
> Quan
>
>
>