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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87k2zeskm9.fsf@rustcorp.com.au>
Date:	Thu, 19 Feb 2015 12:55:34 +1030
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	Lucas De Marchi <lucas.de.marchi@...il.com>
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

Lucas De Marchi <lucas.de.marchi@...il.com> writes:
> 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

It's weird that your code should care about this at all.  Ideally,
userspace would see builtin modules as simply unremovable ones.
Historically, it hasn't; it was only module parameters for builtins
which caused us to expose built modules.

If we returned EEXIST for builtin modules, would you have to do the
initstate check at all?

Thanks,
Rusty.
--
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/