linux-kernel - Re: [PATCH v12 11/20] KVM: xen: allow shared

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: Thu, 8 Feb 2024 16:51:18 +0000
From: Paul Durrant <xadimgnik@...il.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, Jonathan Corbet <corbet@....net>,
 Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
 David Woodhouse <dwmw2@...radead.org>, Shuah Khan <shuah@...nel.org>,
 kvm@...r.kernel.org, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v12 11/20] KVM: xen: allow shared_info to be mapped by
 fixed HVA

On 08/02/2024 16:48, Sean Christopherson wrote:
> On Thu, Feb 08, 2024, Paul Durrant wrote:
>> On 07/02/2024 04:10, Sean Christopherson wrote:
>>> On Mon, Jan 15, 2024, Paul Durrant wrote:
>>>> @@ -638,20 +637,32 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
>>>>    		}
>>>>    		break;
>>>> -	case KVM_XEN_ATTR_TYPE_SHARED_INFO: {
>>>> +	case KVM_XEN_ATTR_TYPE_SHARED_INFO:
>>>> +	case KVM_XEN_ATTR_TYPE_SHARED_INFO_HVA: {
>>>>    		int idx;
>>>>    		mutex_lock(&kvm->arch.xen.xen_lock);
>>>>    		idx = srcu_read_lock(&kvm->srcu);
>>>> -		if (data->u.shared_info.gfn == KVM_XEN_INVALID_GFN) {
>>>> -			kvm_gpc_deactivate(&kvm->arch.xen.shinfo_cache);
>>>> -			r = 0;
>>>> +		if (data->type == KVM_XEN_ATTR_TYPE_SHARED_INFO) {
>>>> +			if (data->u.shared_info.gfn == KVM_XEN_INVALID_GFN) {
>>>> +				kvm_gpc_deactivate(&kvm->arch.xen.shinfo_cache);
>>>> +				r = 0;
>>>> +			} else {
>>>> +				r = kvm_gpc_activate(&kvm->arch.xen.shinfo_cache,
>>>> +						     gfn_to_gpa(data->u.shared_info.gfn),
>>>> +						     PAGE_SIZE);
>>>> +			}
>>>>    		} else {
>>>> -			r = kvm_gpc_activate(&kvm->arch.xen.shinfo_cache,
>>>> -					     gfn_to_gpa(data->u.shared_info.gfn),
>>>> -					     PAGE_SIZE);
>>>> +			if (data->u.shared_info.hva == 0) {
>>>
>>> I know I said I don't care about the KVM Xen ABI, but I still think using '0' as
>>> "invalid" is ridiculous.
>>>
>>
>> With the benefit of some sleep, I'm wondering why 0 is a 'ridiculous'
>> invalid value for a *virtual* address? Surely it's essentially a numerical
>> cast of the canonically invalid NULL pointer?
> 
> It's legal to mmap() virtual address '0', albeit not by default:
> 
>    config DEFAULT_MMAP_MIN_ADDR
> 	int "Low address space to protect from user allocation"
> 	depends on MMU
> 	default 4096
> 	help
> 	  This is the portion of low virtual memory which should be protected
> 	  from userspace allocation.  Keeping a user from writing to low pages
> 	  can help reduce the impact of kernel NULL pointer bugs.
> 
> 	  For most ppc64 and x86 users with lots of address space
> 	  a value of 65536 is reasonable and should cause no problems.
> 	  On arm and other archs it should not be higher than 32768.
> 	  Programs which use vm86 functionality or have some need to map
> 	  this low address space will need CAP_SYS_RAWIO or disable this
> 	  protection by setting the value to 0.
> 
> 	  This value can be changed after boot using the
> 	  /proc/sys/vm/mmap_min_addr tunable.
> 
> 
> Obviously it's equally ridiculous that userspace would ever mmap() '0' and pass
> that as the shared_info, but given that this is x86-only, there are architecturally
> illegal addresses that can be used, at least until Intel adds LA64 ;-)

Ok. Thanks for the reference.