Thread (5 messages) 5 messages, 3 authors, 2020-09-02

Re: [PATCH] module: Add more error message for failed kernel module loading

From: Lucas De Marchi <hidden>
Date: 2020-09-01 18:51:14
Also in: lkml

On Sat, Aug 29, 2020 at 4:15 AM Qu Wenruo [off-list ref] wrote:
quoted hunk ↗ jump to hunk
When kernel module loading failed, user space only get one of the
following error messages:
- -ENOEXEC
  This is the most confusing one. From corrupted ELF header to bad
  WRITE|EXEC flags check introduced by in module_enforce_rwx_sections()
  all returns this error number.

- -EPERM
  This is for blacklisted modules. But mod doesn't do extra explain
  on this error either.

- -ENOMEM
  The only error which needs no explain.

This means, if a user got "Exec format error" from modprobe, it provides
no meaningful way for the user to debug, and will take extra time
communicating to get extra info.

So this patch will add extra error messages for -ENOEXEC and -EPERM
errors, allowing user to do better debugging and reporting.

Signed-off-by: Qu Wenruo <redacted>
---
 kernel/module.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 1c5cff34d9f2..9f748c6eeb48 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2096,8 +2096,12 @@ static int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
        int i;

        for (i = 0; i < hdr->e_shnum; i++) {
-               if ((sechdrs[i].sh_flags & shf_wx) == shf_wx)
+               if ((sechdrs[i].sh_flags & shf_wx) == shf_wx) {
+                       pr_err(
+                       "Module %s section %d has invalid WRITE|EXEC flags\n",
+                               mod->name, i);
                        return -ENOEXEC;
+               }
        }

        return 0;
@@ -3825,8 +3829,10 @@ static int load_module(struct load_info *info, const char __user *uargs,
        char *after_dashes;

        err = elf_header_check(info);
-       if (err)
+       if (err) {
+               pr_err("Module has invalid ELF header\n");
                goto free_copy;
+       }

        err = setup_load_info(info, flags);
        if (err)
@@ -3834,6 +3840,7 @@ static int load_module(struct load_info *info, const char __user *uargs,

        if (blacklisted(info->name)) {
                err = -EPERM;
+               pr_err("Module %s is blacklisted\n", info->name);
I wonder why would anyone actually add the blacklist to the command
line like this and have no
way to revert that back. This was introduced in
be7de5f91fdc modules: Add kernel parameter to blacklist modules
as a way to overcome broken initrd generation afaics. Either kernel
command line (using modprobe.blacklist)
or /etc/modprobe.d options are honoured by libkmod and allow a
sufficiently privileged user to bypass it.

+Rusty, +Prarit: is there anything this module parameter is covering
that I'm missing?

For the changes here,

Reviewed-by: Lucas De Marchi <redacted>

thanks
Lucas De Marchi
                goto free_copy;
        }

--
2.27.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help