On Sat, Aug 29, 2020 at 4:15 AM Qu Wenruo <wqu@xxxxxxxx> wrote:
>
> 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 <wqu@xxxxxxxx>
> ---
> 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 <lucas.demarchi@xxxxxxxxx>
thanks
Lucas De Marchi
> goto free_copy;
> }
>
> --
> 2.27.0
>
