linux-kernel - [PATCH v2] module: print module name on refcount error

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20230717171428.1b229215@endymion.delvare>
Date:   Mon, 17 Jul 2023 17:14:28 +0200
From:   Jean Delvare <jdelvare@...e.de>
To:     Luis Chamberlain <mcgrof@...nel.org>
Cc:     Michal Hocko <mhocko@...e.com>, linux-modules@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: [PATCH v2] module: print module name on refcount error

If module_put() triggers a refcount error, and the module data is
still readable, include the culprit module name in the warning
message, to easy further investigation of the issue.

If the module name can't be read, this means the module has already
been removed while references to it still exist. This is a
user-after-free situation, so report it as such.

Signed-off-by: Jean Delvare <jdelvare@...e.de>
Suggested-by: Michal Hocko <mhocko@...e.com>
Cc: Luis Chamberlain <mcgrof@...nel.org>
---
Hi Luis, this is a different approach to my initial proposal. We no
longer assume that struct module is still available and instead check
that the expected module name string is a valid string before printing
it.

This is safer, and lets us print a better diagnostics message: include
the module name if struct module is still there (the most likely case
IMHO, as rmmod is a relatively rare operation) else explicitly report a
use after free.

The downside is that this requires more code, but I think it's worth
it. What do you think?

 kernel/module/main.c |   31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

--- linux-6.3.orig/kernel/module/main.c
+++ linux-6.3/kernel/module/main.c
@@ -55,6 +55,7 @@
 #include <linux/dynamic_debug.h>
 #include <linux/audit.h>
 #include <linux/cfi.h>
+#include <linux/ctype.h>
 #include <uapi/linux/module.h>
 #include "internal.h"
 
@@ -850,7 +851,35 @@ void module_put(struct module *module)
 	if (module) {
 		preempt_disable();
 		ret = atomic_dec_if_positive(&module->refcnt);
-		WARN_ON(ret < 0);	/* Failed to put refcount */
+		if (ret < 0) {
+			unsigned char modname_copy[MODULE_NAME_LEN];
+			unsigned char *p, *end;
+			bool sane;
+
+			/*
+			 * Report faulty module if name is still readable.
+			 * We must be careful here as the module may have
+			 * been already freed.
+			 */
+			memcpy(modname_copy, module->name, MODULE_NAME_LEN);
+			end = memchr(modname_copy, '\0', MODULE_NAME_LEN);
+			sane = end != NULL;
+			if (sane) {
+				for (p = modname_copy; p < end; p++)
+					if (!isgraph(*p)) {
+						sane = false;
+						break;
+					}
+			}
+
+			if (sane)
+				WARN(1,
+				     KERN_WARNING "Failed to put refcount for module %s\n",
+				     modname_copy);
+			else
+				WARN(1,
+				     KERN_WARNING "Failed to put refcount, use-after-free detected\n");
+		}
 		trace_module_put(module, _RET_IP_);
 		preempt_enable();
 	}


-- 
Jean Delvare
SUSE L3 Support