Re: [PATCH RFC v4 13/44] KVM: guest_memfd: Apply content modes while setting memory attributes

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] WARN_ON() discards the error code from kvm_gmem_apply_content_mode(), leading to silent failures and a user-triggerable kernel warning (DoS vector).
- [Medium] Applying content mode without invalidating MMU mappings when memory attributes do not change causes data races and potential hardware machine checks.
--

--- Patch [13]: [PATCH RFC v4 13/44] KVM: guest_memfd: Apply content modes while setting memory attributes ---
commit 47c20f7dd20767b04a5efc7e5951d56b44e5a590
Author: Ackerley Tng <[email protected]>

KVM: guest_memfd: Apply content modes while setting memory attributes

Provide defined memory content modes so that KVM can make guarantees about
memory content after setting memory attributes, according to userspace
requests.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index e270e54e030f0..eeac7678fcf4e 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c

[ ... ]

> @@ -752,9 +783,23 @@ static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>  
>  	filemap_invalidate_lock(mapping);
>  
> +	if (!kvm_gmem_content_mode_is_supported(kvm, content_mode,
> +						to_private)) {
> +		r = -EOPNOTSUPP;
> +		*err_index = start;
> +		goto out;
> +	}
> +
>  	mas_init(&mas, mt, start);
>  
>  	if (kvm_gmem_range_has_attributes(mt, start, nr_pages, attrs)) {
> +		/*
> +		 * Even if no update is required to attributes, the
> +		 * requested content mode is applied.
> +		 */
> +		WARN_ON(kvm_gmem_apply_content_mode(kvm, content_mode,
> +						    inode, start, end));
> +
>  		r = 0;
>  		goto out;
>  	}

[Severity: Medium]
Does this path allow the host to modify memory (e.g., zeroing it) while the
guest still has active PTEs?

Since this early exit path skips kvm_gmem_invalidate_begin() or
unmap_mapping_pages(), the KVM MMU is not zapped. Could this introduce guest
data races, or hardware state machine violations if the architecture-specific
preserve operation acts on actively mapped private pages?

[Severity: High]
Is the error code from kvm_gmem_apply_content_mode() being discarded here?

WARN_ON() evaluates to a boolean and its return value isn't captured. If a
folio is hardware-poisoned and kvm_gmem_apply_content_mode_zero() returns
-EHWPOISON, it appears this will silently return 0 (success) to userspace 
meaning memory was not zeroed as requested.

Additionally, since this is reachable by untrusted userspace via the 
KVM_SET_MEMORY_ATTRIBUTES2 ioctl, could this trigger a host denial of
service if panic_on_warn is enabled?

[ ... ]

> @@ -786,6 +831,9 @@ static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>  	if (!to_private)
>  		kvm_gmem_invalidate(inode, start, end);
>  
> +	WARN_ON(kvm_gmem_apply_content_mode(kvm, content_mode, inode,
> +					    start, end));
> +
>  	mas_store_prealloc(&mas, xa_mk_value(attrs));
>  
>  	kvm_gmem_invalidate_end(inode, start, end);

[Severity: High]
Similar to the early exit path above, does this discard the error code from
kvm_gmem_apply_content_mode()?

If this is modified to capture and return the error code, would we also
need to call mas_destroy(&mas) on the error path to avoid leaking the
preallocated maple tree nodes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13