Re: [PATCH] KVM: Mask off the address space ID in kvm_set_internal_memslot() check

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] `__x86_set_memory_region()` leaks user virtual memory allocated via `vm_mmap` and leaves the VM in a corrupted partial state on error paths when `kvm_set_internal_memslot()` fails.
--

commit 1d364f757ed17cf4e343c19a31d2b4aeb3b66e5f
Author: Zeng Chi <zengchi@kylinos.cn>

KVM: Mask off the address space ID in kvm_set_internal_memslot() check

This commit fixes a sanity check in kvm_set_internal_memslot to correctly
isolate and compare just the memslot ID against KVM_USER_MEM_SLOTS. By
masking off the address space ID, the check is now effective across all
address spaces rather than silently passing for any address space other
than 0.

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 65eb26a0520d8..e8d0e360ea26b 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2128,7 +2128,7 @@ static int kvm_set_memory_region(struct kvm *kvm,
>  int kvm_set_internal_memslot(struct kvm *kvm,
>  			     const struct kvm_userspace_memory_region2 *mem)
>  {
> -	if (WARN_ON_ONCE(mem->slot < KVM_USER_MEM_SLOTS))
> +	if (WARN_ON_ONCE((u16)mem->slot < KVM_USER_MEM_SLOTS))
>  		return -EINVAL;
>  
>  	if (WARN_ON_ONCE(mem->flags))

[Severity: High]
This is a pre-existing issue, but I have a question about the error handling
in arch/x86/kvm/x86.c:__x86_set_memory_region() when
kvm_set_internal_memslot() fails.

When size > 0, the function allocates a VMA mapping via vm_mmap() and
iterates over all address space IDs:

        for (i = 0; i < kvm_arch_nr_memslot_as_ids(kvm); i++) {
                struct kvm_userspace_memory_region2 m;
                ...
                r = kvm_set_internal_memslot(kvm, &m);
                if (r < 0)
                        return ERR_PTR_USR(r);
        }

If kvm_set_internal_memslot() fails for any address space, it returns early.

Does this early return skip the vm_munmap() cleanup and leak the VMA mapping
allocated by vm_mmap() in the host process?

Additionally, if the loop fails on a later address space, earlier successfully
created memslots are left behind. During VM destruction, kvm_arch_destroy_vm()
calls __x86_set_memory_region() with size = 0 to free these internal memslots.
Will this deletion loop fail with -EINVAL when it attempts to delete the
non-existent slot for the previously failed address space? Does this bypass
the final vm_munmap() step and leak the VMA mapping?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827112200.866476-1-zeng_chi911@163.com?part=1