Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1628655 > unrolled thread

[PATCH v2 2/2] module: Add module name to modinfo

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 2/2] module: Add module name to modinfo Kees Cook <keescook@chromium.org> - 2017-04-22 00:40 +0200
    Re: [PATCH v2 2/2] module: Add module name to modinfo Jon Masters <jcm@jonmasters.org> - 2017-04-25 09:10 +0200
      Re: [PATCH v2 2/2] module: Add module name to modinfo Jessica Yu <jeyu@redhat.com> - 2017-04-26 05:00 +0200
    Re: [PATCH v2 2/2] module: Add module name to modinfo Jon Masters <jcm@jonmasters.org> - 2017-04-25 09:10 +0200

#1628655 — [PATCH v2 2/2] module: Add module name to modinfo

Accessing the mod structure (e.g. for mod->name) prior to having completed
check_modstruct_version() can result in writing garbage to the error logs
if the layout of the mod structure loaded from disk doesn't match the
running kernel's mod structure layout. This kind of mismatch will become
much more likely if a kernel is built with different randomization seed
for the struct layout randomization plugin.

Instead, add and use a new modinfo string for logging the module name.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 kernel/module.c       | 29 ++++++++++++++++++++++-------
 scripts/mod/modpost.c |  1 +
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index ed6cf2367217..29a4a77b6849 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -299,6 +299,7 @@ int unregister_module_notifier(struct notifier_block *nb)
 EXPORT_SYMBOL(unregister_module_notifier);
 
 struct load_info {
+	char *name;
 	Elf_Ehdr *hdr;
 	unsigned long len;
 	Elf_Shdr *sechdrs;
@@ -1297,12 +1298,12 @@ static int check_version(const struct load_info *info,
 	}
 
 	/* Broken toolchain. Warn once, then let it go.. */
-	pr_warn_once("%s: no symbol version for %s\n", mod->name, symname);
+	pr_warn_once("%s: no symbol version for %s\n", info->name, symname);
 	return 1;
 
 bad_version:
 	pr_warn("%s: disagrees about version of symbol %s\n",
-	       mod->name, symname);
+	       info->name, symname);
 	return 0;
 }
 
@@ -2892,9 +2893,15 @@ static int rewrite_section_headers(struct load_info *info, int flags)
 		info->index.vers = 0; /* Pretend no __versions section! */
 	else
 		info->index.vers = find_sec(info, "__versions");
+	info->sechdrs[info->index.vers].sh_flags &= ~(unsigned long)SHF_ALLOC;
+
 	info->index.info = find_sec(info, ".modinfo");
+	if (!info->index.info)
+		info->name = "(missing .modinfo section)";
+	else
+		info->name = get_modinfo(info, "name");
 	info->sechdrs[info->index.info].sh_flags &= ~(unsigned long)SHF_ALLOC;
-	info->sechdrs[info->index.vers].sh_flags &= ~(unsigned long)SHF_ALLOC;
+
 	return 0;
 }
 
@@ -2934,14 +2941,22 @@ static struct module *setup_load_info(struct load_info *info, int flags)
 
 	info->index.mod = find_sec(info, ".gnu.linkonce.this_module");
 	if (!info->index.mod) {
-		pr_warn("No module found in object\n");
+		pr_warn("%s: No module found in object\n",
+			info->name ?: "(missing .modinfo name field)");
 		return ERR_PTR(-ENOEXEC);
 	}
 	/* This is temporary: point mod into copy of data. */
 	mod = (void *)info->sechdrs[info->index.mod].sh_addr;
 
+	/*
+	 * If we didn't load the .modinfo 'name' field, fall back to
+	 * on-disk struct mod 'name' field.
+	 */
+	if (!info->name)
+		info->name = mod->name;
+
 	if (info->index.sym == 0) {
-		pr_warn("%s: module has no symbols (stripped?)\n", mod->name);
+		pr_warn("%s: module has no symbols (stripped?)\n", info->name);
 		return ERR_PTR(-ENOEXEC);
 	}
 
@@ -2969,7 +2984,7 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
 			return err;
 	} else if (!same_magic(modmagic, vermagic, info->index.vers)) {
 		pr_err("%s: version magic '%s' should be '%s'\n",
-		       mod->name, modmagic, vermagic);
+		       info->name, modmagic, vermagic);
 		return -ENOEXEC;
 	}
 
@@ -3249,7 +3264,7 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
 	if (IS_ERR(mod))
 		return mod;
 
-	if (blacklisted(mod->name))
+	if (blacklisted(info->name))
 		return ERR_PTR(-EPERM);
 
 	err = check_modinfo(mod, info, flags);
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 30d752a4a6a6..48397feb08fb 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2126,6 +2126,7 @@ static void add_header(struct buffer *b, struct module *mod)
 	buf_printf(b, "#include <linux/compiler.h>\n");
 	buf_printf(b, "\n");
 	buf_printf(b, "MODULE_INFO(vermagic, VERMAGIC_STRING);\n");
+	buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
 	buf_printf(b, "\n");
 	buf_printf(b, "__visible struct module __this_module\n");
 	buf_printf(b, "__attribute__((section(\".gnu.linkonce.this_module\"))) = {\n");
-- 
2.7.4

[toc] | [next] | [standalone]

#1630231

Nevermind. Missread the patch as doing something different on first pass.

-- 
Computer Architect | Sent from my 64-bit #ARM Powered phone

> On Apr 25, 2017, at 03:00, Jon Masters <jcm@jonmasters.org> wrote:
> 
>> On 04/21/2017 06:35 PM, Kees Cook wrote:
>> 
>> Accessing the mod structure (e.g. for mod->name) prior to having completed
>> check_modstruct_version() can result in writing garbage to the error logs
>> if the layout of the mod structure loaded from disk doesn't match the
>> running kernel's mod structure layout. This kind of mismatch will become
>> much more likely if a kernel is built with different randomization seed
>> for the struct layout randomization plugin.
>> 
>> Instead, add and use a new modinfo string for logging the module name.
> 
> +Lucas - probably something that the modinfo kmod utility should track.
> 
> Jon.
> 

[toc] | [prev] | [next] | [standalone]

#1631120

+++ Jon Masters [25/04/17 03:04 -0400]:
>Nevermind. Missread the patch as doing something different on first pass.

It's good to give the kmod folks a heads up anyway (as name would be
visible in modinfo), thanks Jon!

>> On Apr 25, 2017, at 03:00, Jon Masters <jcm@jonmasters.org> wrote:
>>
>>> On 04/21/2017 06:35 PM, Kees Cook wrote:
>>>
>>> Accessing the mod structure (e.g. for mod->name) prior to having completed
>>> check_modstruct_version() can result in writing garbage to the error logs
>>> if the layout of the mod structure loaded from disk doesn't match the
>>> running kernel's mod structure layout. This kind of mismatch will become
>>> much more likely if a kernel is built with different randomization seed
>>> for the struct layout randomization plugin.
>>>
>>> Instead, add and use a new modinfo string for logging the module name.
>>
>> +Lucas - probably something that the modinfo kmod utility should track.
>>
>> Jon.
>>

[toc] | [prev] | [next] | [standalone]

#1630240

On 04/21/2017 06:35 PM, Kees Cook wrote:

> Accessing the mod structure (e.g. for mod->name) prior to having completed
> check_modstruct_version() can result in writing garbage to the error logs
> if the layout of the mod structure loaded from disk doesn't match the
> running kernel's mod structure layout. This kind of mismatch will become
> much more likely if a kernel is built with different randomization seed
> for the struct layout randomization plugin.
> 
> Instead, add and use a new modinfo string for logging the module name.

+Lucas - probably something that the modinfo kmod utility should track.

Jon.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web