linux-kernel - Re: [RFC PATCH 1/8] KVM: Document KVM_MAP

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: Fri, 8 Mar 2024 13:20:55 +1300
From: "Huang, Kai" <kai.huang@...el.com>
To: Isaku Yamahata <isaku.yamahata@...ux.intel.com>
CC: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "Yamahata, Isaku"
	<isaku.yamahata@...el.com>, "federico.parola@...ito.it"
	<federico.parola@...ito.it>, "pbonzini@...hat.com" <pbonzini@...hat.com>,
	"dmatlack@...gle.com" <dmatlack@...gle.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "isaku.yamahata@...il.com"
	<isaku.yamahata@...il.com>, "michael.roth@....com" <michael.roth@....com>,
	"seanjc@...gle.com" <seanjc@...gle.com>
Subject: Re: [RFC PATCH 1/8] KVM: Document KVM_MAP_MEMORY ioctl


>>>   
>>> +4.143 KVM_MAP_MEMORY
>>> +------------------------
>>> +
>>> +:Capability: KVM_CAP_MAP_MEMORY
>>> +:Architectures: none
>>> +:Type: vcpu ioctl
>>
>> I think "vcpu ioctl" means theoretically it can be called on multiple vcpus.
>>
>> What happens in that case?
> 
> Each vcpu can handle the ioctl simaltaneously.  

Not sure whether it is implied, but should we document it can be called 
simultaneously?

Also, I believe this is only supposed to be called before VM starts to 
run?  I think we should document that too.

This is userspace ABI, we need to be explicit on how it is supposed to 
be called from userspace.

Btw, I believe there should be some justification in the changelog why 
this should be a vcpu ioctl().

[...]

>>> +:Parameters: struct kvm_memory_mapping(in/out)
>>> +:Returns: 0 on success, <0 on error
>>> +
>>> +KVM_MAP_MEMORY populates guest memory without running vcpu.
>>> +
>>> +::
>>> +
>>> +  struct kvm_memory_mapping {
>>> +	__u64 base_gfn;
>>> +	__u64 nr_pages;
>>> +	__u64 flags;
>>> +	__u64 source;
>>> +  };
>>> +
>>> +  /* For kvm_memory_mapping:: flags */
>>> +  #define KVM_MEMORY_MAPPING_FLAG_WRITE         _BITULL(0)
>>> +  #define KVM_MEMORY_MAPPING_FLAG_EXEC          _BITULL(1)
>>> +  #define KVM_MEMORY_MAPPING_FLAG_USER          _BITULL(2)
>>
>> I am not sure what's the good of having "FLAG_USER"?
>>
>> This ioctl is called from userspace, thus I think we can just treat this always
>> as user-fault?
> 
> The point is how to emulate kvm page fault as if vcpu caused the kvm page
> fault.  Not we call the ioctl as user context.

Sorry I don't quite follow.  What's wrong if KVM just append the #PF 
USER error bit before it calls into the fault handler?

My question is, since this is ABI, you have to tell how userspace is 
supposed to use this.  Maybe I am missing something, but I don't see how 
USER should be used here.