lkml.org 
[lkml]   [2008]   [May]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: changeset: Make forced module loading optional
Date
On Monday 05 May 2008 15:05:51 Linus Torvalds wrote:
> On Mon, 5 May 2008, Rusty Russell wrote:
> > I'm trying to figure out how you did this. So fedora builds
> > unversioned modules, and version (and vermagic) matched your kernel? And
> > you somehow mixed them up?
>
> And quite frankly, when I finally figured out what was going on, I was
> like *WHAT THE HELL*. That kernel/module.c code was absolute and utter
> crap in accepting modules that neither matched the kernel version
> signature (because it had CONFIG_MODVERSIONS) *nor* the actual versioned
> symbols (because the distro modules had been built without
> CONFIG_MODVERSIONS).

Erk, yes. We ignore vermagic because we expect modversions, then ignore
missing modversions. This is a logic bug; let's fix it.

BTW, for the peanut gallery: I don't recommend modversions: it's not reliable
in detecting all differences, nor being stable when there are no real
differences.

Untested:

module: don't ignore vermagic string if module doesn't have crcs

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 at all (with tainting, but
still).

We should only ignore the start of the vermagic string if the module
actually *has* crcs to check.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff -r 0cefb252efe8 kernel/module.c
--- a/kernel/module.c Mon May 05 15:00:12 2008 +1000
+++ b/kernel/module.c Mon May 05 15:25:17 2008 +1000
@@ -939,11 +939,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
@@ -963,7 +966,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;
}
@@ -1741,6 +1745,7 @@ static struct module *load_module(void _
void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
struct exception_table_entry *extable;
mm_segment_t old_fs;
+ bool has_crcs = false;

DEBUGP("load_module: umod=%p, len=%lu, uargs=%p\n",
umod, len, uargs);
@@ -1850,13 +1855,21 @@ static struct module *load_module(void _
goto free_hdr;
}

+#ifdef CONFIG_MODVERSIONS
+ if ((mod->num_syms == 0 || crcindex) &&
+ (mod->num_gpl_syms == 0 || gplcrcindex) &&
+ (mod->num_gpl_future_syms == 0 || gplfuturecrcindex) &&
+ (mod->num_unused_syms == 0 || unusedcrcindex) &&
+ (mod->num_unused_gpl_syms == 0 || unusedgplcrcindex))
+ has_crcs = true;
+#endif
modmagic = get_modinfo(sechdrs, infoindex, "vermagic");
/* This is allowed: modprobe --force will invalidate it. */
if (!modmagic) {
add_taint_module(mod, TAINT_FORCED_MODULE);
printk(KERN_WARNING "%s: no version magic, tainting kernel.\n",
mod->name);
- } else if (!same_magic(modmagic, vermagic)) {
+ } else if (!same_magic(modmagic, vermagic, has_crcs)) {
printk(KERN_ERR "%s: version magic '%s' should be '%s'\n",
mod->name, modmagic, vermagic);
err = -ENOEXEC;
@@ -2001,11 +2014,8 @@ static struct module *load_module(void _
= (void *)sechdrs[unusedgplcrcindex].sh_addr;

#ifdef CONFIG_MODVERSIONS
- if ((mod->num_syms && !crcindex) ||
- (mod->num_gpl_syms && !gplcrcindex) ||
- (mod->num_gpl_future_syms && !gplfuturecrcindex) ||
- (mod->num_unused_syms && !unusedcrcindex) ||
- (mod->num_unused_gpl_syms && !unusedgplcrcindex)) {
+ /* If we get this far, it's time to warn about missing versions. */
+ if (!has_crcs) {
printk(KERN_WARNING "%s: No versions for exported symbols."
" Tainting kernel.\n", mod->name);
add_taint_module(mod, TAINT_FORCED_MODULE);

\
 
 \ /
  Last update: 2008-05-05 07:37    [W:0.751 / U:1.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site