2022-07-11 23:43:49

by Sean Christopherson

Subject: [PATCH 2/3] KVM: x86: Set error code to segment selector on LLDT/LTR non-canonical #GP

When injecting a #GP on LLDT/LTR due to a non-canonical LDT/TSS base, set
the error code to the selector. Intel SDM's says nothing about the #GP,
but AMD's APM explicitly states that both LLDT and LTR set the error code
to the selector, not zero.

Note, a non-canonical memory operand on LLDT/LTR does generate a #GP(0),
but the KVM code in question is specific to the base from the descriptor.

Fixes: e37a75a13cda ("KVM: x86: Emulator ignores LDTR/TR extended base on LLDT/LTR")
Cc: [email protected]
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/emulate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 09e4b67b881f..bd9e9c5627d0 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1736,8 +1736,8 @@ static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
if (ret != X86EMUL_CONTINUE)
return ret;
if (emul_is_noncanonical_address(get_desc_base(&seg_desc) |
- ((u64)base3 << 32), ctxt))
- return emulate_gp(ctxt, 0);
+ ((u64)base3 << 32), ctxt))
+ return emulate_gp(ctxt, err_code);
}

if (seg == VCPU_SREG_TR) {
--
2.37.0.144.g8ac04bfd2-goog

2022-07-12 14:10:51

by Maxim Levitsky

Subject: Re: [PATCH 2/3] KVM: x86: Set error code to segment selector on LLDT/LTR non-canonical #GP

On Mon, 2022-07-11 at 23:27 +0000, Sean Christopherson wrote:
> When injecting a #GP on LLDT/LTR due to a non-canonical LDT/TSS base, set
> the error code to the selector.  Intel SDM's says nothing about the #GP,
> but AMD's APM explicitly states that both LLDT and LTR set the error code
> to the selector, not zero.
>
> Note, a non-canonical memory operand on LLDT/LTR does generate a #GP(0),
> but the KVM code in question is specific to the base from the descriptor.
>
> Fixes: e37a75a13cda ("KVM: x86: Emulator ignores LDTR/TR extended base on LLDT/LTR")
> Cc: [email protected]
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
>  arch/x86/kvm/emulate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 09e4b67b881f..bd9e9c5627d0 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -1736,8 +1736,8 @@ static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
>                 if (ret != X86EMUL_CONTINUE)
>                         return ret;
>                 if (emul_is_noncanonical_address(get_desc_base(&seg_desc) |
> -                               ((u64)base3 << 32), ctxt))
> -                       return emulate_gp(ctxt, 0);
> +                                                ((u64)base3 << 32), ctxt))
> +                       return emulate_gp(ctxt, err_code);
>         }
>  
>         if (seg == VCPU_SREG_TR) {

I guess this is the quote from AMD's manual (might we worth to add to the source?)


"The 64-bit system-segment base address must be in canonical form. Otherwise, a general-protection
exception occurs with a selector error-code, #GP(selector), when the system segment is loaded.
System-segment limit values are checked by the processor in both 64-bit and compatibility modes,
under the control of the granularity (G) bit."

Reviewed-by: Maxim Levitsky <[email protected]>

Best regards,
Maxim Levitsky

2022-07-12 18:03:36

by Sean Christopherson

Subject: Re: [PATCH 2/3] KVM: x86: Set error code to segment selector on LLDT/LTR non-canonical #GP

On Tue, Jul 12, 2022, Maxim Levitsky wrote:
> On Mon, 2022-07-11 at 23:27 +0000, Sean Christopherson wrote:
> > When injecting a #GP on LLDT/LTR due to a non-canonical LDT/TSS base, set
> > the error code to the selector.? Intel SDM's says nothing about the #GP,
> > but AMD's APM explicitly states that both LLDT and LTR set the error code
> > to the selector, not zero.
> >
> > Note, a non-canonical memory operand on LLDT/LTR does generate a #GP(0),
> > but the KVM code in question is specific to the base from the descriptor.
> >
> > Fixes: e37a75a13cda ("KVM: x86: Emulator ignores LDTR/TR extended base on LLDT/LTR")
> > Cc: [email protected]
> > Signed-off-by: Sean Christopherson <[email protected]>
> > ---
> > ?arch/x86/kvm/emulate.c | 4 ++--
> > ?1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> > index 09e4b67b881f..bd9e9c5627d0 100644
> > --- a/arch/x86/kvm/emulate.c
> > +++ b/arch/x86/kvm/emulate.c
> > @@ -1736,8 +1736,8 @@ static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
> > ????????????????if (ret != X86EMUL_CONTINUE)
> > ????????????????????????return ret;
> > ????????????????if (emul_is_noncanonical_address(get_desc_base(&seg_desc) |
> > -???????????????????????????????((u64)base3 << 32), ctxt))
> > -???????????????????????return emulate_gp(ctxt, 0);
> > +??????????????????????????????????????????????? ((u64)base3 << 32), ctxt))
> > +???????????????????????return emulate_gp(ctxt, err_code);
> > ????????}
> > ?
> > ????????if (seg == VCPU_SREG_TR) {
>
> I guess this is the quote from AMD's manual (might we worth to add to the source?)

Eh, probably not worth it. Anyone working on segmentation emulation is already
up to their eyeballs in the SDM/APM.