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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 23 Sep 2021 16:37:00 -0600 From: Shuah Khan <skhan@...uxfoundation.org> To: Luis Chamberlain <mcgrof@...nel.org> Cc: jeyu@...nel.org, linux-kernel@...r.kernel.org, Shuah Khan <skhan@...uxfoundation.org> Subject: Re: [PATCH] module: fix invalid ELF structure error to print error code On 9/22/21 8:59 PM, Luis Chamberlain wrote: > 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. > I call this a fix because this message should have included the error code to being with and is useless without it. Having bots pick up is one of the reasons I called this it a fix. I am debugging a problem that a module is failing to load, it would have been helpful to know the error code. > 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. > That is correct. This code path returns error without any indication of what happened and needs fixing. I can do that. > 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? > I considered adding messages to elf_validity_check() error paths. I would add them as pr_err() though so that it will be easier to debug. These are errors that indicate module load failed and pr_err() is a better choice. It doesn't look like we have access to .modinfo for determining the module name. This information will be very useful. thanks, -- Shuah
Powered by blists - more mailing lists