linux-kernel - Re: [PATCH] KVM: x86: restore IP after all far jump failures

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] [day] [month] [year] [list]

Message-ID: <20161123162352.GB7487@potion>
Date:   Wed, 23 Nov 2016 17:23:56 +0100
From:   Radim Krčmář <rkrcmar@...hat.com>
To:     Nadav Amit <nadav.amit@...il.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        KVM list <kvm@...r.kernel.org>, stable@...r.kernel.org,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Steve Rutherford <srutherford@...gle.com>,
        Andrew Honig <ahonig@...gle.com>,
        Prasad Pandit <ppandit@...hat.com>
Subject: Re: [PATCH] KVM: x86: restore IP after all far jump failures

2016-11-22 15:18-0800, Nadav Amit:
>> On Nov 22, 2016, at 12:56 PM, Radim Krčmář <rkrcmar@...hat.com> wrote:
>> 
>> 2016-11-22 11:43-0800, Nadav Amit:
>>> I admit my wrongdoings, but I still think the fix should have been to
>>> remove the entire recovery logic and just return X86EMUL_UNHANDLEABLE if
>>> something goes wrong (exception). This will kill the misbehaving process
>>> but keep the VM running.
>> 
>> X86EMUL_UNHANDLEABLE will kill the whole VM (on QEMU, other userspaces
>> might handle the instruction and resume KVM).
> 
> I don’t think so. If CPL is not 0, handle_emulation_failure() will be called
> and will inject #UD.

This part of the emulator should only be used on Intels without
unrestricted guest, for real and unpaged protected mode.

It is unsafe as we don't make a clear distinction which instructions are
emulated for MMIO and which for old CPUs, but anyway, we should not be
emulating it only any OS that considers security.

(x86 can use pretty much any instruction for MMIO, but I think that KVM
 should let a userspace emulator handle those exotic OSes.)

>> The recovery path is in the spec, which means that nothing goes wrong.
>> I think we implement the spec quite well now, so keeping the #GP and CS
>> recovery is slightly better, although not safer.
>> 
>>> Otherwise, a malicious VM process, which can somehow control descriptors
>>> (LDT?) may modify the descriptor during the emulation and get the system
>>> to inconsistent state and prevent the VM-entry.
>> 
>> We restore the original CS -- malicious guest would get killed on a
>> failed VM entry anyway, so the difference is only in KVM internal error
>> code (assuming there are no other bugs).
>> 
>> Am I misunderstanding something?
> 
> Most likely you are right, but after doing one mistake, I don’t want
> to be accountable for another.
> 
> Note there is another problematic case in em_ret_far(). If an exception
> occurs when RIP is assigned, the RSP updates (of emulate_pop() ) are not 
> going to be reverted. Can it be used for anything malicious? I don’t know.

True, we are not emulating it right ... security is just side-effect.
Benefits of emulation are not worth the work so I'll prepare a patch
that just returns X86EMUL_UNHANDLEABLE.  It's not going to fix other
bugs with stack handling, but at least we won't be making it crazier.

Thanks.