* [PATCH 1/3] module: put modversions in vermagic
@ 2008-05-09 6:23 Rusty Russell
2008-05-09 6:24 ` [PATCH 2/3] module: be more picky about allowing missing module versions Rusty Russell
0 siblings, 1 reply; 5+ messages in thread
From: Rusty Russell @ 2008-05-09 6:23 UTC (permalink / raw
To: linux-kernel; +Cc: Jon Masters, Linus Torvalds
Don't allow a module built without versions altogether to be inserted
into a kernel which expects modversions.
modprobe --force will strip vermagic as well as modversions, so it
won't be effected, but this will make sure that a
non-CONFIG_MODVERSIONS module won't be accidentally inserted into a
CONFIG_MODVERSIONS kernel.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
include/linux/vermagic.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff -r a39eaf516de9 include/linux/vermagic.h
--- a/include/linux/vermagic.h Thu May 08 14:20:44 2008 +1000
+++ b/include/linux/vermagic.h Thu May 08 15:18:47 2008 +1000
@@ -17,6 +17,11 @@
#else
#define MODULE_VERMAGIC_MODULE_UNLOAD ""
#endif
+#ifdef CONFIG_MODVERSIONS
+#define MODULE_VERMAGIC_MODVERSIONS "modversions "
+#else
+#define MODULE_VERMAGIC_MODVERSIONS ""
+#endif
#ifndef MODULE_ARCH_VERMAGIC
#define MODULE_ARCH_VERMAGIC ""
#endif
@@ -24,5 +29,6 @@
#define VERMAGIC_STRING \
UTS_RELEASE " " \
MODULE_VERMAGIC_SMP MODULE_VERMAGIC_PREEMPT \
- MODULE_VERMAGIC_MODULE_UNLOAD MODULE_ARCH_VERMAGIC
+ MODULE_VERMAGIC_MODULE_UNLOAD MODULE_VERMAGIC_MODVERSIONS \
+ MODULE_ARCH_VERMAGIC
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/3] module: be more picky about allowing missing module versions
2008-05-09 6:23 [PATCH 1/3] module: put modversions in vermagic Rusty Russell
@ 2008-05-09 6:24 ` Rusty Russell
2008-05-09 6:25 ` [PATCH 3/3] module: don't ignore vermagic string if module doesn't have modversions Rusty Russell
0 siblings, 1 reply; 5+ messages in thread
From: Rusty Russell @ 2008-05-09 6:24 UTC (permalink / raw
To: linux-kernel; +Cc: Jon Masters, Linus Torvalds
We allow missing __versions sections, because modprobe --force strips
it. It makes less sense to allow sections where there's no version
for a specific symbol the module uses, so disallow that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
kernel/module.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff -r cf690cb1ffad kernel/module.c
--- a/kernel/module.c Thu May 08 21:51:23 2008 +1000
+++ b/kernel/module.c Thu May 08 21:51:48 2008 +1000
@@ -917,6 +917,10 @@ static int check_version(Elf_Shdr *sechd
if (!crc)
return 1;
+ /* No versions at all? modprobe --force does this. */
+ if (versindex == 0)
+ return try_to_force_load(mod, symname) == 0;
+
versions = (void *) sechdrs[versindex].sh_addr;
num_versions = sechdrs[versindex].sh_size
/ sizeof(struct modversion_info);
@@ -932,8 +936,9 @@ static int check_version(Elf_Shdr *sechd
goto bad_version;
}
- if (!try_to_force_load(mod, symname))
- return 1;
+ printk(KERN_WARNING "%s: no symbol version for %s\n",
+ mod->name, symname);
+ return 0;
bad_version:
printk("%s: disagrees about version of symbol %s\n",
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 3/3] module: don't ignore vermagic string if module doesn't have modversions
2008-05-09 6:24 ` [PATCH 2/3] module: be more picky about allowing missing module versions Rusty Russell
@ 2008-05-09 6:25 ` Rusty Russell
2008-05-09 14:44 ` Linus Torvalds
0 siblings, 1 reply; 5+ messages in thread
From: Rusty Russell @ 2008-05-09 6:25 UTC (permalink / raw
To: linux-kernel; +Cc: Jon Masters, Linus Torvalds
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@rustcorp.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;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] module: don't ignore vermagic string if module doesn't have modversions
2008-05-09 6:25 ` [PATCH 3/3] module: don't ignore vermagic string if module doesn't have modversions Rusty Russell
@ 2008-05-09 14:44 ` Linus Torvalds
2008-05-11 0:57 ` Rusty Russell
0 siblings, 1 reply; 5+ messages in thread
From: Linus Torvalds @ 2008-05-09 14:44 UTC (permalink / raw
To: Rusty Russell; +Cc: linux-kernel, Jon Masters
On Fri, 9 May 2008, Rusty Russell wrote:
>
> 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.
Btw, this makes the two vesions of "same_magic()" be basically the same
(since without MODVERSIONS, has_crcs *should* always be zero). Maybe we
should make it just one version, to simplify logic?
Linus
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] module: don't ignore vermagic string if module doesn't have modversions
2008-05-09 14:44 ` Linus Torvalds
@ 2008-05-11 0:57 ` Rusty Russell
0 siblings, 0 replies; 5+ messages in thread
From: Rusty Russell @ 2008-05-11 0:57 UTC (permalink / raw
To: Linus Torvalds; +Cc: linux-kernel, Jon Masters
On Saturday 10 May 2008 00:44:34 Linus Torvalds wrote:
> On Fri, 9 May 2008, Rusty Russell wrote:
> > 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.
>
> Btw, this makes the two vesions of "same_magic()" be basically the same
> (since without MODVERSIONS, has_crcs *should* always be zero). Maybe we
> should make it just one version, to simplify logic?
I know, but it's a little subtle.
If the module has an __versions section, yet the kernel was built without
CONFIG_MODVERSIONS, we should not ignore the first part of the version. We
could just #ifdef CONFIG_MODVERSIONS around the verindex initialization tho,
to guarantee that it's 0 for !MODVERSIONS. But that puts YA #ifdef in that
function, and I already itch to clean up the ones there...
Cheers,
Rusty.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-05-11 0:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-09 6:23 [PATCH 1/3] module: put modversions in vermagic Rusty Russell
2008-05-09 6:24 ` [PATCH 2/3] module: be more picky about allowing missing module versions Rusty Russell
2008-05-09 6:25 ` [PATCH 3/3] module: don't ignore vermagic string if module doesn't have modversions Rusty Russell
2008-05-09 14:44 ` Linus Torvalds
2008-05-11 0:57 ` Rusty Russell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).