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

2017-01-10 Thread Luis R. Rodriguez 
On Fri, Dec 16, 2016 at 09:39:56AM +0100, Luis R. Rodriguez wrote:
> 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.

A simple change can enable us to bail out on finit_module() if a module
is already present by looking at the passed userspace data. I have this
change now but as discussed, whether or not its desirable should be a
matter of whether or not in the typical case (bootup time) things improve.
>From some initial tests it would seem this doesn't help much but it does
help with trying to load the same module over and over again, the explanation
I can think of for this is by introducing a lookup on finit_module() we also
delay module loading by the lookup time, in the general case we would not need
this, so this is likely not worth merging. Will run some final tests to
confirm.

  Luis

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

2017-01-10 Thread Luis R. Rodriguez 
On Fri, Dec 16, 2016 at 09:39:56AM +0100, Luis R. Rodriguez wrote:
> 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.

A simple change can enable us to bail out on finit_module() if a module
is already present by looking at the passed userspace data. I have this
change now but as discussed, whether or not its desirable should be a
matter of whether or not in the typical case (bootup time) things improve.
>From some initial tests it would seem this doesn't help much but it does
help with trying to load the same module over and over again, the explanation
I can think of for this is by introducing a lookup on finit_module() we also
delay module loading by the lookup time, in the general case we would not need
this, so this is likely not worth merging. Will run some final tests to
confirm.

  Luis

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

2016-12-16 Thread Luis R. Rodriguez 
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

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

2016-12-16 Thread Luis R. Rodriguez 
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

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

2016-12-14 Thread Petr Mladek 
On Thu 2016-12-08 11:48:14, Luis R. Rodriguez wrote:
> We currently statically limit the number of modprobe threads which
> we allow to run concurrently to 50. As per Keith Owens, this was a
> completely arbitrary value, and it was set in the 2.3.38 days [0]
> over 16 years ago in year 2000.
> 
> Although we haven't yet hit our lower limits, experimentation [1]
> shows that when and if we hit this limit in the worst case, will be
> fatal -- consider get_fs_type() failures upon mount on a system which
> has many partitions, some of which might even be with the same
> filesystem. Its best to be prudent and increase and set this
> value to something more sensible which ensures we're far from hitting
> the limit and also allows default build/user run time override.
> 
> The worst case is fatal given that once a module fails to load there
> is a period of time during which subsequent request for the same module
> will fail, so in the case of partitions its not just one request that
> could fail, but whole series of partitions. This later issue of a
> module request failure domino effect can be addressed later, but
> increasing the limit to something more meaninful should at least give us
> enough cushion to avoid this for a while.
> 
> Set this value up with a bit more meaninful modern limits:
> 
> Bump this up to 64  max for small systems (CONFIG_BASE_SMALL)
> Bump this up to 128 max for larger systems (!CONFIG_BASE_SMALL)
> 
> 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?
I am not sure how it is implemented but it might require having
some more module loads in progress.

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

> + default 6 if !BASE_SMALL
> + default 7 if BASE_SMALL

Aren't the conditions inversed?

> diff --git a/init/main.c b/init/main.c
> index 8161208d4ece..1fa441aa32c6 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -638,6 +638,7 @@ asmlinkage __visible void __init start_kernel(void)
>   thread_stack_cache_init();
>   cred_init();
>   fork_init();
> + init_kmod_umh();
>   proc_caches_init();
>   buffer_init();
>   key_init();
> diff --git a/kernel/kmod.c b/kernel/kmod.c
> index 0277d1216f80..cb6f7ca7b8a5 100644
> --- a/kernel/kmod.c
> +++ b/kernel/kmod.c
> @@ -186,6 +174,31 @@ int __request_module(bool wait, const char *fmt, ...)
>   return ret;
>  }
>  EXPORT_SYMBOL(__request_module);
> +
> +/*
> + * If modprobe needs a service that is in a module, we get a recursive
> + * loop.  Limit the number of running kmod threads to max_threads/2 or
> + * CONFIG_MAX_KMOD_CONCURRENT, whichever is the smaller.  A cleaner method
> + * would be to run the parents of this process, counting how many times
> + * kmod was invoked.  That would mean accessing the internals of the
> + * process tables to get the command line, proc_pid_cmdline is static
> + * and it is not worth changing the proc code just to handle this case.
> + *
> + * "trace the ppid" is simple, but will fail if someone's
> + * parent exits.  I think this is as good as it gets.
> + *
> + * You can override with with a kernel parameter, for instance to allow
> + * 4096 concurrent modprobe instances:
> + *
> + *   kmod.max_modprobes=4096
> + */
> +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.

Best Regards,
Petr

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

2016-12-14 Thread Petr Mladek 
On Thu 2016-12-08 11:48:14, Luis R. Rodriguez wrote:
> We currently statically limit the number of modprobe threads which
> we allow to run concurrently to 50. As per Keith Owens, this was a
> completely arbitrary value, and it was set in the 2.3.38 days [0]
> over 16 years ago in year 2000.
> 
> Although we haven't yet hit our lower limits, experimentation [1]
> shows that when and if we hit this limit in the worst case, will be
> fatal -- consider get_fs_type() failures upon mount on a system which
> has many partitions, some of which might even be with the same
> filesystem. Its best to be prudent and increase and set this
> value to something more sensible which ensures we're far from hitting
> the limit and also allows default build/user run time override.
> 
> The worst case is fatal given that once a module fails to load there
> is a period of time during which subsequent request for the same module
> will fail, so in the case of partitions its not just one request that
> could fail, but whole series of partitions. This later issue of a
> module request failure domino effect can be addressed later, but
> increasing the limit to something more meaninful should at least give us
> enough cushion to avoid this for a while.
> 
> Set this value up with a bit more meaninful modern limits:
> 
> Bump this up to 64  max for small systems (CONFIG_BASE_SMALL)
> Bump this up to 128 max for larger systems (!CONFIG_BASE_SMALL)
> 
> 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?
I am not sure how it is implemented but it might require having
some more module loads in progress.

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

> + default 6 if !BASE_SMALL
> + default 7 if BASE_SMALL

Aren't the conditions inversed?

> diff --git a/init/main.c b/init/main.c
> index 8161208d4ece..1fa441aa32c6 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -638,6 +638,7 @@ asmlinkage __visible void __init start_kernel(void)
>   thread_stack_cache_init();
>   cred_init();
>   fork_init();
> + init_kmod_umh();
>   proc_caches_init();
>   buffer_init();
>   key_init();
> diff --git a/kernel/kmod.c b/kernel/kmod.c
> index 0277d1216f80..cb6f7ca7b8a5 100644
> --- a/kernel/kmod.c
> +++ b/kernel/kmod.c
> @@ -186,6 +174,31 @@ int __request_module(bool wait, const char *fmt, ...)
>   return ret;
>  }
>  EXPORT_SYMBOL(__request_module);
> +
> +/*
> + * If modprobe needs a service that is in a module, we get a recursive
> + * loop.  Limit the number of running kmod threads to max_threads/2 or
> + * CONFIG_MAX_KMOD_CONCURRENT, whichever is the smaller.  A cleaner method
> + * would be to run the parents of this process, counting how many times
> + * kmod was invoked.  That would mean accessing the internals of the
> + * process tables to get the command line, proc_pid_cmdline is static
> + * and it is not worth changing the proc code just to handle this case.
> + *
> + * "trace the ppid" is simple, but will fail if someone's
> + * parent exits.  I think this is as good as it gets.
> + *
> + * You can override with with a kernel parameter, for instance to allow
> + * 4096 concurrent modprobe instances:
> + *
> + *   kmod.max_modprobes=4096
> + */
> +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.

Best Regards,
Petr

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

2016-12-08 Thread Luis R. Rodriguez 
On Thu, Dec 08, 2016 at 12:28:07PM -0800, Kees Cook wrote:
> On Thu, Dec 8, 2016 at 11:48 AM, Luis R. Rodriguez  wrote:
> > diff --git a/kernel/kmod.c b/kernel/kmod.c
> > index 0277d1216f80..cb6f7ca7b8a5 100644
> > --- a/kernel/kmod.c
> > +++ b/kernel/kmod.c
> > @@ -44,6 +44,9 @@
> > @@ -186,6 +174,31 @@ int __request_module(bool wait, const char *fmt, ...)
> > return ret;
> >  }
> >  EXPORT_SYMBOL(__request_module);
> > +
> > +/*
> > + * If modprobe needs a service that is in a module, we get a recursive
> > + * loop.  Limit the number of running kmod threads to max_threads/2 or
> > + * CONFIG_MAX_KMOD_CONCURRENT, whichever is the smaller.  A cleaner method
> > + * would be to run the parents of this process, counting how many times
> > + * kmod was invoked.  That would mean accessing the internals of the
> > + * process tables to get the command line, proc_pid_cmdline is static
> > + * and it is not worth changing the proc code just to handle this case.
> > + *
> > + * "trace the ppid" is simple, but will fail if someone's
> > + * parent exits.  I think this is as good as it gets.
> > + *
> > + * You can override with with a kernel parameter, for instance to allow
> > + * 4096 concurrent modprobe instances:
> > + *
> > + * kmod.max_modprobes=4096
> > + */
> > +void __init init_kmod_umh(void)
> 
> What does umh mean?

umh is user mode helper. kmod.c actually implements the kernel's umh code.
A subsequent series I will want to move all that to umh.c and keep module
loading separate in kmod.c But that's for later as a cleanup.

BTW any chance I can have you trim replies to file name and hunk for changes
you reply to ? As an example I did that here :)

  Luis

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

2016-12-08 Thread Luis R. Rodriguez 
On Thu, Dec 08, 2016 at 12:28:07PM -0800, Kees Cook wrote:
> On Thu, Dec 8, 2016 at 11:48 AM, Luis R. Rodriguez  wrote:
> > diff --git a/kernel/kmod.c b/kernel/kmod.c
> > index 0277d1216f80..cb6f7ca7b8a5 100644
> > --- a/kernel/kmod.c
> > +++ b/kernel/kmod.c
> > @@ -44,6 +44,9 @@
> > @@ -186,6 +174,31 @@ int __request_module(bool wait, const char *fmt, ...)
> > return ret;
> >  }
> >  EXPORT_SYMBOL(__request_module);
> > +
> > +/*
> > + * If modprobe needs a service that is in a module, we get a recursive
> > + * loop.  Limit the number of running kmod threads to max_threads/2 or
> > + * CONFIG_MAX_KMOD_CONCURRENT, whichever is the smaller.  A cleaner method
> > + * would be to run the parents of this process, counting how many times
> > + * kmod was invoked.  That would mean accessing the internals of the
> > + * process tables to get the command line, proc_pid_cmdline is static
> > + * and it is not worth changing the proc code just to handle this case.
> > + *
> > + * "trace the ppid" is simple, but will fail if someone's
> > + * parent exits.  I think this is as good as it gets.
> > + *
> > + * You can override with with a kernel parameter, for instance to allow
> > + * 4096 concurrent modprobe instances:
> > + *
> > + * kmod.max_modprobes=4096
> > + */
> > +void __init init_kmod_umh(void)
> 
> What does umh mean?

umh is user mode helper. kmod.c actually implements the kernel's umh code.
A subsequent series I will want to move all that to umh.c and keep module
loading separate in kmod.c But that's for later as a cleanup.

BTW any chance I can have you trim replies to file name and hunk for changes
you reply to ? As an example I did that here :)

  Luis

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

2016-12-08 Thread Kees Cook 
On Thu, Dec 8, 2016 at 11:48 AM, Luis R. Rodriguez  wrote:
> We currently statically limit the number of modprobe threads which
> we allow to run concurrently to 50. As per Keith Owens, this was a
> completely arbitrary value, and it was set in the 2.3.38 days [0]
> over 16 years ago in year 2000.
>
> Although we haven't yet hit our lower limits, experimentation [1]
> shows that when and if we hit this limit in the worst case, will be
> fatal -- consider get_fs_type() failures upon mount on a system which
> has many partitions, some of which might even be with the same
> filesystem. Its best to be prudent and increase and set this
> value to something more sensible which ensures we're far from hitting
> the limit and also allows default build/user run time override.
>
> The worst case is fatal given that once a module fails to load there
> is a period of time during which subsequent request for the same module
> will fail, so in the case of partitions its not just one request that
> could fail, but whole series of partitions. This later issue of a
> module request failure domino effect can be addressed later, but
> increasing the limit to something more meaninful should at least give us
> enough cushion to avoid this for a while.
>
> Set this value up with a bit more meaninful modern limits:
>
> Bump this up to 64  max for small systems (CONFIG_BASE_SMALL)
> Bump this up to 128 max for larger systems (!CONFIG_BASE_SMALL)
>
> Also allow the default max limit to be further fine tuned at compile
> time and at initialization at run time at boot up using the kernel
> parameter: max_modprobes.
>
> [0] 
> https://git.kernel.org/cgit/linux/kernel/git/history/history.git/commit/?id=ab1c4ec7410f6ec64e1511d1a7d850fc99c09b44
> [1] https://github.com/mcgrof/test_request_module
>
> Signed-off-by: Luis R. Rodriguez 
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  7 
>  include/linux/kmod.h|  3 +-
>  init/Kconfig| 23 +
>  init/main.c |  1 +
>  kernel/kmod.c   | 43 
> -
>  5 files changed, 61 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
> b/Documentation/admin-guide/kernel-parameters.txt
> index be2d6d0a03a4..92b65ea4 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1700,6 +1700,13 @@
>
> keepinitrd  [HW,ARM]
>
> +   kmod.max_modprobes [KNL]
> +   This lets you set the max allowed of concurrent
> +   modprobes threads possible on a system overriding the
> +   default heuristic of:
> +
> +   min(max_threads/2, 2 << 
> CONFIG_MAX_KMOD_CONCURRENT)
> +
> kernelcore= [KNL,X86,IA-64,PPC]
> Format: nn[KMGTPE] | "mirror"
> This parameter
> diff --git a/include/linux/kmod.h b/include/linux/kmod.h
> index fcfd2bf14d3f..15783cd7f056 100644
> --- a/include/linux/kmod.h
> +++ b/include/linux/kmod.h
> @@ -38,13 +38,14 @@ int __request_module(bool wait, const char *name, ...);
>  #define request_module_nowait(mod...) __request_module(false, mod)
>  #define try_then_request_module(x, mod...) \
> ((x) ?: (__request_module(true, mod), (x)))
> +void init_kmod_umh(void);
>  #else
>  static inline int request_module(const char *name, ...) { return -ENOSYS; }
>  static inline int request_module_nowait(const char *name, ...) { return 
> -ENOSYS; }
> +static inline void init_kmod_umh(void) { }
>  #define try_then_request_module(x, mod...) (x)
>  #endif
>
> -
>  struct cred;
>  struct file;
>
> 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
> +   default 6 if !BASE_SMALL
> +   default 7 if BASE_SMALL
> +   help
> + The kernel restricts the number of possible concurrent calls to
> + request_module() to help avoid a recursive loop possible with
> + modules. The default maximum number of concurrent threads allowed
> + to run request_module() will be:
> +
> +   max_modprobes = min(max_threads/2, 2 << 
> CONFIG_MAX_KMOD_CONCURRENT);
> +
> + The value set in CONFIG_MAX_KMOD_CONCURRENT represents then the 
> power
> + of 2 value used at boot time for the above computation. You can
> + override the default built value using the kernel parameter:
> +
> +   kmod.max_modprobes=4096
> +
> + We set this to default to

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

2016-12-08 Thread Kees Cook 
On Thu, Dec 8, 2016 at 11:48 AM, Luis R. Rodriguez  wrote:
> We currently statically limit the number of modprobe threads which
> we allow to run concurrently to 50. As per Keith Owens, this was a
> completely arbitrary value, and it was set in the 2.3.38 days [0]
> over 16 years ago in year 2000.
>
> Although we haven't yet hit our lower limits, experimentation [1]
> shows that when and if we hit this limit in the worst case, will be
> fatal -- consider get_fs_type() failures upon mount on a system which
> has many partitions, some of which might even be with the same
> filesystem. Its best to be prudent and increase and set this
> value to something more sensible which ensures we're far from hitting
> the limit and also allows default build/user run time override.
>
> The worst case is fatal given that once a module fails to load there
> is a period of time during which subsequent request for the same module
> will fail, so in the case of partitions its not just one request that
> could fail, but whole series of partitions. This later issue of a
> module request failure domino effect can be addressed later, but
> increasing the limit to something more meaninful should at least give us
> enough cushion to avoid this for a while.
>
> Set this value up with a bit more meaninful modern limits:
>
> Bump this up to 64  max for small systems (CONFIG_BASE_SMALL)
> Bump this up to 128 max for larger systems (!CONFIG_BASE_SMALL)
>
> Also allow the default max limit to be further fine tuned at compile
> time and at initialization at run time at boot up using the kernel
> parameter: max_modprobes.
>
> [0] 
> https://git.kernel.org/cgit/linux/kernel/git/history/history.git/commit/?id=ab1c4ec7410f6ec64e1511d1a7d850fc99c09b44
> [1] https://github.com/mcgrof/test_request_module
>
> Signed-off-by: Luis R. Rodriguez 
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  7 
>  include/linux/kmod.h|  3 +-
>  init/Kconfig| 23 +
>  init/main.c |  1 +
>  kernel/kmod.c   | 43 
> -
>  5 files changed, 61 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
> b/Documentation/admin-guide/kernel-parameters.txt
> index be2d6d0a03a4..92b65ea4 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1700,6 +1700,13 @@
>
> keepinitrd  [HW,ARM]
>
> +   kmod.max_modprobes [KNL]
> +   This lets you set the max allowed of concurrent
> +   modprobes threads possible on a system overriding the
> +   default heuristic of:
> +
> +   min(max_threads/2, 2 << 
> CONFIG_MAX_KMOD_CONCURRENT)
> +
> kernelcore= [KNL,X86,IA-64,PPC]
> Format: nn[KMGTPE] | "mirror"
> This parameter
> diff --git a/include/linux/kmod.h b/include/linux/kmod.h
> index fcfd2bf14d3f..15783cd7f056 100644
> --- a/include/linux/kmod.h
> +++ b/include/linux/kmod.h
> @@ -38,13 +38,14 @@ int __request_module(bool wait, const char *name, ...);
>  #define request_module_nowait(mod...) __request_module(false, mod)
>  #define try_then_request_module(x, mod...) \
> ((x) ?: (__request_module(true, mod), (x)))
> +void init_kmod_umh(void);
>  #else
>  static inline int request_module(const char *name, ...) { return -ENOSYS; }
>  static inline int request_module_nowait(const char *name, ...) { return 
> -ENOSYS; }
> +static inline void init_kmod_umh(void) { }
>  #define try_then_request_module(x, mod...) (x)
>  #endif
>
> -
>  struct cred;
>  struct file;
>
> 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
> +   default 6 if !BASE_SMALL
> +   default 7 if BASE_SMALL
> +   help
> + The kernel restricts the number of possible concurrent calls to
> + request_module() to help avoid a recursive loop possible with
> + modules. The default maximum number of concurrent threads allowed
> + to run request_module() will be:
> +
> +   max_modprobes = min(max_threads/2, 2 << 
> CONFIG_MAX_KMOD_CONCURRENT);
> +
> + The value set in CONFIG_MAX_KMOD_CONCURRENT represents then the 
> power
> + of 2 value used at boot time for the above computation. You can
> + override the default built value using the kernel parameter:
> +
> +   kmod.max_modprobes=4096
> +
> + We set this to default to 64 (2^6) concurrent modprobe threads

[RFC 03/10] kmod: add dynamic max concurrent thread count

2016-12-08 Thread Luis R. Rodriguez 
We currently statically limit the number of modprobe threads which
we allow to run concurrently to 50. As per Keith Owens, this was a
completely arbitrary value, and it was set in the 2.3.38 days [0]
over 16 years ago in year 2000.

Although we haven't yet hit our lower limits, experimentation [1]
shows that when and if we hit this limit in the worst case, will be
fatal -- consider get_fs_type() failures upon mount on a system which
has many partitions, some of which might even be with the same
filesystem. Its best to be prudent and increase and set this
value to something more sensible which ensures we're far from hitting
the limit and also allows default build/user run time override.

The worst case is fatal given that once a module fails to load there
is a period of time during which subsequent request for the same module
will fail, so in the case of partitions its not just one request that
could fail, but whole series of partitions. This later issue of a
module request failure domino effect can be addressed later, but
increasing the limit to something more meaninful should at least give us
enough cushion to avoid this for a while.

Set this value up with a bit more meaninful modern limits:

Bump this up to 64  max for small systems (CONFIG_BASE_SMALL)
Bump this up to 128 max for larger systems (!CONFIG_BASE_SMALL)

Also allow the default max limit to be further fine tuned at compile
time and at initialization at run time at boot up using the kernel
parameter: max_modprobes.

[0] 
https://git.kernel.org/cgit/linux/kernel/git/history/history.git/commit/?id=ab1c4ec7410f6ec64e1511d1a7d850fc99c09b44
[1] https://github.com/mcgrof/test_request_module

Signed-off-by: Luis R. Rodriguez 
---
 Documentation/admin-guide/kernel-parameters.txt |  7 
 include/linux/kmod.h|  3 +-
 init/Kconfig| 23 +
 init/main.c |  1 +
 kernel/kmod.c   | 43 -
 5 files changed, 61 insertions(+), 16 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index be2d6d0a03a4..92b65ea4 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1700,6 +1700,13 @@
 
keepinitrd  [HW,ARM]
 
+   kmod.max_modprobes [KNL]
+   This lets you set the max allowed of concurrent
+   modprobes threads possible on a system overriding the
+   default heuristic of:
+
+   min(max_threads/2, 2 << 
CONFIG_MAX_KMOD_CONCURRENT)
+
kernelcore= [KNL,X86,IA-64,PPC]
Format: nn[KMGTPE] | "mirror"
This parameter
diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index fcfd2bf14d3f..15783cd7f056 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -38,13 +38,14 @@ int __request_module(bool wait, const char *name, ...);
 #define request_module_nowait(mod...) __request_module(false, mod)
 #define try_then_request_module(x, mod...) \
((x) ?: (__request_module(true, mod), (x)))
+void init_kmod_umh(void);
 #else
 static inline int request_module(const char *name, ...) { return -ENOSYS; }
 static inline int request_module_nowait(const char *name, ...) { return 
-ENOSYS; }
+static inline void init_kmod_umh(void) { }
 #define try_then_request_module(x, mod...) (x)
 #endif
 
-
 struct cred;
 struct file;
 
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
+   default 6 if !BASE_SMALL
+   default 7 if BASE_SMALL
+   help
+ The kernel restricts the number of possible concurrent calls to
+ request_module() to help avoid a recursive loop possible with
+ modules. The default maximum number of concurrent threads allowed
+ to run request_module() will be:
+
+   max_modprobes = min(max_threads/2, 2 << CONFIG_MAX_KMOD_CONCURRENT);
+
+ The value set in CONFIG_MAX_KMOD_CONCURRENT represents then the power
+ of 2 value used at boot time for the above computation. You can
+ override the default built value using the kernel parameter:
+
+   kmod.max_modprobes=4096
+
+ We set this to default to 64 (2^6) concurrent modprobe threads for
+ small systems, for larger systems this defaults to 128 (2^7)
+ concurrent modprobe threads.
+
 endif # MODULES
 
 config MODULES_TREE_LOOKUP
diff --git a/init/main.c b/init/main.c
index 8161208d4ece..1fa441aa32c6 100644
---

[RFC 03/10] kmod: add dynamic max concurrent thread count

2016-12-08 Thread Luis R. Rodriguez 
We currently statically limit the number of modprobe threads which
we allow to run concurrently to 50. As per Keith Owens, this was a
completely arbitrary value, and it was set in the 2.3.38 days [0]
over 16 years ago in year 2000.

Although we haven't yet hit our lower limits, experimentation [1]
shows that when and if we hit this limit in the worst case, will be
fatal -- consider get_fs_type() failures upon mount on a system which
has many partitions, some of which might even be with the same
filesystem. Its best to be prudent and increase and set this
value to something more sensible which ensures we're far from hitting
the limit and also allows default build/user run time override.

The worst case is fatal given that once a module fails to load there
is a period of time during which subsequent request for the same module
will fail, so in the case of partitions its not just one request that
could fail, but whole series of partitions. This later issue of a
module request failure domino effect can be addressed later, but
increasing the limit to something more meaninful should at least give us
enough cushion to avoid this for a while.

Set this value up with a bit more meaninful modern limits:

Bump this up to 64  max for small systems (CONFIG_BASE_SMALL)
Bump this up to 128 max for larger systems (!CONFIG_BASE_SMALL)

Also allow the default max limit to be further fine tuned at compile
time and at initialization at run time at boot up using the kernel
parameter: max_modprobes.

[0] 
https://git.kernel.org/cgit/linux/kernel/git/history/history.git/commit/?id=ab1c4ec7410f6ec64e1511d1a7d850fc99c09b44
[1] https://github.com/mcgrof/test_request_module

Signed-off-by: Luis R. Rodriguez 
---
 Documentation/admin-guide/kernel-parameters.txt |  7 
 include/linux/kmod.h|  3 +-
 init/Kconfig| 23 +
 init/main.c |  1 +
 kernel/kmod.c   | 43 -
 5 files changed, 61 insertions(+), 16 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index be2d6d0a03a4..92b65ea4 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1700,6 +1700,13 @@
 
keepinitrd  [HW,ARM]
 
+   kmod.max_modprobes [KNL]
+   This lets you set the max allowed of concurrent
+   modprobes threads possible on a system overriding the
+   default heuristic of:
+
+   min(max_threads/2, 2 << 
CONFIG_MAX_KMOD_CONCURRENT)
+
kernelcore= [KNL,X86,IA-64,PPC]
Format: nn[KMGTPE] | "mirror"
This parameter
diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index fcfd2bf14d3f..15783cd7f056 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -38,13 +38,14 @@ int __request_module(bool wait, const char *name, ...);
 #define request_module_nowait(mod...) __request_module(false, mod)
 #define try_then_request_module(x, mod...) \
((x) ?: (__request_module(true, mod), (x)))
+void init_kmod_umh(void);
 #else
 static inline int request_module(const char *name, ...) { return -ENOSYS; }
 static inline int request_module_nowait(const char *name, ...) { return 
-ENOSYS; }
+static inline void init_kmod_umh(void) { }
 #define try_then_request_module(x, mod...) (x)
 #endif
 
-
 struct cred;
 struct file;
 
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
+   default 6 if !BASE_SMALL
+   default 7 if BASE_SMALL
+   help
+ The kernel restricts the number of possible concurrent calls to
+ request_module() to help avoid a recursive loop possible with
+ modules. The default maximum number of concurrent threads allowed
+ to run request_module() will be:
+
+   max_modprobes = min(max_threads/2, 2 << CONFIG_MAX_KMOD_CONCURRENT);
+
+ The value set in CONFIG_MAX_KMOD_CONCURRENT represents then the power
+ of 2 value used at boot time for the above computation. You can
+ override the default built value using the kernel parameter:
+
+   kmod.max_modprobes=4096
+
+ We set this to default to 64 (2^6) concurrent modprobe threads for
+ small systems, for larger systems this defaults to 128 (2^7)
+ concurrent modprobe threads.
+
 endif # MODULES
 
 config MODULES_TREE_LOOKUP
diff --git a/init/main.c b/init/main.c
index 8161208d4ece..1fa441aa32c6 100644
--- a/init/main.c
+++