linux-kernel - Re: [PATCH] KVM: nVMX: Don't return error on nested bitmap memory allocation failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 29 Apr 2015 10:27:17 +0300
From:	Nadav Amit <nadav.amit@...il.com>
To:	Bandan Das <bsd@...hat.com>
Cc:	kvm list <kvm@...r.kernel.org>,
	Paolo Bonzini <pbonzini@...hat.com>,
	Jan Kiszka <jan.kiszka@...mens.com>,
	Wincy Van <fanwenyi0529@...il.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: nVMX: Don't return error on nested bitmap memory allocation failure



Bandan Das <bsd@...hat.com> wrote:

> 
> If get_free_page() fails for nested bitmap area, it's evident that
> we are gonna get screwed anyway but returning failure because we failed
> allocating memory for a nested structure seems like an unnecessary big
> hammer. Also, save the call for later; after we are done with other
> non-nested allocations.
> 
> Signed-off-by: Bandan Das <bsd@...hat.com>
> ---
> arch/x86/kvm/vmx.c | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index f7b6168..200bc5c 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6039,20 +6039,22 @@ static __init int hardware_setup(void)
> 	if (!vmx_msr_bitmap_longmode_x2apic)
> 		goto out4;
> 
> -	if (nested) {
> -		vmx_msr_bitmap_nested =
> -			(unsigned long *)__get_free_page(GFP_KERNEL);
> -		if (!vmx_msr_bitmap_nested)
> -			goto out5;
> -	}
> -
> 	vmx_vmread_bitmap = (unsigned long *)__get_free_page(GFP_KERNEL);
> 	if (!vmx_vmread_bitmap)
> -		goto out6;
> +		goto out5;
> 
> 	vmx_vmwrite_bitmap = (unsigned long *)__get_free_page(GFP_KERNEL);
> 	if (!vmx_vmwrite_bitmap)
> -		goto out7;
> +		goto out6;
> +
> +	if (nested) {
> +		vmx_msr_bitmap_nested =
> +			(unsigned long *)__get_free_page(GFP_KERNEL);
> +		if (!vmx_msr_bitmap_nested) {
> +			printk(KERN_WARNING
> +			       "vmx: Failed getting memory for nested bitmap\n");
> +			nested = 0;
> +	}
> 
> 	memset(vmx_vmread_bitmap, 0xff, PAGE_SIZE);
> 	memset(vmx_vmwrite_bitmap, 0xff, PAGE_SIZE);
> @@ -6073,7 +6075,7 @@ static __init int hardware_setup(void)
> 
> 	if (setup_vmcs_config(&vmcs_config) < 0) {
> 		r = -EIO;
> -		goto out8;
> +		goto out7;
> 	}
> 
> 	if (boot_cpu_has(X86_FEATURE_NX))
> @@ -6190,13 +6192,12 @@ static __init int hardware_setup(void)
> 
> 	return alloc_kvm_area();
> 
> -out8:
> -	free_page((unsigned long)vmx_vmwrite_bitmap);
> out7:
> -	free_page((unsigned long)vmx_vmread_bitmap);
> -out6:
> 	if (nested)
> 		free_page((unsigned long)vmx_msr_bitmap_nested);
> +	free_page((unsigned long)vmx_vmwrite_bitmap);
> +out6:
> +	free_page((unsigned long)vmx_vmread_bitmap);
> out5:
> 	free_page((unsigned long)vmx_msr_bitmap_longmode_x2apic);
> out4:
> 
free_page appears to check whether the address is zero before it actually
frees the page. Perhaps it is better to leverage this behaviour to remove
all the outX and simplify the code.

Nadav

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/