> 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. > > 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. Nadav-- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html