linux-kernel - [PATCH 3/3] module: don't ignore vermagic string if module doesn't have modversions

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: <200805091625.28705.rusty@rustcorp.com.au>
Date:	Fri, 9 May 2008 16:25:28 +1000
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	linux-kernel@...r.kernel.org
Cc:	Jon Masters <jonathan@...masters.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: [PATCH 3/3] module: don't ignore vermagic string if module doesn't have modversions

Linus found a logic bug: we ignore the version number in a module's
vermagic string if we have CONFIG_MODVERSIONS set, but modversions
also lets through a module with no __versions section for modprobe
--force (with tainting, but still).

We should only ignore the start of the vermagic string if the module
actually *has* crcs to check.  Rather than (say) having an
entertaining hissy fit and creating a config option to work around the
buggy code.

Signed-off-by: Rusty Russell <rusty@...tcorp.com.au>
---
 init/Kconfig    |    6 +++---
 kernel/module.c |   16 ++++++++++------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff -r 8805b380fdff init/Kconfig
--- a/init/Kconfig	Thu May 08 21:14:37 2008 +1000
+++ b/init/Kconfig	Thu May 08 21:16:20 2008 +1000
@@ -837,9 +837,9 @@ config MODULE_FORCE_LOAD
 	depends on MODULES
 	default n
 	help
-	  This option allows loading of modules even if that would set the
-          'F' (forced) taint, due to lack of version info.  Which is
-	  usually a really bad idea.
+	  Allow loading of modules without version information (ie. modprobe
+	  --force).  Forced module loading sets the 'F' (forced) taint flag and
+	  is usually a really bad idea.
 
 config MODULE_UNLOAD
 	bool "Module unloading"
diff -r 8805b380fdff kernel/module.c
--- a/kernel/module.c	Thu May 08 21:14:37 2008 +1000
+++ b/kernel/module.c	Thu May 08 21:16:20 2008 +1000
@@ -956,11 +956,14 @@ static inline int check_modstruct_versio
 	return check_version(sechdrs, versindex, "struct_module", mod, crc);
 }
 
-/* First part is kernel version, which we ignore. */
-static inline int same_magic(const char *amagic, const char *bmagic)
+/* First part is kernel version, which we ignore if module has crcs. */
+static inline int same_magic(const char *amagic, const char *bmagic,
+			     bool has_crcs)
 {
-	amagic += strcspn(amagic, " ");
-	bmagic += strcspn(bmagic, " ");
+	if (has_crcs) {
+		amagic += strcspn(amagic, " ");
+		bmagic += strcspn(bmagic, " ");
+	}
 	return strcmp(amagic, bmagic) == 0;
 }
 #else
@@ -980,7 +983,8 @@ static inline int check_modstruct_versio
 	return 1;
 }
 
-static inline int same_magic(const char *amagic, const char *bmagic)
+static inline int same_magic(const char *amagic, const char *bmagic,
+			     bool has_crcs)
 {
 	return strcmp(amagic, bmagic) == 0;
 }
@@ -1873,7 +1877,7 @@ static struct module *load_module(void _
 		err = try_to_force_load(mod, "magic");
 		if (err)
 			goto free_hdr;
-	} else if (!same_magic(modmagic, vermagic)) {
+	} else if (!same_magic(modmagic, vermagic, versindex)) {
 		printk(KERN_ERR "%s: version magic '%s' should be '%s'\n",
 		       mod->name, modmagic, vermagic);
 		err = -ENOEXEC;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/