linux-kernel - Re: [RFC 03/10] kmod: add dynamic max concurrent thread count

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: <20161216083956.GG13946@wotan.suse.de>
Date:   Fri, 16 Dec 2016 09:39:56 +0100
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     Petr Mladek <pmladek@...e.com>
Cc:     "Luis R. Rodriguez" <mcgrof@...nel.org>, shuah@...nel.org,
        jeyu@...hat.com, rusty@...tcorp.com.au, ebiederm@...ssion.com,
        dmitry.torokhov@...il.com, acme@...hat.com, corbet@....net,
        martin.wilck@...e.com, mmarek@...e.com, hare@...e.com,
        rwright@....com, jeffm@...e.com, DSterba@...e.com,
        fdmanana@...e.com, neilb@...e.com, linux@...ck-us.net,
        rgoldwyn@...e.com, subashab@...eaurora.org, xypron.glpk@....de,
        keescook@...omium.org, atomlin@...hat.com, mbenes@...e.cz,
        paulmck@...ux.vnet.ibm.com, dan.j.williams@...el.com,
        jpoimboe@...hat.com, davem@...emloft.net, mingo@...hat.com,
        akpm@...ux-foundation.org, torvalds@...ux-foundation.org,
        linux-kselftest@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC 03/10] kmod: add dynamic max concurrent thread count

On Wed, Dec 14, 2016 at 04:38:27PM +0100, Petr Mladek wrote:
> On Thu 2016-12-08 11:48:14, Luis R. Rodriguez wrote:
> > diff --git a/init/Kconfig b/init/Kconfig
> > index 271692a352f1..da2c25746937 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -2111,6 +2111,29 @@ config TRIM_UNUSED_KSYMS
> >  
> >  	  If unsure, or if you need to build out-of-tree modules, say N.
> >  
> > +config MAX_KMOD_CONCURRENT
> > +	int "Max allowed concurrent request_module() calls (6=>64, 10=>1024)"
> > +	range 0 14
> 
> Would not too small range break loading module dependencies?

No, dependencies are resolved by depmod, so userspace looks at the list and
just finit_module() the depenencies, skipping kmod. So the limit is
really only for kernel acting like a boss.

> I am not sure how it is implemented but it might require having
> some more module loads in progress.

Dependencies should be OK, a more serious concern with dependencies is
the aggregate memory it takes to load all dep modules for one required
module since finit_module() ends up allocating the struct module to copy
over data from userspace.

> I would give 6 as minimum. Nobody has troubles with the current limit.

Fair enough! Although disabling modprobe calls all together seemed like
a fun test, that should we allow that via the module parameter at least?

> > +	default 6 if !BASE_SMALL
> > +	default 7 if BASE_SMALL
> 
> Aren't the conditions inversed?

Whoops yes, sorry.

> > +void __init init_kmod_umh(void)
> > +{
> > +	if (!max_modprobes)
> > +		max_modprobes = min(max_threads/2,
> > +				    2 << CONFIG_MAX_KMOD_CONCURRENT);
> 
> This should be
> 
> 	1 << CONFIG_MAX_KMOD_CONCURRENT);
> 
> 1 << 1 = 2;
> 
> Note that this calculation is mentioned also some comments and
> documentation.

Heh sorry, yes fixed! Good thing I had still tested all along with the
value I intended though :P

  Luis