Re: [PATCH V4 1/9] mshv: Provide a way to get partition ID if running in a VMM process

[sashiko-bot@kernel.org]

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

New issues:
- [High] Using global `current->tgid` to track and authorize partition ownership introduces a PID reuse vulnerability and breaks support for VMMs managing multiple partitions.
- [High] `mshv_current_partid()` returns a raw partition ID without acquiring a reference, leading to a TOCTOU race where the partition can be freed and its ID reused.

Pre-existing issues:
- [High] VPs are freed before their parent partition is unlinked from the RCU hash table, causing a Use-After-Free for concurrent RCU readers.
--

--- Patch [1]: [PATCH V4 1/9] mshv: Provide a way to get partition ID if running in a VMM process ---
commit ce04f426ae72519f5c20b1bad8ffd34a07810200
Author: Mukesh R <mrathor@linux.microsoft.com>

mshv: Provide a way to get partition ID if running in a VMM process

Many PCI passthru related hypercalls require partition ID of the target
guest. Guests are actually managed by MSHV driver and the partition ID
is only maintained there. Add a field in the partition struct in MSHV
driver to save the tgid of the VMM process creating the partition, and
add a function there to retrieve partition ID if the current process is
a VMM process.

> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index 146726cc4e9ba..f1ff45a77b42c 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;
>  }

[Severity: High]
This is a pre-existing issue, but there appears to be a use-after-free
condition when a partition is being destroyed.

In destroy_partition(), child mshv_vp objects are freed while the parent
partition remains linked in the RCU-protected mshv_root.pt_htable:

destroy_partition() {
    ...
    kfree(vp);
    partition->pt_vp_array[i] = NULL;
    ...
    remove_partition(partition);
}

If a concurrent asynchronous interrupt (like handle_bitset_message) looks up
the partition via mshv_partition_find() and accesses the VP before
remove_partition() unlinks the partition and waits for a grace period,
could the ISR call kick_vp(vp) on freed memory?

> +/* 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) {
> +			ret_ptid = pt->pt_id;
> +			break;
> +		}
> +	}

[Severity: High]
Is it safe to track partition ownership using a global transient identifier
like current->tgid instead of an explicit file descriptor or reference-counted
token?

If a single VMM process creates multiple partitions, wouldn't
mshv_current_partid() arbitrarily return the first one found in the hash
table, breaking PCI passthru operations for all subsequent partitions?

Furthermore, could PID recycling lead to a situation where a new VMM process
accidentally matches an old partition whose fd was passed to a long-running
process that outlived the original creator?

> +
> +	rcu_read_unlock();
> +	return ret_ptid;

[Severity: High]
Does returning the raw pt_id here without acquiring a reference to the
partition object introduce a race condition?

Once the RCU read lock is dropped, a concurrent thread could close the
partition fd, causing destroy_partition() to delete it in the hypervisor.

If the hypervisor reuses this ID for a newly created partition belonging to a
different VMM, could the original caller's subsequent PCI passthru hypercall
mistakenly target the unrelated partition?

> +}
> +EXPORT_SYMBOL_GPL(mshv_current_partid);

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