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
