From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [RFC PATCH 3/3] x86 emulator: Add segment limit checks and helper functions Date: Thu, 08 Jul 2010 11:01:49 +0300 Message-ID: <4C3585ED.3010309@redhat.com> References: <1278537839-20144-1-git-send-email-m.gamal005@gmail.com> <1278537839-20144-4-git-send-email-m.gamal005@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org To: Mohammed Gamal Return-path: Received: from mx1.redhat.com ([209.132.183.28]:26155 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753252Ab0GHIBw (ORCPT ); Thu, 8 Jul 2010 04:01:52 -0400 In-Reply-To: <1278537839-20144-4-git-send-email-m.gamal005@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On 07/08/2010 12:23 AM, Mohammed Gamal wrote: > This patch adds segment limit checks to the x86 emulator, in addition to some > helper functions and changes to the return values of emulate_push to accomodate > the new checks. > > > > +static u32 seg_limit(struct x86_emulate_ctxt *ctxt, > + struct x86_emulate_ops *ops, int seg) > +{ > + if (ctxt->mode == X86EMUL_MODE_PROT64&& seg< VCPU_SREG_FS) > Why the check on VCPU_SREG_FS? There are no limits in long mode (well there's some AMD thing that does allow them). > + return 0; > better to return -1ULL, that indicates no practical limit. > + > + return ops->get_cached_segment_limit(seg, ctxt->vcpu); > +} > + > > static void emulate_exception(struct x86_emulate_ctxt *ctxt, int vec, > u32 error, bool valid) > { > @@ -718,6 +745,11 @@ static int do_insn_fetch(struct x86_emulate_ctxt *ctxt, > { > int rc; > > + if (eip + size> cs_base(ctxt, ops) + cs_limit(ctxt, ops)) { > This can wrap around, for example if cs.base=0xf0000000, cs.limit=0x2000000. Comparing eip - cs_base + size < cs_limit works around that. > @@ -1202,6 +1234,10 @@ done_prefixes: > c->src.ptr = (unsigned long *) > register_address(c, seg_override_base(ctxt, ops, c), > c->regs[VCPU_REGS_RSI]); > + if (c->src.ptr> (unsigned long *) (es_base(ctxt, ops) + es_limit(ctxt, ops))) { > + emulate_gp(ctxt, 0); > + return X86EMUL_PROPAGATE_FAULT; > Need to take into account the size fetched from src.ptr. For data segments, there are expand-down segments which modify the check. See SDM 5.3, "Limit Checking". -- error compiling committee.c: too many arguments to function