Re: [PATCH v1 10/13] mshv: unmap SNP memory before state teardown

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

Pre-existing issues:
- [High] Userspace can trigger a kernel memory leak or DoS by unmapping memory regions of an active encrypted partition.
--

--- Patch [10]: [PATCH v1 10/13] mshv: unmap SNP memory before state teardown ---
commit f9f61905fec1fff131a4505ade5ecacaee8647a1
Author: Wei Liu <[email protected]>

mshv: unmap SNP memory before state teardown

This commit updates the partition teardown process to unmap memory regions
before destroying the SNP partition state. The regions are only released
after the SNP state is fully torn down, which allows the host to regain
access while the partition is still secure.

[ ... ]
> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index c81a8d0493c19..0fbd2158968da 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
[ ... ]
> @@ -2166,10 +2166,8 @@ static void destroy_partition(struct mshv_partition *partition)
>  		if (mshv_partition_encrypted(partition)) {
>  			hlist_for_each_entry_safe(region, n,
>  						  &partition->pt_mem_regions,
> -						  hnode) {
> -				hlist_del(&region->hnode);
> -				mshv_region_put(region);
> -			}
> +						  hnode)
> +				mshv_region_unmap(region);
>  
>  			ret = destroy_snp_partition_state(partition);
>  			if (ret) {

[Severity: High]
This is a pre-existing issue, but does a similar problem occur when userspace
unmaps memory via the MSHV_SET_GUEST_MEMORY ioctl?

When a userspace process unmaps memory from an active SEV-SNP partition,
mshv_unmap_user_memory() drops the region unconditionally without checking the
SNP state:

drivers/hv/mshv_root_main.c:mshv_unmap_user_memory() {
    ...
    region = mshv_partition_region_by_gfn(partition, mem->guest_pfn);
    ...
    hlist_del(&region->hnode);
    ...
    mshv_region_put(region);
}

Because the partition's SNP state is still active, could mshv_region_share()
fail to regain host access during teardown?

drivers/hv/mshv_regions.c:mshv_region_destroy() {
    ...
    if (mshv_partition_encrypted(partition)) {
        ret = mshv_region_share(region);
        if (ret) {
            pt_err(partition,
                   "Failed to regain access to memory...\n",
                   ret);
            return;
        }
    }
}

If it returns early here, does this result in a permanent leak of the pinned
host pages and kernel structures?

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