Re: [RFC PATCH 3/3] x86 emulator: Add segment limit checks and helper functions

[Avi Kivity <avi@redhat.com>]

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