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

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: <20250918201109.24620-10-julian.lagattuta@gmail.com>
Date: Thu, 18 Sep 2025 16:11:12 -0400
From: julian-lagattuta <julian.lagattuta@...il.com>
To: Luis Chamberlain <mcgrof@...nel.org>,
	Petr Pavlu <petr.pavlu@...e.com>
Cc: Sami Tolvanen <samitolvanen@...gle.com>,
	Daniel Gomez <da.gomez@...sung.com>,
	linux-modules@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	julian-lagattuta <julian.lagattuta@...il.com>
Subject: [PATCH 4/6] module: move idempotent allocation to heap

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@...il.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