module: put modversions in vermagic
PATCH 1/3 module: put modversions in vermagic
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.
--- include/linux/vermagic.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
include/linux/vermagic.h +7 -1
@@ -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
PATCH 2/3 module: be more picky about allowing missing module versions
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.
--- kernel/module.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
kernel/module.c +7 -2
@@ -917,6 +917,10 @@ static int check_version(Elf_Shdr *sechdif (!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",
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.
--- init/Kconfig | 6 +++--- kernel/module.c | 16 ++++++++++------ 2 files changed, 13 insertions(+), 9 deletions(-)
init/Kconfig +3 -3
@@ -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"
kernel/module.c +10 -6
@@ -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_versioreturn 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 ;
Re: [PATCH 3/3] module: don't ignore vermagic string if module doesn't have modversions
Quoted 5 lines · Rusty Russell
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
Re: [PATCH 3/3] module: don't ignore vermagic string if module doesn't have modversions
Quoted 9 lines · On Saturday 10 May 2008 00:44:34 Linus Torvalds wrote:
Quoted 4 lines · Rusty Russell
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.