lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YUvtfUHb+8XjEtTf@bombadil.infradead.org>
Date:   Wed, 22 Sep 2021 19:59:09 -0700
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Shuah Khan <skhan@...uxfoundation.org>
Cc:     jeyu@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] module: fix invalid ELF structure error to print error
 code

On Wed, Sep 22, 2021 at 06:14:42PM -0600, Shuah Khan wrote:
> When elf_validity_check() returns error, load_module() prints an
> error message without error code. It is hard to determine why the
> module ELF structure is invalid without this information. Fix the
> error message to print the error code.
> 
> Signed-off-by: Shuah Khan <skhan@...uxfoundation.org>
> ---
>  kernel/module.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 40ec9a030eec..a0d412d396d6 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3941,7 +3941,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
>  	 */
>  	err = elf_validity_check(info);
>  	if (err) {
> -		pr_err("Module has invalid ELF structures\n");
> +		pr_err("Module has invalid ELF structures:errno(%ld)\n", err);
>  		goto free_copy;

The subject seems to indicate a fix is being made, and I think that the
bots that pick up fixes through language might find that this is a
candidate because of that. It is not, and so if you can change the
subject to indicate that this is just expanding the error print message
with the actual error code passed it would be better.

Now, if you look at elf_validity_check() and how it can fail, it would
seem tons of errors are due to -ENOEXEC. Even a function it calls,
validate_section_offset() also uses that return code. So on failure,
you likely won't still know exactly where this failed as you likely
will juse see the -ENOEXEC value, but that won't tell you much I think.

So, it would seem a bit more useful instead to add some pr_debug()s
on the elf_validity_check() and friends so that dynamic debug could
be used to debug and figure out where this did fail, when needed?
Thoughts?

  Luis

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ