Re: [PATCH] x86/HVM: only kill guest when unknown VM exit occurred in guest kernel mode

[Andrew Cooper <andrew.cooper3@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2840 bytes --]

On 30/10/14 14:43, Jan Beulich wrote:
> A recent KVM change by Nadav Amit <namit@cs.technion.ac.il> pointed out
> that unconditional VM exits (like VMX'es ones for the INVEPT, INVVPID,
> and XSETBV instructions) may result from guest user mode activity (in
> the example cases, e.g. prior to a privilege level check being done).
> Consequently convert the unconditional domain_crash() to a conditional
> one (when guest is in kernel mode) with the alternative of injecting
> #UD (when in user mode).
>
> This is meant to be a precaution against in-guest security issues
> introduced when any such VM exit becomes possible (on newer hardware)
> without the hypervisor immediately being aware of it. There are no such
> unhandled VM exits currently (and hence this is not an active security
> issue), but old (no longer security maintained) versions exhibit issues
> in the cases given as examples above.
>
> Suggested-by: Tim Deegan <tim@xen.org>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

The gdprintk() in vmx.c is not true for some entries via the
exit_and_crash label, but it is probably worth deferring fixing it to a
separate patch.

>
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2680,7 +2680,11 @@ void svm_vmexit_handler(struct cpu_user_
>                   "exitinfo1 = %#"PRIx64", exitinfo2 = %#"PRIx64"\n",
>                   exit_reason, 
>                   (u64)vmcb->exitinfo1, (u64)vmcb->exitinfo2);
> -        domain_crash(v->domain);
> +        if ( vmcb_get_cpl(vmcb) )
> +            hvm_inject_hw_exception(TRAP_invalid_op,
> +                                    HVM_DELIVER_NO_ERROR_CODE);
> +        else
> +            domain_crash(v->domain);
>          break;
>      }
>  
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3157,8 +3157,19 @@ void vmx_vmexit_handler(struct cpu_user_
>      /* fall through */
>      default:
>      exit_and_crash:
> -        gdprintk(XENLOG_ERR, "Bad vmexit (reason %#lx)\n", exit_reason);
> -        domain_crash(v->domain);
> +        {
> +            struct segment_register ss;
> +
> +            gdprintk(XENLOG_WARNING, "Bad vmexit (reason %#lx)\n",
> +                     exit_reason);
> +
> +            vmx_get_segment_register(v, x86_seg_ss, &ss);
> +            if ( ss.attr.fields.dpl )
> +                hvm_inject_hw_exception(TRAP_invalid_op,
> +                                        HVM_DELIVER_NO_ERROR_CODE);
> +            else
> +                domain_crash(v->domain);
> +        }
>          break;
>      }
>  
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #1.2: Type: text/html, Size: 3772 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel