All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 0/1] Delay module uevent until after initialization
@ 2020-12-03 13:51 Jessica Yu
  2020-12-03 13:51 ` [PATCH RFC 1/1] module: delay kobject uevent until after module init call Jessica Yu
  0 siblings, 1 reply; 5+ messages in thread
From: Jessica Yu @ 2020-12-03 13:51 UTC (permalink / raw)
  To: linux-kernel, systemd-devel
  Cc: Nicolas Morey-Chaisemartin, Franck Bui, Jessica Yu

Hi,

This patch addresses a race condition between udev and the module loader
that was recently described here:

    https://github.com/systemd/systemd/issues/17586

Currently, the module loader issues a KOBJ_ADD uevent before it calls a
module's initialization function. Some mount units expect that the module
has initialized already upon receiving the uevent. For instance, the
configfs module creates the /sys/kernel/config mount point during its init
function, but the module loader issues the uevent before the init function
is called. If udev processes the uevent before the module loader calls the
init function, then the mount unit will fail, since /sys/kernel/config will
not exist yet.

Nicolas Morey-Chaisemartin provided a simple test script to trigger the
race condition:

while true; do
        umount configfs
        rmmod configfs
        sleep 1
        modprobe configfs
        ls -alFd /sys/kernel/config
        sleep 1
        systemctl status sys-kernel-config.mount | tail -n 1
done

When the mount fails due to the race condition, you would see a message like:

systemd[1]: Condition check resulted in Kernel Configuration File System being skipped.

I ran the script for about 30 minutes on my own machine and managed to trigger
the failure condition 4 times. With the patch applied, I was not able to
trigger the failed condition anymore after running the script for the same
amount of time. Nicolas also reported similar test results after testing a
kernel with the patch applied.

This is sent first as an RFC to both LKML and systemd mailing lists since
the uevent call has been like this in the module loader for more than a
decade (since v2.6), I would be cautious as to not break anything that
actually relies on the current behavior for whatever reason. More testing
would be greatly appreciated.

Thanks,

Jessica

Jessica Yu (1):
  module: delay kobject uevent until after module init call

 kernel/module.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH RFC 1/1] module: delay kobject uevent until after module init call
  2020-12-03 13:51 [PATCH RFC 0/1] Delay module uevent until after initialization Jessica Yu
@ 2020-12-03 13:51 ` Jessica Yu
  2020-12-03 14:42   ` Nicolas Morey-Chaisemartin
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Jessica Yu @ 2020-12-03 13:51 UTC (permalink / raw)
  To: linux-kernel, systemd-devel
  Cc: Nicolas Morey-Chaisemartin, Franck Bui, Jessica Yu

Apparently there has been a longstanding race between udev/systemd and
the module loader. Currently, the module loader sends a uevent right
after sysfs initialization, but before the module calls its init
function. However, some udev rules expect that the module has
initialized already upon receiving the uevent.

This race has been triggered recently (see link in references) in some
systemd mount unit files. For instance, the configfs module creates the
/sys/kernel/config mount point in its init function, however the module
loader issues the uevent before this happens. sys-kernel-config.mount
expects to be able to mount /sys/kernel/config upon receipt of the
module loading uevent, but if the configfs module has not called its
init function yet, then this directory will not exist and the mount unit
fails. A similar situation exists for sys-fs-fuse-connections.mount, as
the fuse sysfs mount point is created during the fuse module's init
function. If udev is faster than module initialization then the mount
unit would fail in a similar fashion.

To fix this race, delay the module KOBJ_ADD uevent until after the
module has finished calling its init routine.

References: https://github.com/systemd/systemd/issues/17586
Signed-off-by: Jessica Yu <jeyu@kernel.org>
---
 kernel/module.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index a40ec708f8f2..e1dd0df57244 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1897,7 +1897,6 @@ static int mod_sysfs_init(struct module *mod)
 	if (err)
 		mod_kobject_put(mod);
 
-	/* delay uevent until full sysfs population */
 out:
 	return err;
 }
@@ -1934,7 +1933,6 @@ static int mod_sysfs_setup(struct module *mod,
 	add_sect_attrs(mod, info);
 	add_notes_attrs(mod, info);
 
-	kobject_uevent(&mod->mkobj.kobj, KOBJ_ADD);
 	return 0;
 
 out_unreg_modinfo_attrs:
@@ -3656,6 +3654,9 @@ static noinline int do_init_module(struct module *mod)
 	blocking_notifier_call_chain(&module_notify_list,
 				     MODULE_STATE_LIVE, mod);
 
+	/* Delay uevent until module has finished its init routine */
+	kobject_uevent(&mod->mkobj.kobj, KOBJ_ADD);
+
 	/*
 	 * We need to finish all async code before the module init sequence
 	 * is done.  This has potential to deadlock.  For example, a newly
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH RFC 1/1] module: delay kobject uevent until after module init call
  2020-12-03 13:51 ` [PATCH RFC 1/1] module: delay kobject uevent until after module init call Jessica Yu
@ 2020-12-03 14:42   ` Nicolas Morey-Chaisemartin
  2020-12-03 18:32   ` [systemd-devel] " Greg KH
  2020-12-10 12:39   ` Jessica Yu
  2 siblings, 0 replies; 5+ messages in thread
From: Nicolas Morey-Chaisemartin @ 2020-12-03 14:42 UTC (permalink / raw)
  To: Jessica Yu, linux-kernel, systemd-devel; +Cc: Franck Bui

I tested this on a system that had the script in patch#0 fail 100% of the time.
With this patch, I haven't seen any failure in a few hours.
Haven't seen any unexpected side effect either.

Tested-By: Nicolas Morey-Chaisemartin <nmoreychaisemartin@suse.com>

On 12/3/20 2:51 PM, Jessica Yu wrote:
> Apparently there has been a longstanding race between udev/systemd and
> the module loader. Currently, the module loader sends a uevent right
> after sysfs initialization, but before the module calls its init
> function. However, some udev rules expect that the module has
> initialized already upon receiving the uevent.
> 
> This race has been triggered recently (see link in references) in some
> systemd mount unit files. For instance, the configfs module creates the
> /sys/kernel/config mount point in its init function, however the module
> loader issues the uevent before this happens. sys-kernel-config.mount
> expects to be able to mount /sys/kernel/config upon receipt of the
> module loading uevent, but if the configfs module has not called its
> init function yet, then this directory will not exist and the mount unit
> fails. A similar situation exists for sys-fs-fuse-connections.mount, as
> the fuse sysfs mount point is created during the fuse module's init
> function. If udev is faster than module initialization then the mount
> unit would fail in a similar fashion.
> 
> To fix this race, delay the module KOBJ_ADD uevent until after the
> module has finished calling its init routine.
> 
> References: https://github.com/systemd/systemd/issues/17586
> Signed-off-by: Jessica Yu <jeyu@kernel.org>
> ---
>  kernel/module.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index a40ec708f8f2..e1dd0df57244 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1897,7 +1897,6 @@ static int mod_sysfs_init(struct module *mod)
>  	if (err)
>  		mod_kobject_put(mod);
>  
> -	/* delay uevent until full sysfs population */
>  out:
>  	return err;
>  }
> @@ -1934,7 +1933,6 @@ static int mod_sysfs_setup(struct module *mod,
>  	add_sect_attrs(mod, info);
>  	add_notes_attrs(mod, info);
>  
> -	kobject_uevent(&mod->mkobj.kobj, KOBJ_ADD);
>  	return 0;
>  
>  out_unreg_modinfo_attrs:
> @@ -3656,6 +3654,9 @@ static noinline int do_init_module(struct module *mod)
>  	blocking_notifier_call_chain(&module_notify_list,
>  				     MODULE_STATE_LIVE, mod);
>  
> +	/* Delay uevent until module has finished its init routine */
> +	kobject_uevent(&mod->mkobj.kobj, KOBJ_ADD);
> +
>  	/*
>  	 * We need to finish all async code before the module init sequence
>  	 * is done.  This has potential to deadlock.  For example, a newly
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [systemd-devel] [PATCH RFC 1/1] module: delay kobject uevent until after module init call
  2020-12-03 13:51 ` [PATCH RFC 1/1] module: delay kobject uevent until after module init call Jessica Yu
  2020-12-03 14:42   ` Nicolas Morey-Chaisemartin
@ 2020-12-03 18:32   ` Greg KH
  2020-12-10 12:39   ` Jessica Yu
  2 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2020-12-03 18:32 UTC (permalink / raw)
  To: Jessica Yu
  Cc: linux-kernel, systemd-devel, Franck Bui, Nicolas Morey-Chaisemartin

On Thu, Dec 03, 2020 at 02:51:24PM +0100, Jessica Yu wrote:
> Apparently there has been a longstanding race between udev/systemd and
> the module loader. Currently, the module loader sends a uevent right
> after sysfs initialization, but before the module calls its init
> function. However, some udev rules expect that the module has
> initialized already upon receiving the uevent.
> 
> This race has been triggered recently (see link in references) in some
> systemd mount unit files. For instance, the configfs module creates the
> /sys/kernel/config mount point in its init function, however the module
> loader issues the uevent before this happens. sys-kernel-config.mount
> expects to be able to mount /sys/kernel/config upon receipt of the
> module loading uevent, but if the configfs module has not called its
> init function yet, then this directory will not exist and the mount unit
> fails. A similar situation exists for sys-fs-fuse-connections.mount, as
> the fuse sysfs mount point is created during the fuse module's init
> function. If udev is faster than module initialization then the mount
> unit would fail in a similar fashion.
> 
> To fix this race, delay the module KOBJ_ADD uevent until after the
> module has finished calling its init routine.
> 
> References: https://github.com/systemd/systemd/issues/17586
> Signed-off-by: Jessica Yu <jeyu@kernel.org>

Nice fix:
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH RFC 1/1] module: delay kobject uevent until after module init call
  2020-12-03 13:51 ` [PATCH RFC 1/1] module: delay kobject uevent until after module init call Jessica Yu
  2020-12-03 14:42   ` Nicolas Morey-Chaisemartin
  2020-12-03 18:32   ` [systemd-devel] " Greg KH
@ 2020-12-10 12:39   ` Jessica Yu
  2 siblings, 0 replies; 5+ messages in thread
From: Jessica Yu @ 2020-12-10 12:39 UTC (permalink / raw)
  To: linux-kernel, systemd-devel; +Cc: Nicolas Morey-Chaisemartin, Franck Bui

+++ Jessica Yu [03/12/20 14:51 +0100]:
>Apparently there has been a longstanding race between udev/systemd and
>the module loader. Currently, the module loader sends a uevent right
>after sysfs initialization, but before the module calls its init
>function. However, some udev rules expect that the module has
>initialized already upon receiving the uevent.
>
>This race has been triggered recently (see link in references) in some
>systemd mount unit files. For instance, the configfs module creates the
>/sys/kernel/config mount point in its init function, however the module
>loader issues the uevent before this happens. sys-kernel-config.mount
>expects to be able to mount /sys/kernel/config upon receipt of the
>module loading uevent, but if the configfs module has not called its
>init function yet, then this directory will not exist and the mount unit
>fails. A similar situation exists for sys-fs-fuse-connections.mount, as
>the fuse sysfs mount point is created during the fuse module's init
>function. If udev is faster than module initialization then the mount
>unit would fail in a similar fashion.
>
>To fix this race, delay the module KOBJ_ADD uevent until after the
>module has finished calling its init routine.
>
>References: https://github.com/systemd/systemd/issues/17586
>Signed-off-by: Jessica Yu <jeyu@kernel.org>

Thanks all, this has been applied to modules-next to try to get as
much -next time as possible before the upcoming merge window.

Jessica


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-12-10 12:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-03 13:51 [PATCH RFC 0/1] Delay module uevent until after initialization Jessica Yu
2020-12-03 13:51 ` [PATCH RFC 1/1] module: delay kobject uevent until after module init call Jessica Yu
2020-12-03 14:42   ` Nicolas Morey-Chaisemartin
2020-12-03 18:32   ` [systemd-devel] " Greg KH
2020-12-10 12:39   ` Jessica Yu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.