Re: [PATCH RFC v3 32/88] KVM: guest_memfd: Support user fault handling in PFN mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] KVM: guest_memfd: false SIGBUS in kvm_gmem_pfn_mkwrite()
  after mprotect()
--

commit f3ec1839826136ca55a20572dfcf8ef47f4e0d64
Author: Ackerley Tng <ackerleytng@google.com>

KVM: guest_memfd: Support user fault handling in PFN mode

This commit adds user fault handling support for guest_memfd in PFN mode,
inserting PFNs with protections constrained by the provider's PFN
attributes and introducing kvm_gmem_pfn_vm_ops with a .pfn_mkwrite
callback.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 183e15775523f..e94f2359b7c9a 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -565,11 +565,40 @@ static bool kvm_gmem_supports_mmap(struct inode *inode)
>  	return GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_MMAP;
>  }
>  
> +/*
> + * In PFN mode, physical frame attributes are constant for the lifetime of the
> + * mapping. If a page was inserted read-only, it was because the provider
> + * reported a read-only PFN attribute, and write upgrades are never permitted.
> + * Write faults on such pages must fail with SIGBUS.
> + */
> +static vm_fault_t kvm_gmem_pfn_mkwrite(struct vm_fault *vmf)
> +{
> +	return VM_FAULT_SIGBUS;
> +}
> +
> +static vm_fault_t gmem_insert_pfn(struct vm_fault *vmf, kvm_pfn_t pfn,
> +				 u64 pfn_attrs)
> +{
> +	struct vm_area_struct *vma = vmf->vma;
> +	vm_flags_t flags = vma->vm_flags;
> +
> +	if (pfn_attrs & GUEST_MEMFD_PFN_ATTRS_READONLY)
> +		flags &= ~VM_WRITE;
> +
> +	if ((vmf->flags & FAULT_FLAG_WRITE) && !(flags & VM_WRITE))
> +		return VM_FAULT_SIGBUS;
> +
> +	return vmf_insert_pfn_prot(vma, vmf->address, pfn,
> +				   vm_get_page_prot(flags));
> +}

[Severity: Medium]
Can kvm_gmem_pfn_mkwrite() deliver a spurious SIGBUS when userspace writes
to a writable PFN after mprotect(PROT_READ | PROT_WRITE)?

If userspace maps a guest_memfd with PROT_READ and MAP_SHARED and faults
in a writable PFN via a read access, gmem_insert_pfn() installs a clean,
read-only PTE because vma->vm_flags lacks VM_WRITE.  Because
kvm_gmem_pfn_vm_ops sets .pfn_mkwrite, vma_wants_writenotify() is true for
the VMA and vma_set_page_prot() keeps vma->vm_page_prot read-only;
a subsequent mprotect(..., PROT_READ | PROT_WRITE) therefore leaves the
clean PTE read-only (can_change_shared_pte_writable() checks
pte_dirty(pte)) so that the next write traps into .pfn_mkwrite, where
kvm_gmem_pfn_mkwrite() unconditionally returns VM_FAULT_SIGBUS.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-gmem-alternative-providers-v3-0-8484dde66a7e@google.com?part=32