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)


Reply via email to