Skip to contentSkip to conversation

MODSIGN: Warn when module signature checking fails

PATCH In review Chris Samuel 3 participants 6 messages Jan 16, 2013 – Jan 22, 2013

lore
Share & tools
lore

Chris Samuel

author
#0 lore raw

PATCH 1/1 MODSIGN: Warn when module signature checking fails

/* Please CC me, I'm not on LKML */

* Reworked from the original patch based on feedback from Josh Boyer
* (putting the code in load_module()) and Rusty Russel (use
* KERN_NOTICE). Extended to cover the other failure modes.

Currently if a signature check fails on module load for any reason no feedback is given, except if the failure is due to either not having the appropriate key or the signature is missing (both -ENOKEY) *and* we are not doing strict checking (module.sig_enforce is not set). In that situation only a contextless kernel taint message appears:

Disabling lock debugging due to kernel taint

This patch:

a) Reports the reason why a module has failed to load (either one of the explicit failures or an error propagated back from the crypto infrastructure) if it is going to be a hard failure (err is set).

b) causes a single warning to be emitted to explain why the kernel is being tainted, before the above taint warning is output, when the error is -ENOKEY (implicit in err=0 and sig_ok not being true) and module.sig_enforce is not set.

Found whilst trying to work out why all the 3.8 development kernels I was building and testing were warning about taints and why all modules were listed as forced load (F) in /proc/modules.

It would be really nice to be able to work out what the name of the module is, but that doesn't appear to be possible from the information load_module() has access to, plus my kernel-fu is not great.

Signed-off-by Christopher Samuel <[email protected]>
---
 kernel/module.c |   21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)
kernel/module.c +19 -2
index 250092c..ec789d3 100644
@@ -3099,8 +3099,23 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	long err;
  	err = module_sig_check(info);
-	if (err)
+	if (err) {
+		switch (err) {
+		case -EBADMSG:
+			printk(KERN_NOTICE "Module verification failed: module signature corrupt or unrecognised - not loading\n");
+			break;
+		case -ENOPKG:
+			printk(KERN_NOTICE "Module verification failed: requisite algorithm unavailable - not loading\n");
+			break;
+		case -ENOKEY:
+			printk(KERN_NOTICE "Module verification failed: signature and/or required key missing - not loading\n");
+			break;
+		default:
+			printk(KERN_NOTICE "Module verification failed: supporting code reported failure %ld - not loading\n",err);
+			break;
+		}
 		goto free_copy;
+	}
  	err = elf_header_check(info);
 	if (err)

@@ -3115,8 +3130,10 @@ static int load_module(struct load_info *info, const char __user *uargs,
  #ifdef CONFIG_MODULE_SIG
 	mod->sig_ok = info->sig_ok;
-	if (!mod->sig_ok)
+	if (!mod->sig_ok) {
+		printk_once(KERN_NOTICE "Module verification failed: signature and/or required key missing - tainting kernel\n");
 		add_taint_module(mod, TAINT_FORCED_MODULE);
+	}
 #endif
  	/* Now module is in final location, initialize linked lists, etc. */
Signature · 1 line
1.7.10.4
#1 lore raw
Quoted 5 lines · Chris Samuel

/* Please CC me, I'm not on LKML */

* Reworked from the original patch based on feedback from Josh Boyer
* (putting the code in load_module()) and Rusty Russel (use
* KERN_NOTICE). Extended to cover the other failure modes.

We have errnos for a reason; let's not pollute the kernel logs. That's a userspace job.

Review static int load_module(struct load_info *info, · line 3137

Quoted 10 lines
(quoted diff) +3 -1
@@ -3115,8 +3130,10 @@ static int load_module(struct load_info *info, 
 const char __user *uargs,
   #ifdef CONFIG_MODULE_SIG
  	mod->sig_ok = info->sig_ok;
-	if (!mod->sig_ok)
+	if (!mod->sig_ok) {
+		printk_once(KERN_NOTICE "Module verification failed: signature and/or 
 required key missing - tainting kernel\n");
  		add_taint_module(mod, TAINT_FORCED_MODULE);
+	}

This part is OK, but I'll add mod->name to the printk.

How's this:

module: printk message when module signature fail taints kernel.

Reported-by Chris Samuel <[email protected]>
Signed-off-by Rusty Russell <[email protected]>
kernel/module.c +5 -1
index ec535aa..e095e19 100644
@@ -3169,8 +3169,12 @@ again:
 
 #ifdef CONFIG_MODULE_SIG
 	mod->sig_ok = info->sig_ok;
-	if (!mod->sig_ok)
+	if (!mod->sig_ok) {
+		printk_once(KERN_NOTICE
+			    "%s: module verification failed: signature and/or"
+			    " required key missing - tainting kernel\n");
 		add_taint_module(mod, TAINT_FORCED_MODULE);
+	}
 #endif
 
 	/* Now module is in final location, initialize linked lists, etc. */
#2 lore raw

Hi Rusty,

kernel/module.c again: · line 3175

Quoted 8 lines · On Mon, 21 Jan 2013 10:06:11 +1030 Rusty Russell <[email protected]> wrote:

#ifdef CONFIG_MODULE_SIG

kernel/module.c +4 -1
 	mod->sig_ok = info->sig_ok;
-	if (!mod->sig_ok)
+	if (!mod->sig_ok) {
+		printk_once(KERN_NOTICE
+			    "%s: module verification failed: signature and/or"
+			    " required key missing - tainting kernel\n");

Did you forget to add "mod->name"?

Signature · 2 lines
Cheers,
Stephen Rothwell                    [email protected]
#3 lore raw
Quoted 4 lines · Stephen Rothwell

Hi Rusty,

Quoted 1 line · On Mon, 21 Jan 2013 10:06:11 +1030 Rusty Russell <[email protected]> wrote:
Quoted 9 lines

kernel/module.c again: · line 3175

Quoted 7 lines

#ifdef CONFIG_MODULE_SIG

kernel/module.c +4 -1
 	mod->sig_ok = info->sig_ok;
-	if (!mod->sig_ok)
+	if (!mod->sig_ok) {
+		printk_once(KERN_NOTICE
+			    "%s: module verification failed: signature and/or"
+			    " required key missing - tainting kernel\n");

Did you forget to add "mod->name"?

Indeed (turns back on CONFIG_MODULE_SIG).

Thanks,
Rusty.

author
#4 lore raw
In reply to Rusty Russell

/* Please CC me, I'm not on LKML */

Quoted 2 lines · Rusty Russell

We have errnos for a reason; let's not pollute the kernel logs. That's a userspace job.

Fair enough.

Quoted 1 line

This part is OK, but I'll add mod->name to the printk.

Sounds good.

Quoted 1 line

How's this:

Looks fine, modulo the lack of mod->name as Stephen mentioned.

cheers,
Chris

Signature · 1 line
 Chris Samuel  :  http://www.csamuel.org/  :  Melbourne, VIC
#5 lore raw
Quoted 9 lines · Chris Samuel

/* Please CC me, I'm not on LKML */

Quoted 2 lines · Rusty Russell

We have errnos for a reason; let's not pollute the kernel logs. That's a userspace job.

Fair enough.

Quoted 7 lines
Quoted 1 line

This part is OK, but I'll add mod->name to the printk.

Sounds good.

Quoted 1 line

How's this:

Looks fine, modulo the lack of mod->name as Stephen mentioned.

Yeah, here's what is now in Linus' tree:

commit 64748a2c9062da0c32b59c1b368a86fc4613b1e1

Author: Rusty Russell <[email protected]>
Date:   Mon Jan 21 17:03:02 2013 +1030

module: printk message when module signature fail taints kernel.

Reported-by Chris Samuel <[email protected]>
Signed-off-by Rusty Russell <[email protected]>
kernel/module.c +6 -1
index eab0827..e69a5a6 100644
@@ -3192,8 +3192,13 @@ again:
 
 #ifdef CONFIG_MODULE_SIG
 	mod->sig_ok = info->sig_ok;
-	if (!mod->sig_ok)
+	if (!mod->sig_ok) {
+		printk_once(KERN_NOTICE
+			    "%s: module verification failed: signature and/or"
+			    " required key missing - tainting kernel\n",
+			    mod->name);
 		add_taint_module(mod, TAINT_FORCED_MODULE);
+	}
 #endif
 
 	/* Now module is in final location, initialize linked lists, etc. */