On Mon, 2022-07-11 at 23:27 +0000, Sean Christopherson wrote:
> Wait to mark the TSS as busy during LTR emulation until after all fault
> checks for the LTR have passed. Specifically, don't mark the TSS busy if
> the new TSS base is non-canonical.
Took me a while to notice it but I see the canonical check now, so the patch
makes sense, and so:
Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
Unrelated, but I do wonder why we use cmpxchg_emulated for setting the busy bit, while we use
write_segment_descriptor to set the accessed bit.
Best regards,
Maxim Levitsky
>
> Opportunistically drop the one-off !seg_desc.PRESENT check for TR as the
> only reason for the early check was to avoid marking a !PRESENT TSS as
> busy, i.e. the common !PRESENT is now done before setting the busy bit.
>
> Fixes: e37a75a13cda ("KVM: x86: Emulator ignores LDTR/TR extended base on LLDT/LTR")
> Reported-by: syzbot+760a73552f47a8cd0fd9@xxxxxxxxxxxxxxxxxxxxxxxxx
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> Cc: Hou Wenlong <houwenlong.hwl@xxxxxxxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
> arch/x86/kvm/emulate.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 39ea9138224c..09e4b67b881f 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -1699,16 +1699,6 @@ static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
> case VCPU_SREG_TR:
> if (seg_desc.s || (seg_desc.type != 1 && seg_desc.type != 9))
> goto exception;
> - if (!seg_desc.p) {
> - err_vec = NP_VECTOR;
> - goto exception;
> - }
> - old_desc = seg_desc;
> - seg_desc.type |= 2; /* busy */
> - ret = ctxt->ops->cmpxchg_emulated(ctxt, desc_addr, &old_desc, &seg_desc,
> - sizeof(seg_desc), &ctxt->exception);
> - if (ret != X86EMUL_CONTINUE)
> - return ret;
> break;
> case VCPU_SREG_LDTR:
> if (seg_desc.s || seg_desc.type != 2)
> @@ -1749,6 +1739,15 @@ static int __load_segment_descriptor(struct x86_emulate_ctxt *ctxt,
> ((u64)base3 << 32), ctxt))
> return emulate_gp(ctxt, 0);
> }
> +
> + if (seg == VCPU_SREG_TR) {
> + old_desc = seg_desc;
> + seg_desc.type |= 2; /* busy */
> + ret = ctxt->ops->cmpxchg_emulated(ctxt, desc_addr, &old_desc, &seg_desc,
> + sizeof(seg_desc), &ctxt->exception);
> + if (ret != X86EMUL_CONTINUE)
> + return ret;
> + }
> load:
> ctxt->ops->set_segment(ctxt, selector, &seg_desc, base3, seg);
> if (desc)
