Re: [PATCH] MODSIGN: Warn when sign check fails due to -ENOKEY

[Chris Samuel <chris@csamuel.org>]

On 12/01/13 00:49, Josh Boyer wrote:

> On Fri, Jan 11, 2013 at 4:44 AM, Chris Samuel <chris@csamuel.org> wrote:
 >
>> /* Please CC me in responses, I am not subscribed to LKML */
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 250092c..27de534 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -2443,8 +2443,10 @@ static int module_sig_check(struct load_info *info)
>>          if (err < 0 && fips_enabled)
>>                  panic("Module verification failed with error %d in FIPS
>> mode\n",
>>                        err);
>> -       if (err == -ENOKEY && !sig_enforce)
>> +       if (err == -ENOKEY && !sig_enforce) {
>> +               printk_once(KERN_DEBUG "Module verification failed, required
>> key not present, tainting kernel\n");
>>                  err = 0;
>> +       }
>>          return err;
>
> I'd suggest putting the printk in load_module where we call the
> add_taint_module function instead.

I did ponder that, but I used module_sig_check() instead as here we know 
explicitly that the failure is -ENOKEY, that information doesn't seem to 
get propagated back to load_module().

Looking at the code again though it seems that any other reason will 
make module_sig_check() return non-zero and hence cause the module to 
fail to load, so currently we can infer that the reason was -ENOKEY.

I'm happy either way, just my inner pedant thought this was better as in 
future module_sig_check() may find another reason to have to return with 
a zero status when modules aren't signed and so we can no longer tell 
the user the reason the signature failed.

Rusty, which is your preference?

> Also, you might want to make the priority a bit higher if it's meant
> to be informative.  Something like KERN_INFO.

Yup, sounds good, I see Rusty suggested KERN_NOTICE so I'll use that.

cheers,
Chris
-- 
  Chris Samuel  :  http://www.csamuel.org/  :  Melbourne, VIC