[PATCH 0/6] module: enable force unloading of modules that have crashed during init

[PATCH 0/6] module: enable force unloading of modules that have crashed during init

[julian-lagattuta]

Running a module that encounters a fatal error during the initcall leaves
the module in a state where it cannot be forcefully unloaded since it is 
"being used" despite there being no reason it couldn't be unloaded. 
This means that unloading the crashed module requires rebooting.

This patch allows modules that have crashed during initialization to be
forcefully unloaded with CONFIG_MODULE_FORCE_UNLOAD enabled.


Here are the costs:
 - 2 extra pointers stored in struct module (if CONFIG_MODULE_FORCE_UNLOAD is enabled)
 - struct idempotent is allocated on heap instead of stack (regardless of config)
 
I had to make a design decision for cases where another (f)init_module is
waiting for the crashed module to finish and delete_module is called on it.

Here is an example of the behavior my patch causes:
> insmod crash.ko # insmod calls finit_module and crash.ko runs 0/0 in init
Segmentation Fault
> insmod crash.ko & # insmod will hang forever since it waits for cash.ko init to finish
> rmmod -f crash
[1]+  Exit 1                  insmod crash.ko
insmod: ERROR: could not insert module crash.ko: Device or resource busy

Here, anyone waiting for init to finish will receive -EBUSY upon removal. 
This is true for finit_module and init_module syscalls.
I chose -EBUSY since it means I don't need to modify module_patient_check_exists.
error -ECANCELED might work better

P.S. This is my first patch so I'm new to this.

Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
julian-lagattuta (6):
  module: store init_pid and idempotent in module
  module: detect if init crashed and unload
  module: move freeinit allocation to avoid memory leak
  module: move idempotent allocation to heap
  module: store and complete idempotent upon force unloading
  module: comment to describe a new codepath

 include/linux/module.h   |   4 ++
 kernel/module/internal.h |   3 ++
 kernel/module/main.c     | 112 +++++++++++++++++++++++++++++----------
 3 files changed, 92 insertions(+), 27 deletions(-)

-- 
2.45.2

[PATCH 1/6] module: store init_pid and idempotent in module

[julian-lagattuta]

Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
 include/linux/module.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/module.h b/include/linux/module.h
index 5faa1fb1f4b4..5ac5f4992fe8 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -599,6 +599,10 @@ struct module {
 #ifdef CONFIG_DYNAMIC_DEBUG_CORE
 	struct _ddebug_info dyndbg_info;
 #endif
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+	struct pid *init_pid;
+	struct idempotent *idempotent;
+#endif
 } ____cacheline_aligned __randomize_layout;
 #ifndef MODULE_ARCH_INIT
 #define MODULE_ARCH_INIT {}
-- 
2.45.2

[PATCH 2/6] module: detect if init crashed and unload

[julian-lagattuta]

Store idempotent and init_pid in struct module.

Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
 kernel/module/main.c | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 413ac6ea3702..2277c53aef2e 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -752,6 +752,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 	char name[MODULE_NAME_LEN];
 	char buf[MODULE_FLAGS_BUF_SIZE];
 	int ret, forced = 0;
+	bool did_init_crash __maybe_unused = false;
 
 	if (!capable(CAP_SYS_MODULE) || modules_disabled)
 		return -EPERM;
@@ -778,8 +779,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 	}
 
 	/* Doing init or already dying? */
-	if (mod->state != MODULE_STATE_LIVE) {
-		/* FIXME: if (force), slam module count damn the torpedoes */
+	if (mod->state == MODULE_STATE_GOING ||
+		(mod->state != MODULE_STATE_LIVE &&
+		!IS_ENABLED(CONFIG_MODULE_FORCE_UNLOAD))
+	) {
+		if (mod->state == MODULE_STATE_GOING)
 		pr_debug("%s already dying\n", mod->name);
 		ret = -EBUSY;
 		goto out;
@@ -795,6 +799,21 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 		}
 	}
 
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+	if (mod->state == MODULE_STATE_COMING) {
+		struct task_struct *init_process = get_pid_task(mod->init_pid, PIDTYPE_PID);
+
+		/* Did the init process die? */
+		if (init_process) {
+			put_task_struct(init_process);
+			ret = -EBUSY;
+			goto out;
+		} else {
+			did_init_crash = true;
+		}
+	}
+#endif
+
 	ret = try_stop_module(mod, flags, &forced);
 	if (ret != 0)
 		goto out;
@@ -1380,6 +1399,10 @@ static void free_module(struct module *mod)
 	mod->state = MODULE_STATE_UNFORMED;
 	mutex_unlock(&module_mutex);
 
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+	if (mod->init_pid)
+		put_pid(mod->init_pid);
+#endif
 	/* Arch-specific cleanup. */
 	module_arch_cleanup(mod);
 
@@ -3044,6 +3067,11 @@ static noinline int do_init_module(struct module *mod)
 	ftrace_free_mem(mod, mod->mem[MOD_INIT_TEXT].base,
 			mod->mem[MOD_INIT_TEXT].base + mod->mem[MOD_INIT_TEXT].size);
 	mutex_lock(&module_mutex);
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+	put_pid(mod->init_pid);
+	mod->init_pid = NULL;
+	mod->idempotent = NULL;
+#endif
 	/* Drop initial reference. */
 	module_put(mod);
 	trim_init_extable(mod);
@@ -3474,6 +3502,10 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	if (codetag_load_module(mod))
 		goto sysfs_cleanup;
 
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+	mod->init_pid = get_pid(task_pid(current));
+	mod->idempotent = info->idempotent;
+#endif
 	/* Get rid of temporary copy. */
 	free_copy(info, flags);
 
-- 
2.45.2

[PATCH 3/6] module: move freeinit allocation to avoid memory leak

[julian-lagattuta]

move freeinit allocation after do_one_initcall in case do_one_initcall crashes.
Otherwise, freeinit would leak memory after every initalization of a crashed module.
I could not find a reason for why freeinit allocation happened before do_one_initcall.

Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
 kernel/module/main.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 2277c53aef2e..2825ac177c62 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3021,21 +3021,13 @@ static noinline int do_init_module(struct module *mod)
 	}
 #endif
 
-	freeinit = kmalloc(sizeof(*freeinit), GFP_KERNEL);
-	if (!freeinit) {
-		ret = -ENOMEM;
-		goto fail;
-	}
-	freeinit->init_text = mod->mem[MOD_INIT_TEXT].base;
-	freeinit->init_data = mod->mem[MOD_INIT_DATA].base;
-	freeinit->init_rodata = mod->mem[MOD_INIT_RODATA].base;
 
 	do_mod_ctors(mod);
 	/* Start the module */
 	if (mod->init != NULL)
 		ret = do_one_initcall(mod->init);
 	if (ret < 0) {
-		goto fail_free_freeinit;
+		goto fail;
 	}
 	if (ret > 0) {
 		pr_warn("%s: '%s'->init suspiciously returned %d, it should "
@@ -3045,6 +3037,14 @@ static noinline int do_init_module(struct module *mod)
 		dump_stack();
 	}
 
+	freeinit = kmalloc(sizeof(*freeinit), GFP_KERNEL);
+	if (!freeinit) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+	freeinit->init_text = mod->mem[MOD_INIT_TEXT].base;
+	freeinit->init_data = mod->mem[MOD_INIT_DATA].base;
+	freeinit->init_rodata = mod->mem[MOD_INIT_RODATA].base;
 	/* Now it's a first class citizen! */
 	mod->state = MODULE_STATE_LIVE;
 	blocking_notifier_call_chain(&module_notify_list,
@@ -3123,8 +3123,6 @@ static noinline int do_init_module(struct module *mod)
 
 	return 0;
 
-fail_free_freeinit:
-	kfree(freeinit);
 fail:
 	/* Try to protect us from buggy refcounters. */
 	mod->state = MODULE_STATE_GOING;
-- 
2.45.2

[PATCH 4/6] module: move idempotent allocation to heap

[julian-lagattuta]

struct idempotent is normally allocated on the stack.
If init crashes during a finit_module syscall and then the module is unloaded,
reloading the module causes issues. This is because the struct idempotent stored
in the list becomes stale after the crash.

Therefore idempotent is stored on the heap and placed inside the struct module 
to be completed by delete_module.

I am open to the idea of only storing it in the heap when 
CONFIG_MODULE_FORCE_UNLOAD is enabled; however, simple seemed better.

Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
 kernel/module/main.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 2825ac177c62..217185dbc3c1 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3650,6 +3650,8 @@ static int idempotent_complete(struct idempotent *u, int ret)
 		complete(&pos->complete);
 	}
 	spin_unlock(&idem_lock);
+
+	kfree(u);
 	return ret;
 }
 
@@ -3666,13 +3668,19 @@ static int idempotent_complete(struct idempotent *u, int ret)
  */
 static int idempotent_wait_for_completion(struct idempotent *u)
 {
+	int ret;
+
 	if (wait_for_completion_interruptible(&u->complete)) {
 		spin_lock(&idem_lock);
 		if (!hlist_unhashed(&u->entry))
 			hlist_del(&u->entry);
 		spin_unlock(&idem_lock);
 	}
-	return u->ret;
+	ret = u->ret;
+
+	kfree(u);
+
+	return ret;
 }
 
 static int init_module_from_file(struct file *f, const char __user * uargs, int flags)
@@ -3705,21 +3713,26 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
 
 static int idempotent_init_module(struct file *f, const char __user * uargs, int flags)
 {
-	struct idempotent idem;
+	struct idempotent *idem;
+
+	idem = kmalloc(sizeof(*idem), GFP_KERNEL);
+	if (!idem)
+		return -ENOMEM;
+
 
 	if (!(f->f_mode & FMODE_READ))
 		return -EBADF;
 
 	/* Are we the winners of the race and get to do this? */
-	if (!idempotent(&idem, file_inode(f))) {
-		int ret = init_module_from_file(f, uargs, flags);
-		return idempotent_complete(&idem, ret);
+	if (!idempotent(idem, file_inode(f))) {
+		int ret = init_module_from_file(f, uargs, flags, idem);
+		return idempotent_complete(idem, ret);
 	}
 
 	/*
 	 * Somebody else won the race and is loading the module.
 	 */
-	return idempotent_wait_for_completion(&idem);
+	return idempotent_wait_for_completion(idem);
 }
 
 SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
-- 
2.45.2

[PATCH 5/6] module: store and complete idempotent upon force unloading

[julian-lagattuta]

Move idempotent definition up and add idempotent_complete declaration.

Add idempotent to struct load_info which gets passed into load_module
and then stored in the struct module.

run idempotent_complete after module is unloaded and give EBUSY
to any process waiting for the module to finish initializing
via finit_module.

Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
 kernel/module/internal.h |  3 +++
 kernel/module/main.c     | 29 +++++++++++++++++++++--------
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 8d74b0a21c82..43f537475859 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -89,6 +89,9 @@ struct load_info {
 		unsigned int vers_ext_crc;
 		unsigned int vers_ext_name;
 	} index;
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+	struct idempotent* idempotent;
+#endif
 };
 
 enum mod_license {
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 217185dbc3c1..256e30259bcf 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -80,6 +80,17 @@ static void do_free_init(struct work_struct *w);
 static DECLARE_WORK(init_free_wq, do_free_init);
 static LLIST_HEAD(init_free_list);
 
+
+struct idempotent {
+	const void *cookie;
+	struct hlist_node entry;
+	struct completion complete;
+	int ret;
+};
+
+static int idempotent_complete(struct idempotent *u, int ret);
+
+
 struct mod_tree_root mod_tree __cacheline_aligned = {
 	.addr_min = -1UL,
 };
@@ -784,7 +795,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 		!IS_ENABLED(CONFIG_MODULE_FORCE_UNLOAD))
 	) {
 		if (mod->state == MODULE_STATE_GOING)
-		pr_debug("%s already dying\n", mod->name);
+			pr_debug("%s already dying\n", mod->name);
 		ret = -EBUSY;
 		goto out;
 	}
@@ -833,6 +844,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 	strscpy(last_unloaded_module.name, mod->name);
 	strscpy(last_unloaded_module.taints, module_flags(mod, buf, false));
 
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+	if (did_init_crash && mod->idempotent)
+		idempotent_complete(mod->idempotent, -EBUSY);
+#endif
+
 	free_module(mod);
 	/* someone could wait for the module in add_unformed_module() */
 	wake_up_all(&module_wq);
@@ -3591,12 +3607,6 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
 	return load_module(&info, uargs, 0);
 }
 
-struct idempotent {
-	const void *cookie;
-	struct hlist_node entry;
-	struct completion complete;
-	int ret;
-};
 
 #define IDEM_HASH_BITS 8
 static struct hlist_head idem_hash[1 << IDEM_HASH_BITS];
@@ -3683,7 +3693,7 @@ static int idempotent_wait_for_completion(struct idempotent *u)
 	return ret;
 }
 
-static int init_module_from_file(struct file *f, const char __user * uargs, int flags)
+static int init_module_from_file(struct file *f, const char __user * uargs, int flags, struct idempotent *idempotent __maybe_unused)
 {
 	struct load_info info = { };
 	void *buf = NULL;
@@ -3707,6 +3717,9 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
 		info.hdr = buf;
 		info.len = len;
 	}
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+	info.idempotent = idempotent;
+#endif
 
 	return load_module(&info, uargs, flags);
 }
-- 
2.45.2

[PATCH 6/6] module: comment describing new codepath

[julian-lagattuta]

added comment so future authors know about this codepath

Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
 kernel/module/main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 256e30259bcf..f4ce431163fa 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3220,6 +3220,11 @@ static int module_patient_check_exists(const char *name,
         */
        if (old && old->state == MODULE_STATE_LIVE)
                return -EEXIST;
+
+       /* 
+        * Can occur if the module was forcefully unloaded after
+        * its initcall crashed.
+       */
        return -EBUSY;
 }
 
-- 
2.45.2

Re: [PATCH 0/6] module: enable force unloading of modules that have crashed during init

[Petr Pavlu]

On 9/18/25 10:11 PM, julian-lagattuta wrote:
> Running a module that encounters a fatal error during the initcall leaves
> the module in a state where it cannot be forcefully unloaded since it is 
> "being used" despite there being no reason it couldn't be unloaded. 
> This means that unloading the crashed module requires rebooting.
> 
> This patch allows modules that have crashed during initialization to be
> forcefully unloaded with CONFIG_MODULE_FORCE_UNLOAD enabled.

Could you please explain the motivation for doing this in more detail?

I think we shouldn't attempt to do anything clever with modules that
crashed during initialization. Such a module can already leave the
system in an unstable state and trying to recover can cause even more
problems. For instance, I don't see how it is safe to call the module's
exit function.

-- 
Thanks,
Petr

Re: [PATCH 0/6] module: enable force unloading of modules that have crashed during init

[Julian LaGattuta]

> Could you please explain the motivation for doing this in more detail?
>
> I think we shouldn't attempt to do anything clever with modules that
> crashed during initialization. Such a module can already leave the
> system in an unstable state and trying to recover can cause even more
> problems. For instance, I don't see how it is safe to call the module's
> exit function.
>
> --
> Thanks,
> Petr

Thank you for your response Petr. The motivation comes from when I
wanted to replace a crashed module with one which does not crash
without having to reboot. I looked around and saw some other people
complain about it on stackoverflow.

I thought that if a module crashed during init, it would be in a no
better position compared to if it were forcefully removed.
Therefore, there is no reason why this shouldn't be an option as it
couldn't make the problem worse.

I agree that calling the exit function doesn't make sense and so I
could change the behavior.

That being said, I understand why someone would be wary of this type
of change; this is just my thought process.

Sincerely,
Julian

Re: [PATCH 0/6] module: enable force unloading of modules that have crashed during init

[Petr Pavlu]

On 9/25/25 12:16 AM, Julian LaGattuta wrote:
>> Could you please explain the motivation for doing this in more detail?
>>
>> I think we shouldn't attempt to do anything clever with modules that
>> crashed during initialization. Such a module can already leave the
>> system in an unstable state and trying to recover can cause even more
>> problems. For instance, I don't see how it is safe to call the module's
>> exit function.
> 
> Thank you for your response Petr. The motivation comes from when I
> wanted to replace a crashed module with one which does not crash
> without having to reboot. I looked around and saw some other people
> complain about it on stackoverflow.

Hm, I'm still not sure I understand the use case. If it is about being
able to remove a crashed module when developing it, then I wouldn't
expect rebooting the machine to be much of an issue. If it is on the
other hand about removing it on a production machine, then I think
attempting this can leave the machine in a worse state and not something
we should encourage or support.

> 
> I thought that if a module crashed during init, it would be in a no
> better position compared to if it were forcefully removed.
> Therefore, there is no reason why this shouldn't be an option as it
> couldn't make the problem worse.

A module can be halfway through its initialization when it crashes. It
may have already registered with various parts of the kernel and
I believe that removing the module from under the kernel's control could
result in even more problems.

The current support for forcefully removing a module overrides the
kernel's tracking of module references. This option was originally
introduced by "[PATCH] Forced module unload" [1]. As far as I can see,
it was related to the module loader rework at that time in "[PATCH]
In-kernel Module Loader" [2]. This rework provided raceless
loading/unloading and marked several MOD_INC_USE_COUNT/MOD_DEC_USE_COUNT
interfaces as obsolete and unsafe. Since some modules still used the old
racy interfaces, it seems the forced removal option was added to make it
possible to remove such modules.

However, this issue should have been fixed a long time ago, so I wonder
if even the current CONFIG_MODULE_FORCE_UNLOAD support is useful.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/mpe/linux-fullhistory.git/commit/?id=d0f8c9a4c2c9d93463d157248c73028670e80a97
[2] https://git.kernel.org/pub/scm/linux/kernel/git/mpe/linux-fullhistory.git/commit/?id=4c877b08daf4b463c144cbd2748ed1659931a0dd

-- 
Thanks,
Petr

Re: [PATCH 0/6] module: enable force unloading of modules that have crashed during init

[Julian LaGattuta]

Thank you so much for your well written reply. I understand now where
I went wrong.
Have a good rest of your day.

Sincerely,
Julian