[RFC PATCH] kobject: fix uevent helper execution order issues

[RFC PATCH] kobject: fix uevent helper execution order issues

[zhoumin]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 4456 bytes --]

The current use of the UMH_NO_WAIT parameter does not guarantee the
sequential execution of CONFIG_STATIC_USERMODEHELPER_PATH.

For example, when fdisk completes and issues a BLKRRPART ioctl command,
kernel first sending remove events followed by an add events. However, 
process CONFIG_STATIC_USERMODEHELPER_PATH may execute
out-of-order—potentially handling the add before the remove. This can
result in the new partition being unexpectedly unmounted instead of mounted
as intended.

Admittedly, the current approach does not fully ensure that all
CONFIG_STATIC_USERMODEHELPER_PATH helpers execute in strict sequential
order. I have not yet identified a more robust solution and welcome
feedback on this issue. That said, if a mechanism can be established to
enforce reliable ordering of these user-mode helpers, higher-level
synchronization primitives—such as file locks—could then be used to
coordinate the overall execution sequence in userspace.

Test Log:

CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"

~ # cat /sbin/hotplug
#!/bin/sh
echo hotplug $DEVPATH $ACTION $DEVTYPE $MAJOR $MINOR $SUBSYSTEM > /dev/console
~ #

~ # fdisk /dev/sda

The number of cylinders for this disk is set to 8354.
There is nothing wrong with that, but this is larger than 1024,
and could in certain setups cause problems with:
1) software that runs at boot time (e.g., old versions of LILO)
2) booting and partitioning software from other OSs
   (e.g., DOS FDISK, OS/2 FDISK)

Command (m for help): p
Disk /dev/sda: 64 GB, 68719476736 bytes, 134217728 sectors
8354 cylinders, 255 heads, 63 sectors/track
Units: sectors of 1 * 512 = 512 bytes

Device  Boot StartCHS    EndCHS        StartLBA     EndLBA    Sectors  Size Id Type
/dev/sda1    0,1,1       12,191,50           63     204862     204800  100M 83 Linux
/dev/sda2    12,191,51   25,127,37       204863     409662     204800  100M 83 Linux
/dev/sda3    25,127,38   127,123,59      409663    2048062    1638400  800M 83 Linux
/dev/sda4    127,123,60  178,122,7      2048063    2867262     819200  400M  5 Extended
/dev/sda5    127,124,60  178,122,7      2048126    2867262     819137  399M 83 Linux

Command (m for help): wq
The partition table has been altered.
Calling ioctl() to re-read partition table
[  282.069158]  sda: sda1 sda2 sda3 sda4 < sda5 >
hotplug /devices/platform/4010000000.pcie/pci0000:00/0000:00:01.0/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda2 remove partition 8 2 block
~ # hotplug /devices/platform/4010000000.pcie/pci0000:00/0000:00:01.0/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda1 remove partition 8 1 block
hotplug /devices/platform/4010000000.pcie/pci0000:00/0000:00:01.0/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda3 remove partition 8 3 block
hotplug /devices/platform/4010000000.pcie/pci0000:00/0000:00:01.0/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda5 add partition 8 5 block
hotplug /devices/platform/4010000000.pcie/pci0000:00/0000:00:01.0/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda5 remove partition 8 5 block
hotplug /devices/platform/4010000000.pcie/pci0000:00/0000:00:01.0/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda3 add partition 8 3 block
hotplug /devices/platform/4010000000.pcie/pci0000:00/0000:00:01.0/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda4 remove partition 8 4 block
hotplug /devices/platform/4010000000.pcie/pci0000:00/0000:00:01.0/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda2 add partition 8 2 block
hotplug /devices/platform/4010000000.pcie/pci0000:00/0000:00:01.0/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda1 add partition 8 1 block
hotplug /devices/platform/4010000000.pcie/pci0000:00/0000:00:01.0/ata1/host0/target0:0:0/0:0:0:0/block/sda change disk 8 0 block
hotplug /devices/platform/4010000000.pcie/pci0000:00/0000:00:01.0/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda4 add partition 8 4 block




Signed-off-by: zhoumin <teczm@foxmail.com>
---
 lib/kobject_uevent.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index b7f2fa08d9c8..fd7b9d4c46c0 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -629,7 +629,7 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
 						 env->envp, GFP_KERNEL,
 						 NULL, cleanup_uevent_env, env);
 		if (info) {
-			retval = call_usermodehelper_exec(info, UMH_NO_WAIT);
+			retval = call_usermodehelper_exec(info, UMH_WAIT_EXEC);
 			env = NULL;	/* freed by cleanup_uevent_env */
 		}
 	}
-- 
2.43.0

Re: [RFC PATCH] kobject: fix uevent helper execution order issues

[Greg KH]

On Wed, Aug 20, 2025 at 01:18:28AM +0800, zhoumin wrote:
> The current use of the UMH_NO_WAIT parameter does not guarantee the
> sequential execution of CONFIG_STATIC_USERMODEHELPER_PATH.

There is never any such guarantee.  Unless you are on a single processor
system.

> For example, when fdisk completes and issues a BLKRRPART ioctl command,
> kernel first sending remove events followed by an add events. However, 
> process CONFIG_STATIC_USERMODEHELPER_PATH may execute
> out-of-order—potentially handling the add before the remove. This can
> result in the new partition being unexpectedly unmounted instead of mounted
> as intended.

Why are you not looking at the SEQNUM value?  That should be what orders
the events, right?  Otherwise how would any of this work?  :)

Also, what usermode helper program are you using that requires a binary?
What is wrong with udev?

thanks,

greg k-h

Re: [RFC PATCH] kobject: fix uevent helper execution order issues

[zhoumin]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1203 bytes --]

Hi Greg

Thanks for your patience.

> Why are you not looking at the SEQNUM value?  That should be what orders
> the events, right?  Otherwise how would any of this work?  :)

> Also, what usermode helper program are you using that requires a binary?
> What is wrong with udev?

Due to historical reasons, our system does not use udev and relies entirely
on custom shell scripts to handle kobject events.

The helper_lock and UMH_WAIT_EXEC in the kernel ensure that the uevent
helper serializes the execution of our shell scripts. That is why I
proposed the RFC patch to change UMH_NO_WAIT to UMH_WAIT_EXEC—for our use
case, this change provides clear benefits without downsides, aslo it avoids
blocking like UMH_WAIT_PROC while improving script execution order.

Regarding SEQNUM: using it would require parsing and tracking global
sequence numbers manually. In contrast, using a file lock would allow me to
focus only on serializing events for the same kobject, since events from
different objects don’t require synchronization.

If you believe this patch isn’t suitable for mainline, I’m open to
implementing a SEQNUM-based approach for better event ordering instead.

Best regards,
zhoumin

Re: [RFC PATCH] kobject: fix uevent helper execution order issues

[Greg KH]

On Wed, Aug 20, 2025 at 10:04:26PM +0800, zhoumin wrote:
> Hi Greg
> 
> Thanks for your patience.
> 
> > Why are you not looking at the SEQNUM value?  That should be what orders
> > the events, right?  Otherwise how would any of this work?  :)
> 
> > Also, what usermode helper program are you using that requires a binary?
> > What is wrong with udev?
> 
> Due to historical reasons, our system does not use udev and relies entirely
> on custom shell scripts to handle kobject events.

Given that udev is older than any other "historical reason" that feels
very odd.  udev was created at the same exact time the uevent api was
added, it's over 20 years old now.

> The helper_lock and UMH_WAIT_EXEC in the kernel ensure that the uevent
> helper serializes the execution of our shell scripts. That is why I
> proposed the RFC patch to change UMH_NO_WAIT to UMH_WAIT_EXEC—for our use
> case, this change provides clear benefits without downsides, aslo it avoids
> blocking like UMH_WAIT_PROC while improving script execution order.

But it does not guarantee what you are saying the real problem is here,
out-of-order events.

> Regarding SEQNUM: using it would require parsing and tracking global
> sequence numbers manually. In contrast, using a file lock would allow me to
> focus only on serializing events for the same kobject, since events from
> different objects don’t require synchronization.

SEQNUM is there for this very reason, to ignore it is wrong, and broken.
This is how the uevent system was designed to work.  Please fix your
userspace implementation, do not attempt to paper over the race in the
kernel, because as you say, it's not going to really solve it, only
reduce it.

And you will find that events from different objects WILL require
synchronization in many places, to ignore that is just going to cause
you even more problems.

> If you believe this patch isn’t suitable for mainline, I’m open to
> implementing a SEQNUM-based approach for better event ordering instead.

Please do so.  Even better yet, use udev!  Or mdev.  or one of the other
very old uevent handlers out there, you have plenty to choose from.

thanks,

greg k-h