Re: [PATCH] module support: during lockdown, log name of unsigned module

[Jessica Yu]

+++ Martin Haass [19/02/20 10:02 +0100]:
> during lockdown loading of unsigned modules is restricted to signed
> modules only. The old error message does not show which module misses
> the signature, making it very difficult for a user to determine which
> module is at fault.
> This patch adds a line to the logs which additionally contains the
> module name that caused the error message. The old message cannot
> be replaced as it is generated by lockdown_is_locked_down
> ---
> kernel/module.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 33569a01d6e..6dcb28139a0 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2807,7 +2807,8 @@ static int module_sig_check(struct load_info *info,
> int flags)
>  const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
>  const char *reason;
>  const void *mod = info->hdr;
> -
> + int is_locked = -EPERM;
> +
>  /*
>  * Require flags == 0, as a module with version information
>  * removed is no longer the module that was signed
> @@ -2843,7 +2844,12 @@ static int module_sig_check(struct load_info *info,
> int flags)
>  return -EKEYREJECTED;
>  }
>
> - return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
> + is_locked = security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
> + if (is_locked == -EPERM) {
> + pr_notice("Lockdown: %s: rejected module '%s' cause: %s",
> + current->comm, info->name, reason);
> + }
> + return is_locked;

Hi!

Actually, I think we can just reuse the pr_notice() from the previous if
(is_module_sig_enforced()) block. It already logs the module name as well as
the reason. And we'd better leave the lockdown-specific messages to the LSM.
Something like this perhaps?

diff --git a/kernel/module.c b/kernel/module.c
index b88ec9cd2a7f..2c881e3b9d92 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2838,12 +2838,13 @@ static int module_sig_check(struct load_info *info, int flags)
       case -ENOKEY:
               reason = "Loading of module with unavailable key";
       decide:
-               if (is_module_sig_enforced()) {
+               err = is_module_sig_enforced() ? \
+                     -EKEYREJECTED : security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
+
+               if (err)
                       pr_notice("%s: %s is rejected\n", info->name, reason);
-                       return -EKEYREJECTED;
-               }

-               return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
+               return err;

               /* All other errors are fatal, including nomem, unparseable
                * signatures and signature check failures - even if signatures