Messages in this thread Patch in this message | | | Subject | Re: [PATCH 0/2] module: some refactoring in module_sig_check() | From | Joe Perches <> | Date | Tue, 13 Oct 2020 15:44:42 -0700 |
| |
On Tue, 2020-10-13 at 23:32 +0300, Sergey Shtylyov wrote: > Here are 2 patches against the 'modules-next' branch of Jessica Yu's 'linux.git' repo. > I'm doing some little refactoring in module_sig_check()... > > [1/2] module: merge repetitive strings in module_sig_check() > [2/2] module: unindent comments in module_sig_check()
I think this code is rather cryptic and could be made clearer.
How about: --- kernel/module.c | 51 ++++++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 25 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c index c075a18103fb..98b3af96a5bd 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2884,46 +2884,47 @@ static int module_sig_check(struct load_info *info, int flags) * Require flags == 0, as a module with version information * removed is no longer the module that was signed */ - if (flags == 0 && - info->len > markerlen && - memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) { + if (flags == 0 && info->len > markerlen && + !memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen)) { /* We truncate the module to discard the signature */ info->len -= markerlen; err = mod_verify_sig(mod, info); + if (!err) { + info->sig_ok = true; + return 0; + } } + /* + * We don't permit modules to be loaded into trusted kernels + * without a valid signature on them, but if we're not enforcing, + * some errors are non-fatal. + */ switch (err) { - case 0: - info->sig_ok = true; - return 0; - - /* We don't permit modules to be loaded into trusted kernels - * without a valid signature on them, but if we're not - * enforcing, certain errors are non-fatal. - */ case -ENODATA: - reason = "Loading of unsigned module"; - goto decide; + reason = "unsigned module"; + break; case -ENOPKG: - reason = "Loading of module with unsupported crypto"; - goto decide; + reason = "module with unsupported crypto"; + break; case -ENOKEY: - reason = "Loading of module with unavailable key"; - decide: - if (is_module_sig_enforced()) { - pr_notice("%s: %s is rejected\n", info->name, reason); - return -EKEYREJECTED; - } - - return security_locked_down(LOCKDOWN_MODULE_SIGNATURE); - + reason = "module with unavailable key"; + break; + default: /* All other errors are fatal, including nomem, unparseable * signatures and signature check failures - even if signatures * aren't required. */ - default: return err; } + + if (is_module_sig_enforced()) { + pr_notice("%s: loading of %s is rejected\n", + info->name, reason); + return -EKEYREJECTED; + } + + return security_locked_down(LOCKDOWN_MODULE_SIGNATURE); } #else /* !CONFIG_MODULE_SIG */ static int module_sig_check(struct load_info *info, int flags)
| |