linux-kernel - Re: [PATCH] libkmod-module: Remove directory existence check for KMOD_MODULE

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: <CAKi4VAKc_zrPptt2ZKT_Jj8bLdL1bt0emhtEuodV2WKkas9ZOw@mail.gmail.com>
Date:	Wed, 18 Feb 2015 23:19:14 -0200
From:	Lucas De Marchi <lucas.de.marchi@...il.com>
To:	Rusty Russell <rusty@...tcorp.com.au>
Cc:	Harish Jenny K N <harish_kandiga@...tor.com>,
	linux-modules <linux-modules@...r.kernel.org>,
	lkml <linux-kernel@...r.kernel.org>,
	greg KH <gregkh@...uxfoundation.org>
Subject: Re: [PATCH] libkmod-module: Remove directory existence check for KMOD_MODULE_BUILTIN

On Wed, Feb 18, 2015 at 8:40 PM, Rusty Russell <rusty@...tcorp.com.au> wrote:
> Lucas De Marchi <lucas.de.marchi@...il.com> writes:
>> On Wed, Feb 18, 2015 at 2:07 AM, Rusty Russell <rusty@...tcorp.com.au> wrote:
>> Yeah, I just thought (an wanted that) the attributes were being
>> created first and then hooked up in the sysfs tree under
>> /sys/module/<modulename>. I.e. if the directory exists and there's no
>> initstate this is because it's a builtin module. I don't want to
>> wait/sleep on the file to appear because users of
>> kmod_module_get_initstate() may not tolerate this behavior.
>>
>> Looking up at the old module-init-tools, it used an ugly loop with
>> usleep() before trying to read the file again :-/
>>
>> Can we change kernel side guaranteeing the initstate file appears
>> together with the directory?
>
> Greg?  The core problem is that kmod looks for
> /sys/module/<name>/initstate; if it's not there, it assumes a builtin
> module.

Just to make it clear:

We try to open /sys/module/<name>/initstate. If it fails we stat
/sys/module/<name> checking if it exists and is a directory. If it
does then we assume the module is builtin.

> However, this is racy when a module is being inserted.  Is there a way
> to create this sysfs file and dir atomically?

Greg, the question is still valid since it'd be nice to have this
guarantee and be able to correctly reply the state with whatever is in
initstate file, but...

Rusty, thinking again if we fallback to "coming" instead of "builtin"
everything should be fine, no? Because the decision about builtin has
already been taken by looking at the modules.builtin index. If we
return "coming" here the second call to modprobe would call
init_module() again which would wait for the first one to complete (or
return EEXIST if it's already live) since we only shortcut the
init_module() call if the module is live or builtin

what do you think?

Harrish, in your patch if you just change the "return
KMOD_MODULE_BUILTIN;" to "return KMOD_MODULE_COMING;" does it work?

-- 
Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/