[PATCH 0/6] Initialize device state before userspace registration

[PATCH 0/6] Initialize device state before userspace registration

[Jiale Yao]

Several drivers register character devices or sysfs attributes before
initializing synchronization objects and related state used by their
userspace entry points. Registration makes those entry points immediately
available, so a concurrent access can reach partially initialized state.

Move each initialization ahead of the corresponding registration. The
patches are independent and cover AoE, fsl-mc, MSHV, IBM VMC, DCSSBLK,
and the RDC321x watchdog.

Jiale Yao (6):
  aoe: initialize error message synchronization before registration
  bus: fsl-mc: initialize UAPI state before device registration
  Drivers: hv: initialize partition table before device registration
  misc: ibmvmc: initialize HMC state before device registration
  s390/dcssblk: initialize device semaphore before sysfs registration
  watchdog: rdc321x: initialize device state before registration

 drivers/block/aoe/aoechr.c       |  6 +++---
 drivers/bus/fsl-mc/fsl-mc-uapi.c |  6 ++----
 drivers/hv/mshv_root_main.c      |  6 +++---
 drivers/misc/ibmvmc.c            | 16 ++++++++--------
 drivers/s390/block/dcssblk.c     |  3 ++-
 drivers/watchdog/rdc321x_wdt.c   | 12 ++++++------
 6 files changed, 24 insertions(+), 25 deletions(-)

-- 
2.34.1

[PATCH 3/6] Drivers: hv: initialize partition table before device registration

[Jiale Yao]

misc_register() exposes /dev/mshv before the partition hash table and
its lock are initialized. A partition ioctl racing module initialization
can therefore access the table through an uninitialized spinlock.

Initialize the partition table and lock before registering the misc
device.

Fixes: 621191d709b1 ("Drivers: hv: Introduce mshv_root module to expose /dev/mshv to VMMs")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/hv/mshv_root_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index cc2cfce2aefd..ce26320035b6 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -2369,6 +2369,9 @@ static int __init mshv_parent_partition_init(void)
 	if (hv_get_hypervisor_version(&version_info))
 		return -ENODEV;
 
+	spin_lock_init(&mshv_root.pt_ht_lock);
+	hash_init(mshv_root.pt_htable);
+
 	ret = misc_register(&mshv_dev);
 	if (ret)
 		return ret;
@@ -2407,9 +2410,6 @@ static int __init mshv_parent_partition_init(void)
 	if (ret)
 		goto exit_debugfs;
 
-	spin_lock_init(&mshv_root.pt_ht_lock);
-	hash_init(mshv_root.pt_htable);
-
 	hv_setup_mshv_handler(mshv_isr);
 
 	return 0;
-- 
2.34.1

Re: [PATCH 3/6] Drivers: hv: initialize partition table before device registration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hv: mshv: premature misc_register() in mshv_parent_partition_init()
--

commit c189d106f4a7c8456e535fcfffc6e71d86c62b6d
Author: Jiale Yao <yaojiale02@163.com>

Drivers: hv: initialize partition table before device registration

This commit moves the initialization of mshv_root.pt_ht_lock and
mshv_root.pt_htable ahead of misc_register(&mshv_dev) in
mshv_parent_partition_init() so that concurrent partition ioctls do not
access an uninitialized spinlock or hash table.

> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index cc2cfce2aefdb..ce26320035b6f 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -2369,6 +2369,9 @@ static int __init mshv_parent_partition_init(void)
>  	if (hv_get_hypervisor_version(&version_info))
>  		return -ENODEV;
>  
> +	spin_lock_init(&mshv_root.pt_ht_lock);
> +	hash_init(mshv_root.pt_htable);
> +
>  	ret = misc_register(&mshv_dev);
>  	if (ret)
>  		return ret;
> @@ -2407,9 +2410,6 @@ static int __init mshv_parent_partition_init(void)
>  	if (ret)
>  		goto exit_debugfs;
>  
> -	spin_lock_init(&mshv_root.pt_ht_lock);
> -	hash_init(mshv_root.pt_htable);
> -
>  	hv_setup_mshv_handler(mshv_isr);
>  
>  	return 0;

[Severity: High]
Does moving only pt_ht_lock and pt_htable initialization ahead of
misc_register(&mshv_dev) leave /dev/mshv exposed while the rest of the
subsystem is still uninitialized?

In drivers/hv/mshv_root_main.c:mshv_parent_partition_init(),
misc_register(&mshv_dev) is still called before mshv_synic_init(),
mshv_init_vmm_caps(), mshv_retrieve_scheduler_type(), root_scheduler_init(),
mshv_debugfs_init(), mshv_irqfd_wq_init(), and hv_setup_mshv_handler().

If userspace opens /dev/mshv and issues MSHV_CREATE_PARTITION via
mshv_dev_ioctl() -> mshv_ioctl_create_partition() during this window,
partition creation now succeeds, which can trigger several issues before
mshv_parent_partition_init() finishes:

1. If the partition fd is closed before mshv_irqfd_wq_init() runs,
mshv_partition_release() -> mshv_eventfd_release() -> mshv_irqfd_release() in
drivers/hv/mshv_eventfd.c calls:

	flush_workqueue(irqfd_cleanup_wq);

while irqfd_cleanup_wq is still NULL.

2. If MSHV_INITIALIZE_PARTITION or MSHV_CREATE_VP runs before
mshv_debugfs_init() sets mshv_debugfs, mshv_debugfs_partition_create() and
mshv_debugfs_vp_create() return early and leave partition->pt_stats_dentry
and vp->vp_stats_dentry as NULL. When the partition is later destroyed after
mshv_debugfs_init() completes, destroy_partition() calls
mshv_debugfs_partition_remove() in drivers/hv/mshv_debugfs.c:

	if (!mshv_debugfs)
		return;

	partition_debugfs_remove(partition->pt_id,
				 partition->pt_stats_dentry);

which dereferences the NULL dentry pointer.

3. If MSHV_RUN_VP runs after mshv_retrieve_scheduler_type() sets
hv_scheduler_type to HV_SCHEDULER_TYPE_ROOT, but before root_scheduler_init()
allocates the per-CPU buffers, mshv_vp_dispatch() in
drivers/hv/mshv_root_main.c dereferences NULL per-CPU pointers:

	preempt_disable();
	input = *this_cpu_ptr(root_scheduler_input);
	output = *this_cpu_ptr(root_scheduler_output);

	memset(input, 0, sizeof(*input));

4. In mshv_ioctl_create_partition(), the partition file descriptor is created
with anon_inode_getfile("mshv_partition", &mshv_partition_fops, partition,
O_RDWR), which does not take a module reference. If a subsequent init step in
mshv_parent_partition_init() fails and triggers misc_deregister(&mshv_dev),
module initialization fails while the hypervisor partition remains allocated
and userspace retains an open file descriptor referencing unloaded module
fops.

Could misc_register(&mshv_dev) instead be moved to the end of
mshv_parent_partition_init() (or after all subsystem state is initialized,
passing NULL or a dedicated device pointer to the early init helpers)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028044828.96177-2-yaojiale02@163.com?part=3