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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 23 Sep 2021 16:37:00 -0600
From:   Shuah Khan <>
To:     Luis Chamberlain <>
        Shuah Khan <>
Subject: Re: [PATCH] module: fix invalid ELF structure error to print error

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 <>
>> ---
>>   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.

-- Shuah

Powered by blists - more mailing lists