Re: [PATCH v5 6/9] mshv: wire SEV-SNP partition ioctls

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing host access restoration on successful PSP guest request permanently revokes host permissions.
- [High] Unbounded user-controlled array allocations and non-preemptible loops cause CPU soft lockups.
--

commit ff3cccc86d268ae551d99e559a8b63d138066868
Author: Wei Liu [off-list ref]
Subject: mshv: wire SEV-SNP partition ioctls

This commit wires the SEV-SNP UAPI into partition creation, memory ownership,
and isolated-page import. It adds tracking for host access permissions and
handles transitions to release/acquire host access.

[Severity: High]
Does this code allow an unbounded, user-controlled loop without yielding?

If a user provides an extremely large args.page_count in 
mshv_partition_ioctl_modify_gpa_host_access(), this loop will repeatedly 
acquire and release region mutexes for the entire array. Without a 
cond_resched(), this could cause a CPU soft lockup.

[ ... ]

+bool mshv_u64_array_has_duplicates(u64 *values, u64 count)
+{
+	u64 i;
+
+	sort(values, count, sizeof(*values), mshv_u64_cmp, NULL);

[Severity: High]
Can this sort() call cause a CPU soft lockup?

Since count originates from the user-controlled args.page_count without 
an upper bound, sort() (which is non-preemptible) might block the CPU 
for an O(N log N) duration on a massive array.

+	for (i = 1; i < count; i++)
+		if (values[i - 1] == values[i])
+			return true;
+	return false;
+}

[ ... ]

+static long
+mshv_partition_ioctl_modify_gpa_host_access(struct mshv_partition *partition,
+					    void __user *user_args)
+{

[ ... ]

+	guest_gpas = u64_to_user_ptr(args.guest_gpas);
+	gpfns = vmemdup_array_user(guest_gpas, args.page_count,
+				   sizeof(*gpfns));
+	if (IS_ERR(gpfns)) {
+		ret = PTR_ERR(gpfns);
+		gpfns = NULL;
+		goto out;
+	}
+	host_pages = kvmalloc_array(args.page_count, sizeof(*host_pages),
+				    GFP_KERNEL);
+	duplicates = kvmalloc_array(args.page_count, sizeof(*duplicates),
+				    GFP_KERNEL);

[Severity: High]
Are these allocations sufficiently bounded to prevent kernel memory exhaustion?

Since args.page_count is controlled by userspace, allocating multiple arrays 
that scale with this count could lead to severe memory pressure or a denial of 
service.

[ ... ]

+static long
+mshv_partition_ioctl_issue_psp_guest_request(struct mshv_partition *partition,
+					     void __user *user_args)
+{

[ ... ]

+	ret = hv_call_issue_psp_guest_request(partition->pt_id, gpfns[0],
+					      gpfns[1],
+					      mshv_async_hvcall_handler,
+					      partition);
+	if (!ret)
+		goto out_put_pages;
+
+restore_host_access:

[Severity: High]
Does a successful hypercall skip host access restoration?

If hv_call_issue_psp_guest_request() succeeds and ret is 0, the code 
jumps to out_put_pages, bypassing the restore_host_access block. 
This leaves the host permissions permanently revoked for the request and 
response pages, which could break functionality or leak memory.

+	{
+		int restore_ret;
+
+		restore_ret = mshv_restore_psp_pages(partition, host_pages,
+						     original, transitioned,
+						     transitioned_count);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908121403.1160280-1-weh@linux.microsoft.com?part=6