Re: [Xen-devel] [PATCH 4/4] x86/vvmx: Don't handle unknown nested vmexit reasons at L0

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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@xxxxxxxxxx>
>>>> ---
>>>> CC: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
>>>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>>>> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
>>>> CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
>>>> CC: Kevin Tian <kevin.tian@xxxxxxxxx>
>>>> ---
>>>>  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@xxxxxxxxxx>
...
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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel