linux-kernel - Re: [PATCH] kobject_uevent: add uevent

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <tencent_019E709405D8474A5D5A21E429D046331808@qq.com>
Date: Mon,  7 Apr 2025 23:18:12 +0800
From: zhoumin <teczm@...mail.com>
To: gregkh@...uxfoundation.org
Cc: dakr@...nel.org,
	akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org,
	rafael@...nel.org,
	teczm@...mail.com,
	viro@...iv.linux.org.uk
Subject: Re: [PATCH] kobject_uevent: add uevent_helper exist check

Hi Greg

I appreciate your patience in addressing this.

> > The kernel creates uevent_helper process for every uevent sent,
> > even if the uevent_helper does not exist. Before the rootfs is
> > mounted, a large number of events are generated. This change
> > introduces uevent_helper existence check to prevent unnecessary
> > operations.

> What problem is this causing? What changed to make this actually be a
problem?

I think calling uevent_helper before rootfs mount is pointless and waste
time, because uevent_helper does not exist at that stage. For example,
fs_initcall(chr_dev_init),subsys_initcall(usb_init) and etc, these module
run before rootfs_initcall and will trigger kobject_uevent when
uevent_helper isn't ready.

However, I've discovered this issue was already addressed by commit:
<b234ed6d629420827e2839c8c8935be85a0867fd> ("init: move
usermodehelper_enable() to populate_rootfs()")

Due to my device running an outdated kernel version, this change wasn't
immediately apparent to me.

But I propose we should make call_usermodehelper_exec fail earlier, link
this:
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -610,7 +610,7 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,

 #ifdef CONFIG_UEVENT_HELPER
        /* call uevent_helper, usually only enabled during early boot */
-       if (uevent_helper[0] && !kobj_usermode_filter(kobj)) {
+       if (uevent_helper[0] && !usermodehelper_disabled && !kobj_usermode_filter(kobj)) {
                struct subprocess_info *info;

                retval = add_uevent_var(env, "HOME=/");


If this is helpful, I can submit a follow-up patch.

> > Logs a debug messase before call_usermodehelper_setup.

> I can not parse this sentence, sorry.

What I'd say is to add such a line of debug logs.

> > e.g.: pr_emerg("action:%s devpath:%s\n", action_string, devpath);
> > You will see a large number of uevent_helper processes
> > are attempted to be created before the rootfs is mounted.

> Again, why is that a problem?

Like I said at the beginning.


> Signed-off-by: zhoumin

> Please use your name here, not an alias.

This is the Pinyin spelling of my Chinese name.

> ---
> lib/kobject_uevent.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
> index b7f2fa08d9c8..f3d34ded141a 100644
> --- a/lib/kobject_uevent.c
> +++ b/lib/kobject_uevent.c
> @@ -28,6 +28,7 @@
> #include
> #include
> #include
> +#include
>
>
> atomic64_t uevent_seqnum;
> @@ -58,6 +59,23 @@ static const char *kobject_actions[] = {
> [KOBJ_UNBIND] = "unbind",
> };
>
> +#ifdef CONFIG_UEVENT_HELPER
> +static int uevent_helper_lookup(void)
> +{
> + static int ret = -ENOENT;
> + struct path path;
> +
> + if (!ret)
> + return ret;
> +
> + ret = kern_path(uevent_helper, LOOKUP_FOLLOW, &path);
> + if (!ret)
> + path_put(&path);

> What happens when the root filesystem changes to the new one? This
feels wrong as Andrew said.

If a user performs actions like chroot into a new rootfs that lacks
uevent_helper, this should be considered a rootfs configuration error
rather than a kernel issue. The kernel shouldn't handle missing helpers
at this stage.

Apply to the question of another mail.
Link: https://lore.kernel.org/lkml/2025040731-eternity-statutory-4a80@gregkh/


> > > Is there any measurable reduction in boot time?
> >
> > This depends on the number of uevent and the performance of the device.
> > In fact, I found this issue during a project to optomize boot time,
> > and on my embedded device, it can be optimized by at least 2 seconds.
 
> So you are spending 2 seconds in failing to lookup the uevent helper
> path? That feels really wrong, how long does your boot take overall?
> What % is 2 seconds?

The measurements were taken on a device running kernel 4.4.65 using
/proc/uptime. The 129s average boot time served as baseline for comparing
the time difference before and after the changes, though this metric is no
longer the primary concern.

Would you like me to submit a formal patch with above changes.


Best regards,

zhoumin