Sean Christopherson <[email protected]> writes: > > [...snip...] > >> +#ifdef CONFIG_USERFAULTFD >> +static bool kvm_gmem_can_userfault(struct vm_area_struct *vma, vm_flags_t >> vm_flags) >> +{ >> + struct inode *inode = file_inode(vma->vm_file); >> + >> + /* >> + * Only support userfaultfd for guest_memfd with INIT_SHARED flag. >> + * This ensures the memory can be mapped to userspace. >> + */
Is the principle here that any memory that is allowed to be mapped to userspace can be userfault-ed? >> + if (!(GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_INIT_SHARED)) >> + return false; > > I'm not comfortable with this change. It works for now, but it's going to be > wildly wrong when in-place conversion comes along. While I agree with the > "Let's > solve each problem in it's time :)"[*], the time for in-place conversion is > now. > In-place conversion isn't landing this cycle or next, but it's been in > development > for longer than UFFD support, and I'm not willing to punt solvable problems to > that series, because it's plenty fat as is. > Thanks :) > Happily, IIUC, this is an easy problem to solve, and will have a nice side > effect > for the common UFFD code. > > My objection to an early, global "can_userfault()" check is that it's > guaranteed > to cause TOCTOU issues. E.g. for VM_UFFD_MISSING and VM_UFFD_MINOR, the > check on > whether or not a given address can be faulted in needs to happen in > __do_userfault(), > not broadly when VM_UFFD_MINOR is added to a VMA. Conceptually, that also > better > aligns the code with the "normal" user fault path in > kvm_gmem_fault_user_mapping(). > > I'm definitely not asking to fully prep for in-place conversion, I just want > to > set us up for success and also to not have to churn a pile of code. > Concretely, > again IIUC, I think we just need to move the INIT_SHARED check to > ->alloc_folio() > and ->get_folio_noalloc(). And if we extract kvm_gmem_is_shared_mem() now > instead > of waiting for in-place conversion, then we'll avoid a small amount of churn > when > in-place conversion comes along. > I think it would help if we do some refactoring of existing allocation functions to provide guest_memfd's uffd ->alloc_folio and ->get_folio_noalloc callbacks instead of having new functions. Perhaps uffd ->alloc_folio and ->get_folio_noalloc can call an alloc function refactored out of kvm_gmem_get_folio(), and also do the kvm_gmem_is_shared_mem() check. [email protected], who is starting work on guest_memfd preservation (as in LUO/KHO). > > [...snip...] >
