Yan Zhao <yan.y.zhao@xxxxxxxxx> writes:
> On Tue, Sep 10, 2024 at 11:43:58PM +0000, Ackerley Tng wrote:
>> guest_memfd files can always be mmap()ed to userspace, but
>> faultability is controlled by an attribute on the inode.
>>
>> Co-developed-by: Fuad Tabba <tabba@xxxxxxxxxx>
>> Signed-off-by: Fuad Tabba <tabba@xxxxxxxxxx>
>> Co-developed-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
>> Signed-off-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
>>
>> ---
>> virt/kvm/guest_memfd.c | 46 ++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 44 insertions(+), 2 deletions(-)
>>
>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>> index b603518f7b62..fc2483e35876 100644
>> --- a/virt/kvm/guest_memfd.c
>> +++ b/virt/kvm/guest_memfd.c
>> @@ -781,7 +781,8 @@ static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
>> {
> Hi Ackerley,
>
> If userspace mmaps a guest_memfd to a VA when a GFN range is shared, it looks
> that even after the GFN range has been successfully converted to private,
> userspace can still call madvise(mem, size, MADV_REMOVE) on the userspace VA.
> This action triggers kvm_gmem_punch_hole() and kvm_gmem_invalidate_begin(),
> which can zap the private GFNs in the EPT.
>
> Is this behavior intended for in-place conversion, and could it potentially lead
> to private GFN ranges being accidentally zapped from the EPT?
>
> Apologies if I missed any related discussions on this topic.
No worries and thank you for your review! The next revision will not be
requiring userspace to do madvise(MADV_REMOVE), because memory could be
mapped in multiple processes, so unmapping from the kernel saves the
trouble of coordination in userspace between multiple processes.
>
> Thanks
> Yan
>
>> struct list_head *gmem_list = &inode->i_mapping->i_private_list;
>> pgoff_t start = offset >> PAGE_SHIFT;
>> - pgoff_t end = (offset + len) >> PAGE_SHIFT;
>> + pgoff_t nr = len >> PAGE_SHIFT;
>> + pgoff_t end = start + nr;
>> struct kvm_gmem *gmem;
>>
>> /*
>> @@ -790,6 +791,9 @@ static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
>> */
>> filemap_invalidate_lock(inode->i_mapping);
>>
>> + /* TODO: Check if even_cows should be 0 or 1 */
>> + unmap_mapping_range(inode->i_mapping, start, len, 0);
>> +
>> list_for_each_entry(gmem, gmem_list, entry)
>> kvm_gmem_invalidate_begin(gmem, start, end);
>>
