From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Yang, Sheng" Subject: Re: [PATCH] KVM: emulator: Only allow VMCALL/VMMCALL trapped by #UD Date: Mon, 7 Jan 2008 10:21:11 +0800 Message-ID: <200801071021.12038.sheng.yang@intel.com> References: <200801040936.08670.sheng.yang@intel.com> <478093F0.6060003@qumranet.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, Eric Liu To: Avi Kivity Return-path: In-Reply-To: <478093F0.6060003-atKUWr5tajBWk0Htik3J/w@public.gmane.org> Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org On Sunday 06 January 2008 16:40:16 Avi Kivity wrote: > Yang, Sheng wrote: > > From 9743b5299bae1779c2b893cbeb86122bcccb9b2d Mon Sep 17 00:00:00 2001 > > From: Sheng Yang > > Date: Wed, 2 Jan 2008 14:49:22 +0800 > > Subject: [PATCH] KVM: emulator: Only allow VMCALL/VMMCALL trapped by #UD > > > > When executing a test program called "crashme", we found the KVM guest > > cannot survived more than ten seconds, then encounterd kernel panic. The > > basic concept of "crashme" is graduating random assembly code and trying > > to execute them in a fork process. > > > > After some fix on emulator valid judgment, we found it's hard to get the > > current emulator handle the invalid instructions correctly, for the #UD > > trap for hypercall patching caused troubles. The problem is, if the > > opcode itself was OK, but combination of opcode and modrm_reg was > > invalid, and one operand of the opcode was memory(SrcMem or DstMem), > > emulator would fetched the memory operand first rather than judged the > > validity, and may encounter error there. For example, ".byte 0xfe, 0x34, > > 0xcd" got this trouble. > > > > In the patch, we simply check that if the invalid opcode isn't > > vmcall/vmmcall, then return from emulate_instruction() and inject a #UD > > to guest. With the patch, the guest had been run for more than 12 hours. > > Applied, thanks. As Anthony says, good catch indeed. > > I have a vague plan for improving decode; basically extend the decode > tables to add group decoding. We add a bit to opcode_table and > twobyte_table that is set for all instructions which need group > decoding. When the bit is set, the rest of the value in opcode_table is > interpreted as an index (together with modrm_reg) into a new > group_table, so we can have different decoding for such instructions. I also have tried to propose a table for Grp opcode, but can't find a easy way. Using the rest of the value in opcode_table is a good idea, but I'm afraid the same value for different group exists, e.g. 0x82(Grp1) and 0xc0 (Grp2) got the same value as: ByteOp | DstMem | SrcImm | ModRM. If we add more factors to this, it would become unclear and more tricky, the table also may become larger... Currently, if we want to using group_table, I think the straightforward way is better: another big "switch"... The only exception is 1a, and we may use 0 instead of it. -- Thanks Yang, Sheng ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/