Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Tying VM partition IDs to process TGIDs (`current->tgid`) is an architectural flaw causing isolation breakage and security risks. - [High] `mshv_current_partid()` returns a raw partition ID without taking a reference, creating a TOCTOU vulnerability. - [High] Pre-existing Use-After-Free in `destroy_partition()` due to RCU-001 violation during VP teardown. --
commit 528e38745b2ae16c0fd8ad4876d893d1b1fea027 Author: Mukesh R <[email protected]> mshv: Provide a way to get partition ID if running in a VMM process This commit introduces mshv_current_partid() to allow retrieving the partition id for the current vmm process. It adds a pt_vmm_tgid field to the mshv_partition struct and sets it during partition creation. The partition id is found by iterating the global hash table. > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c > index bd1359eb58dd4..02c107458be90 100644 > --- a/drivers/hv/mshv_root_main.c > +++ b/drivers/hv/mshv_root_main.c > @@ -1908,6 +1908,27 @@ mshv_partition_release(struct inode *inode, struct > file *filp) > return 0; > } > > +/* Given a process tgid, return partition id if it is a VMM process */ > +u64 mshv_current_partid(void) > +{ > + struct mshv_partition *pt; > + int i; > + u64 ret_ptid = HV_PARTITION_ID_INVALID; > + > + rcu_read_lock(); > + > + hash_for_each_rcu(mshv_root.pt_htable, i, pt, pt_hnode) { > + if (pt->pt_vmm_tgid == current->tgid) { Is tying the partition id to the process tgid safe? Since a single vmm process can create multiple partitions, this loop returns the first partition matching the tgid. This could result in pci passthrough operations being misrouted to the wrong partition if a process manages multiple virtual machines. Also, since the partition's lifecycle is tied to an anonymous file descriptor, passing the fd to another process and exiting would break mshv_current_partid() for the new process. Additionally, if the operating system recycles the old tgid, an unrelated process might gain access to the partition id. Should explicit file descriptor tracking be used instead of global process ids? > + ret_ptid = pt->pt_id; > + break; > + } > + } > + > + rcu_read_unlock(); > + return ret_ptid; Does returning a raw partition id without taking a reference create a race condition? After rcu_read_unlock() is called and the pt_id is returned, a concurrent thread in the vmm could close the partition's file descriptor, causing the partition to be deleted in the hypervisor. If the hypervisor recycles the pt_id for a newly created partition belonging to a completely different process, the pending iommu attachment or pci passthrough hypercall will use the stale pt_id. Can this allow the vmm to inadvertently attach its devices to an unrelated partition? > +} This isn't a regression in this patch, but looking at the partition lifecycle code, does destroy_partition() have a use-after-free? destroy_partition() frees the virtual processor structures (kfree(vp)) before calling remove_partition(partition), which removes the partition from the rcu-protected global hash table (pt_htable) and calls synchronize_rcu(). Because the partition remains visible to rcu readers during the teardown loop, can an interrupt handler (e.g., in mshv_synic.c) call mshv_partition_find(), successfully retrieve the partition, and read vp = partition->pt_vp_array[vp_index]? If this happens concurrently with or immediately after kfree(vp), the interrupt handler would access freed memory. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
