All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Sergey Dyasli <sergey.dyasli@citrix.com>,
	Xen-devel <xen-devel@lists.xen.org>
Cc: Kevin Tian <kevin.tian@intel.com>, Wei Liu <wei.liu2@citrix.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	Jan Beulich <JBeulich@suse.com>
Subject: Re: [PATCH 4/4] x86/vvmx: Don't handle unknown nested vmexit reasons at L0
Date: Fri, 26 Oct 2018 12:09:40 +0100	[thread overview]
Message-ID: <84386ade-538b-2a60-fd0e-e3661c53c386@citrix.com> (raw)
In-Reply-To: <dd055507-dbbc-c6f5-1b33-1d399e62bb18@citrix.com>

On 26/10/18 10:13, Sergey Dyasli wrote:
> On 26/10/2018 10:10, Andrew Cooper wrote:
>> On 26/10/2018 10:05, Sergey Dyasli wrote:
>>> On 25/10/2018 16:39, Andrew Cooper wrote:
>>>> This is very dangerous from a security point of view, because a missing entry
>>>> will cause L2's action to be interpreted as L1's action.
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> ---
>>>> CC: Sergey Dyasli <sergey.dyasli@citrix.com>
>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>> CC: Wei Liu <wei.liu2@citrix.com>
>>>> CC: Jun Nakajima <jun.nakajima@intel.com>
>>>> CC: Kevin Tian <kevin.tian@intel.com>
>>>> ---
>>>>  xen/arch/x86/hvm/vmx/vvmx.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
>>>> index d1c8a41..817d85f 100644
>>>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>>>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>>>> @@ -2609,8 +2609,9 @@ int nvmx_n2_vmexit_handler(struct cpu_user_regs *regs,
>>>>              nvcpu->nv_vmexit_pending = 1;
>>>>          break;
>>>>      default:
>>>> -        gprintk(XENLOG_ERR, "Unexpected nested vmexit: reason %u\n",
>>>> +        gprintk(XENLOG_ERR, "Unhandled nested vmexit: reason %u\n",
>>>>                  exit_reason);
>>>> +        domain_crash(v->domain);
>>>>      }
>>>>  
>>>>      return ( nvcpu->nv_vmexit_pending == 1 );
>>> Can you consider adding handling for the following?
>>>
>>> 	EXIT_REASON_INVD
>>> 	EXIT_REASON_RDTSCP
>>> 	EXIT_REASON_VMFUNC
>> Looks like these should be merged in with the INVVPID change, if your
>> happy with that?
> Yes, that would be a cleaner way, thanks.

Ok - that patch is now as follows:

x86/vvmx: Let L1 handle all the unconditional vmexit instructions

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
...
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index aa202e0..7051eb3 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2383,6 +2383,8 @@ int nvmx_n2_vmexit_handler(struct cpu_user_regs *regs,
     case EXIT_REASON_TRIPLE_FAULT:
     case EXIT_REASON_TASK_SWITCH:
     case EXIT_REASON_CPUID:
+    case EXIT_REASON_GETSEC:
+    case EXIT_REASON_INVD:
     case EXIT_REASON_VMCALL:
     case EXIT_REASON_VMCLEAR:
     case EXIT_REASON_VMLAUNCH:
@@ -2395,6 +2397,7 @@ int nvmx_n2_vmexit_handler(struct cpu_user_regs *regs,
     case EXIT_REASON_VMXON:
     case EXIT_REASON_INVEPT:
     case EXIT_REASON_XSETBV:
+    case EXIT_REASON_INVVPID:
         /* inject to L1 */
         nvcpu->nv_vmexit_pending = 1;
         break;

RDTSCP and VMFUNC mustn't be blindly forwarded to L1.  We need to check
whether the intercepts are active and either forward to L1 or handle at L0.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply	other threads:[~2018-10-26 11:09 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-25 15:36 [PATCH 0/4] x86/vvmx: XSA-278 followup fixes Andrew Cooper
2018-10-25 15:36 ` [PATCH 1/4] x86/vvmx: Unconditionally initialise vmxon_region_pa during vcpu construction Andrew Cooper
2018-10-26  9:09   ` Sergey Dyasli
2018-10-29 15:20   ` Jan Beulich
2018-10-30  7:11   ` Tian, Kevin
2018-10-25 15:38 ` [PATCH 2/4] x86/vvmx: Drop the now-obsolete vmx_inst_check_privilege() Andrew Cooper
2018-10-26  9:09   ` Sergey Dyasli
2018-10-29 15:21   ` Jan Beulich
2018-10-30  7:11   ` Tian, Kevin
2018-10-25 15:38 ` [PATCH 3/4] x86/vvmx: INVVPID instructions should be handled at by L1 Andrew Cooper
2018-10-26  8:21   ` Sergey Dyasli
2018-10-30  7:12   ` Tian, Kevin
2018-10-25 15:39 ` [PATCH 4/4] x86/vvmx: Don't handle unknown nested vmexit reasons at L0 Andrew Cooper
2018-10-26  9:05   ` Sergey Dyasli
2018-10-26  9:10     ` Andrew Cooper
2018-10-26  9:13       ` Sergey Dyasli
2018-10-26 11:09         ` Andrew Cooper [this message]
2018-10-26 11:31           ` Sergey Dyasli
2018-10-30  7:20   ` Tian, Kevin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=84386ade-538b-2a60-fd0e-e3661c53c386@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=jun.nakajima@intel.com \
    --cc=kevin.tian@intel.com \
    --cc=sergey.dyasli@citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.