Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Critical] Concurrent kfree in partition creation error path causes UAF or NULL pointer dereference in mshv_freeze_and_get_partition_ids(). - [High] KHO serialization APIs are invoked without verifying if the KHO subsystem is enabled. - [High] Lockless call to mshv_eventfd_release() races with concurrent ioctls, causing list corruption and double-frees. --
commit 1d32c3ff3fb719ececcc6529a01a0e1cb8e01bc9 Author: Jork Loeser <[email protected]> mshv: freeze and vacuum partitions across kexec This commit introduces logic to freeze VM partitions prior to a kexec, ensuring no VP modifies VM-memory reused by the new kernel. It preserves the partition IDs using the KHO subsystem so the successor kernel can retrieve and clean them up. Finally, it provides a mechanism to vacuum stale partitions left over from a previous kernel. > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c > --- a/drivers/hv/mshv_root_main.c > +++ b/drivers/hv/mshv_root_main.c > @@ -1762,6 +1790,201 @@ static void drain_all_vps(const struct mshv_partition > *partition) [ ... ] > + if (!nr_alloc) { > + pr_info("Frozen 0 partition(s) for kexec\n"); > + return 0; > + } > + > + ids = kho_alloc_preserve(nr_alloc * sizeof(*ids)); [Severity: High] Should we verify kho_is_enabled() before calling KHO serialization APIs like kho_alloc_preserve() here and kho_add_subtree() during preservation? If a standard kexec is performed (with kexec_in_progress true but KHO disabled), can calling these KHO serialization APIs lead to a NULL pointer dereference on kho_out.fdt? > + if (IS_ERR(ids)) { [ ... ] > + for (i = 0; i < nr_ref; i++) { > + /* Ref held; partition stays in hash and alive outside RCU */ > + scoped_guard(rcu) > + partition = mshv_partition_find(ids[i]); > + > + /* Barrier: wait for any in-flight create_vp to complete */ > + scoped_guard(mutex, &partition->pt_mutex) {} [Severity: Critical] Can mshv_partition_find() return NULL here if a partition creation error path concurrently unlinks and unconditionally frees the partition? If mshv_freeze_and_get_partition_ids() obtains a reference in the first loop, but the error path ignores the reference count and frees the partition anyway, would mshv_partition_find() return NULL after the partition is removed from the hash table? This would cause a panic when attempting to acquire &partition->pt_mutex. Additionally, if the reference count was ignored during a concurrent kfree, could this result in a use-after-free regression? [ ... ] > + /* > + * Tear down doorbell ports owned by the parent partition. > + * These survive child partition deletion and kexec, so the > + * new kernel would collide on port IDs if we leave them. > + */ > + mshv_eventfd_release(partition); [Severity: High] Is it safe to call mshv_eventfd_release() here without holding partition->pt_mutex? If mshv_root.frozen is not checked in the ioeventfd ioctl dispatch path, could userspace concurrently issue ioctls that modify the ioeventfds_list while this function is moving it, resulting in list corruption or double-frees? > + > + mshv_partition_put(partition); > + } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=20
