On 09/30/2014 07:49 PM, Nadav Amit wrote:
> GP and SS exceptions deliver as error-code the segment selector if the
> exception occurred when the segment is loaded.  However, if the exception
> occurs during the memory access itself, due to limit violations, the 
> error-code
> should be zero.  Fix it.
> 
> Signed-off-by: Nadav Amit <[email protected]>
> ---
>  arch/x86/kvm/emulate.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index a46207a..13a1c76 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -621,7 +621,7 @@ static int __linearize(struct x86_emulate_ctxt *ctxt,
>       bool usable;
>       ulong la;
>       u32 lim;
> -     u16 sel;
> +     u16 sel, error_code = 0;
>       unsigned cpl;
>  
>       la = seg_base(ctxt, addr.seg) + addr.ea;
> @@ -634,14 +634,14 @@ static int __linearize(struct x86_emulate_ctxt *ctxt,
>               usable = ctxt->ops->get_segment(ctxt, &sel, &desc, NULL,
>                                               addr.seg);
>               if (!usable)
> -                     goto bad;
> +                     goto bad_sel;

This can only happen because of a NULL selector, which means the error
code is zero anyway.

>               /* code segment in protected mode or read-only data segment */
>               if ((((ctxt->mode != X86EMUL_MODE_REAL) && (desc.type & 8))
>                                       || !(desc.type & 2)) && write)
> -                     goto bad;
> +                     goto bad_sel;

This is not "detected while loading a segment descriptor", so the error
code should be zero.

>               /* unreadable code segment */
>               if (!fetch && (desc.type & 8) && !(desc.type & 2))
> -                     goto bad;
> +                     goto bad_sel;

Same here.

>               lim = desc_limit_scaled(&desc);
>               if ((ctxt->mode == X86EMUL_MODE_REAL) && !fetch &&
>                   (ctxt->d & NoBigReal)) {
> @@ -664,15 +664,15 @@ static int __linearize(struct x86_emulate_ctxt *ctxt,
>               if (!(desc.type & 8)) {
>                       /* data segment */
>                       if (cpl > desc.dpl)
> -                             goto bad;
> +                             goto bad_sel;
>               } else if ((desc.type & 8) && !(desc.type & 4)) {
>                       /* nonconforming code segment */
>                       if (cpl != desc.dpl)
> -                             goto bad;
> +                             goto bad_sel;
>               } else if ((desc.type & 8) && (desc.type & 4)) {
>                       /* conforming code segment */
>                       if (cpl < desc.dpl)
> -                             goto bad;
> +                             goto bad_sel;

These three should be deleted, as you pointed out in patch 5.

So I've dropped this patch, and posted a simpler alternative that just
uses error code 0 in __linearize.  Can you look at it?

Paolo

>               }
>               break;
>       }
> @@ -682,11 +682,13 @@ static int __linearize(struct x86_emulate_ctxt *ctxt,
>               return emulate_gp(ctxt, 0);
>       *linear = la;
>       return X86EMUL_CONTINUE;
> +bad_sel:
> +     error_code = sel;
>  bad:
>       if (addr.seg == VCPU_SREG_SS)
> -             return emulate_ss(ctxt, sel);
> +             return emulate_ss(ctxt, error_code);
>       else
> -             return emulate_gp(ctxt, sel);
> +             return emulate_gp(ctxt, error_code);
>  }
>  
>  static int linearize(struct x86_emulate_ctxt *ctxt,
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to