Re: [PATCH v8 24/46] KVM: guest_memfd: Make in-place conversion the default

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 6/25/2026 7:47 AM, Ackerley Tng wrote:
> sashiko-bot@kernel.org writes:
> 
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [Medium] Unprivileged userspace can trigger a WARN_ON_ONCE if the host administrator misconfigures a module parameter.
>> --
>>
>> commit bf8cb04a544d7b8235d73034e0febf195ab335c3
>> Author: Ackerley Tng <ackerleytng@google.com>
>>
>> KVM: guest_memfd: Make in-place conversion the default
>>
>> This commit makes in-place conversion the default when the architecture
>> supports private memory. It introduces a module parameter to override the
>> default and adds a warning in the CoCo VM memory population path if in-place
>> conversion is disabled without enabling per-VM memory attributes.
>>
>>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>>> index 86c9f5b0863cb..5cb73543c03c8 100644
>>> --- a/virt/kvm/guest_memfd.c
>>> +++ b/virt/kvm/guest_memfd.c
>>> @@ -1193,10 +1193,15 @@ static bool kvm_gmem_range_is_private(struct file *file, pgoff_t index,
>>>   {
>>>   	struct maple_tree *mt = &GMEM_I(file_inode(file))->attributes;
>>>
>>> +#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
>>>   	if (!gmem_in_place_conversion)
>>>   		return kvm_range_has_vm_memory_attributes(kvm, gfn, gfn + nr_pages,
>>>   							  KVM_MEMORY_ATTRIBUTE_PRIVATE,
>>>   							  KVM_MEMORY_ATTRIBUTE_PRIVATE);
>>> +#else
>>> +	if (WARN_ON_ONCE(!gmem_in_place_conversion))
>>> +		return false;
>>> +#endif
>>
>> [Severity: Medium]
>> Could this WARN_ON_ONCE() be triggered by unprivileged userspace if the
>> administrator misconfigures the module parameter?
>>
>> If the system administrator sets kvm.gmem_in_place_conversion=0 on a kernel
>> compiled without CONFIG_KVM_VM_MEMORY_ATTRIBUTES, it appears this warning can
>> be reached from an unprivileged ioctl path calling kvm_gmem_populate().
>>
>> Since an unprivileged user creating a CoCo VM can intentionally trigger this
>> warning, could this act as a host-side denial of service on systems configured
>> with panic_on_warn=1?
>>
>> Would it be better to convert this to a pr_warn_once()
> 
> Sean, what do you think of using pr_warn_once()?

How about don't expose the module param when 
CONFIG_KVM_VM_MEMORY_ATTRIBUTES=n? something like:

--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -103,7 +103,9 @@ module_param(allow_unsafe_mappings, bool, 0444);

  #ifdef kvm_arch_has_private_mem
  bool __ro_after_init gmem_in_place_conversion = 
!IS_ENABLED(CONFIG_KVM_VM_MEMORY_ATTRIBUTES);
+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
  module_param(gmem_in_place_conversion, bool, 0444);
+#endif
  EXPORT_SYMBOL_FOR_KVM_INTERNAL(gmem_in_place_conversion);
  #endif


>> or perhaps validate the
>> module parameter at initialization time?
> 
> I thought about this too, but didn't want to move it to initialization
> time because this (populate) is probably the first time the kernel
> knows for sure something is used weirdly.
> 
> Like, perhaps the admin did compile without
> CONFIG_KVM_VM_MEMORY_ATTRIBUTES and also set gmem_in_place_conversion=0
> but wants to use the host only for non-CoCo VMs and hence doesn't need a
> warning, idk.
> 
>>
>> --
>> Sashiko AI review · https://sashiko.dev/#/patchset/20260618-gmem-inplace-conversion-v8-0-9d2959357853@google.com?part=24
>