[PATCH 2/2] kvm, ept: remove the default write bit

[PATCH 2/2] kvm, ept: remove the default write bit

[Lai Jiangshan]

When ept enabled, current code set shadow_base_present_pte
including the write bit, thus all pte entries have
writabe bit, and it means guest os can always
write to any mapped page (even VMM maps RO pages for
the guest.)

We always use get_user_pages(write=1), so this bad code does not
cause any bad result currently.

But it is really bad, so fix it, and we will use RO pages future.

We will set writabe bit when it is really writable (determined by
the parameters of the set_spte())

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index fdb18cf..c7565ea 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4365,8 +4365,7 @@ static int __init vmx_init(void)
 
 	if (enable_ept) {
 		bypass_guest_pf = 0;
-		kvm_mmu_set_base_ptes(VMX_EPT_READABLE_MASK |
-			VMX_EPT_WRITABLE_MASK);
+		kvm_mmu_set_base_ptes(VMX_EPT_READABLE_MASK);
 		kvm_mmu_set_mask_ptes(0ull, 0ull, 0ull, 0ull,
 				VMX_EPT_EXECUTABLE_MASK);
 		kvm_enable_tdp();

Re: [PATCH 2/2] kvm, ept: remove the default write bit

[Marcelo Tosatti]

On Fri, Jun 11, 2010 at 07:30:50PM +0800, Lai Jiangshan wrote:
> When ept enabled, current code set shadow_base_present_pte
> including the write bit, thus all pte entries have
> writabe bit, and it means guest os can always
> write to any mapped page (even VMM maps RO pages for
> the guest.)
> 
> We always use get_user_pages(write=1), so this bad code does not
> cause any bad result currently.
> 
> But it is really bad, so fix it, and we will use RO pages future.
> 
> We will set writabe bit when it is really writable (determined by
> the parameters of the set_spte())
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index fdb18cf..c7565ea 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -4365,8 +4365,7 @@ static int __init vmx_init(void)
>  
>  	if (enable_ept) {
>  		bypass_guest_pf = 0;
> -		kvm_mmu_set_base_ptes(VMX_EPT_READABLE_MASK |
> -			VMX_EPT_WRITABLE_MASK);
> +		kvm_mmu_set_base_ptes(VMX_EPT_READABLE_MASK);
>  		kvm_mmu_set_mask_ptes(0ull, 0ull, 0ull, 0ull,
>  				VMX_EPT_EXECUTABLE_MASK);
>  		kvm_enable_tdp();

You can remove the call to kvm_mmu_set_base_ptes entirely, because
VMX_EPT_READABLE_MASK == PT_PRESENT_MASK.

Re: [PATCH 2/2] kvm, ept: remove the default write bit

[Avi Kivity]

On 06/11/2010 10:50 PM, Marcelo Tosatti wrote:
> On Fri, Jun 11, 2010 at 07:30:50PM +0800, Lai Jiangshan wrote:
>    
>> When ept enabled, current code set shadow_base_present_pte
>> including the write bit, thus all pte entries have
>> writabe bit, and it means guest os can always
>> write to any mapped page (even VMM maps RO pages for
>> the guest.)
>>
>> We always use get_user_pages(write=1), so this bad code does not
>> cause any bad result currently.
>>
>> But it is really bad, so fix it, and we will use RO pages future.
>>
>> We will set writabe bit when it is really writable (determined by
>> the parameters of the set_spte())
>>
>> Signed-off-by: Lai Jiangshan<laijs@cn.fujitsu.com>
>> ---
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index fdb18cf..c7565ea 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -4365,8 +4365,7 @@ static int __init vmx_init(void)
>>
>>   	if (enable_ept) {
>>   		bypass_guest_pf = 0;
>> -		kvm_mmu_set_base_ptes(VMX_EPT_READABLE_MASK |
>> -			VMX_EPT_WRITABLE_MASK);
>> +		kvm_mmu_set_base_ptes(VMX_EPT_READABLE_MASK);
>>   		kvm_mmu_set_mask_ptes(0ull, 0ull, 0ull, 0ull,
>>   				VMX_EPT_EXECUTABLE_MASK);
>>   		kvm_enable_tdp();
>>      
> You can remove the call to kvm_mmu_set_base_ptes entirely, because
> VMX_EPT_READABLE_MASK == PT_PRESENT_MASK.
>    

We can leave that to a later patch which removes kvm_mmu_set_base_ptes() 
entirely.

-- 
error compiling committee.c: too many arguments to function