2016-11-22 15:18-0800, Nadav Amit: >> On Nov 22, 2016, at 12:56 PM, Radim Krčmář <rkrcmar@xxxxxxxxxx> 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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
