Linux Kernel module notification bug

From: Adam Zabrocki <pi3@pi3.com.pl>
To: Jessica Yu <jeyu@kernel.org>,
	Nicolas Morey-Chaisemartin <nmoreychaisemartin@suse.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org
Cc: Solar Designer <solar@openwall.com>
Subject: Linux Kernel module notification bug
Date: Sun, 10 Jan 2021 18:54:01 +0100	[thread overview]
Message-ID: <20210110175401.GB32505@pi3.com.pl> (raw)

Hello,

I believe that the following commit does introduce a gentle "functionality 
bug":

"module: delay kobject uevent until after module init call":
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/kernel/module.c?id=38dc717e97153e46375ee21797aa54777e5498f3

The official Linux Kernel API for the kernel module activities notification has
been divided based on the readiness 'stage' for such module. We have the
following stages:

        MODULE_STATE_LIVE,      /* Normal state. */
        MODULE_STATE_COMING,    /* Full formed, running module_init. */
        MODULE_STATE_GOING,     /* Going away. */
        MODULE_STATE_UNFORMED,  /* Still setting it up. */

LIVE means that the kernel module is correctly running and all initialization
work has been already done. Otherwise, we have event 'COMING' or 'UNFORMED'.

In the described commit, creation of the KOBJECT has been moved after invoking
a notficiation of the newly formed kernel module state (LIVE). That's somehow
inconsistent from my understanding of the kernel modules notifiers logic.
Creation of the new objects (like KOBJ) should be done before notification of
the stage LIVE is invoked.

This commit breaks some of the projects that rely on the LIVE notification to
monitor when the newly loaded module is ready.

I believe that the latest time when 'kobject_uevent' function can be called is
just before invoking 'blocking_notifier_call_chain', e.g:

diff --git a/kernel/module.c b/kernel/module.c
index 4bf30e4b3eaa..7d56b1b07237 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3681,14 +3681,14 @@ static noinline int do_init_module(struct module *mod)
                dump_stack();
        }

+       /* Delay uevent until module has finished its init routine */
+       kobject_uevent(&mod->mkobj.kobj, KOBJ_ADD);
+
        /* Now it's a first class citizen! */
        mod->state = MODULE_STATE_LIVE;
        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

Thanks,
Adam

--
pi3 (pi3ki31ny) - pi3 (at) itsec pl
http://pi3.com.pl

