All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jessica Yu <jeyu@redhat.com>
To: "Luis R. Rodriguez" <mcgrof@kernel.org>
Cc: shuah@kernel.org, rusty@rustcorp.com.au, ebiederm@xmission.com,
	dmitry.torokhov@gmail.com, acme@redhat.com, corbet@lwn.net,
	martin.wilck@suse.com, mmarek@suse.com, pmladek@suse.com,
	hare@suse.com, rwright@hpe.com, jeffm@suse.com, DSterba@suse.com,
	fdmanana@suse.com, neilb@suse.com, linux@roeck-us.net,
	rgoldwyn@suse.com, subashab@codeaurora.org, xypron.glpk@gmx.de,
	keescook@chromium.org, atomlin@redhat.com, mbenes@suse.cz,
	paulmck@linux.vnet.ibm.com, dan.j.williams@intel.com,
	jpoimboe@redhat.com, davem@davemloft.net, mingo@redhat.com,
	akpm@linux-foundation.org, torvalds@linux-foundation.org,
	linux-kselftest@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: kmod: add a sanity check on module loading
Date: Tue, 3 Jan 2017 18:47:45 -0800	[thread overview]
Message-ID: <20170104024745.t3selvpcqcbtfxfs@jeyu> (raw)
In-Reply-To: <20161208194930.2816-1-mcgrof@kernel.org>

+++ Luis R. Rodriguez [08/12/16 11:49 -0800]:
>kmod has an optimization in place whereby if a some kernel code
>uses request_module() on a module already loaded we never bother
>userspace as the module already is loaded. This is not true for
>get_fs_type() though as it uses aliases.
>
>Additionally kmod <= v19 was broken -- it returns 0 to modprobe calls,
>assuming the kernel module is built-in, where really we have a race as
>the module starts forming. kmod <= v19 has incorrect userspace heuristics,
>a userspace kmod fix is available for it:
>
>http://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/libkmod/libkmod-module.c?id=fd44a98ae2eb5eb32161088954ab21e58e19dfc4
>
>This changes kmod to address both:
>
> o Provides the alias optimization for get_fs_type() so modules already
>   loaded do not get re-requested.
>
> o Provides a sanity test to verify modprobe's work
>
>This is important given how any get_fs_type() users assert success
>means we're ready to go, and tests with the new test_kmod stress driver
>reveal that request_module() and get_fs_type() might fail for a few
>other reasons. You don't need old kmod to fail on request_module() or
>get_fs_type(), with the right system setup, these calls *can* fail
>today.
>
>Although this does get us in the business of keeping alias maps in
>kernel, the the work to support and maintain this is trivial.
>Aditionally, since it may be important get_fs_type() should not fail on
>certain systems, this tightens things up a bit more.
>
>The TL;DR:
>
>kmod <= v19 will return 0 on modprobe calls if you are built-in,
>however its heuristics for checking if you are built-in were broken.
>
>It assumed that having the directory /sys/module/module-name
>but not having the file /sys/module/module-name/initstate
>is sufficient to assume a module is built-in.
>
>The kernel loads the inittstate attribute *after* it creates the
>directory. This is an issue when modprobe returns 0 for kernel calls
>which assumes a return of 0 on request_module() can give you the
>right to assert the module is loaded and live.
>
>We cannot trust returns of modprobe as 0 in the kernel, we need to
>verify that modules are live if modprobe return 0 but only if modules
>*are* modules. The kernel heuristic we use to determine if a module is
>built-in is that if modprobe returns 0 we know we must be built-in or
>a module, but if we are a module clearly we must have a lingering kmod
>dangling on our linked list. If there is no modules there we are *somewhat*
>certain the module must be built in.
>
>This is not enough though... we cannot easily work around this since the
>kernel can use aliases to userspace for modules calls. For instance
>fs/namespace.c uses fs-modulename for filesystesms on get_fs_type(), so
>these need to be taken into consideration as well.
>
>Using kmod <= 19 will give you a NULL get_fs_type() return even though
>the module was loaded... That is a corner case, there are other failures
>for request_module() though -- the other failures are not easy to
>reproduce though but fortunately we have a stress test driver to help
>with that now. Use the following tests:
>
> # tools/testing/selftests/kmod/kmod.sh -t 0008
> # tools/testing/selftests/kmod/kmod.sh -t 0009
>
>You can more easily see this error if you have kmod <= v19 installed.
>
>You will need to install kmod <= v19, be sure to install its modprobe
>into /sbin/ as by default the 'make install' target does not replace
>your own.
>
>This test helps cure test_kmod cases 0008 0009 so enable them.
>
>Reported-by: Martin Wilck <martin.wilck@suse.com>
>Reported-by: Randy Wright <rwright@hpe.com>
>Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>

Back from travel today, apologies for the delay. Will be able to give
this a proper look this week.

Jessica

  parent reply	other threads:[~2017-01-04  2:54 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-08 18:47 [RFC 00/10] kmod: stress test driver, few fixes and enhancements Luis R. Rodriguez
2016-12-08 18:47 ` [RFC 01/10] kmod: add test driver to stress test the module loader Luis R. Rodriguez
2016-12-08 20:24   ` Kees Cook
2016-12-13 21:10     ` Luis R. Rodriguez
2016-12-16  7:41       ` Luis R. Rodriguez
2016-12-08 19:48 ` [RFC 02/10] module: fix memory leak on early load_module() failures Luis R. Rodriguez
2016-12-08 20:30   ` Kees Cook
2016-12-08 21:10     ` Luis R. Rodriguez
2016-12-08 21:17       ` Kees Cook
2016-12-09 17:06   ` Miroslav Benes
2016-12-16  8:51     ` Luis R. Rodriguez
2016-12-15 18:46   ` Aaron Tomlin
2016-12-08 19:48 ` [RFC 03/10] kmod: add dynamic max concurrent thread count Luis R. Rodriguez
2016-12-08 20:28   ` Kees Cook
2016-12-08 21:00     ` Luis R. Rodriguez
2016-12-14 15:38   ` Petr Mladek
2016-12-16  8:39     ` Luis R. Rodriguez
2017-01-10 19:24       ` Luis R. Rodriguez
2016-12-08 19:48 ` [RFC 04/10] kmod: provide wrappers for kmod_concurrent inc/dec Luis R. Rodriguez
2016-12-08 20:29   ` Kees Cook
2016-12-08 21:08     ` Luis R. Rodriguez
2016-12-15 12:46       ` Petr Mladek
2016-12-16  8:05         ` Luis R. Rodriguez
2016-12-22  4:48           ` Jessica Yu
2017-01-06 20:54             ` Luis R. Rodriguez
2017-01-10 18:57           ` [RFC 04/10] " Luis R. Rodriguez
2017-01-11 20:08             ` Luis R. Rodriguez
2017-05-16 18:02               ` Luis R. Rodriguez
2017-05-18  2:37                 ` Luis R. Rodriguez
2016-12-22  5:07   ` Jessica Yu
2017-01-10 20:28     ` Luis R. Rodriguez
2016-12-08 19:48 ` [RFC 05/10] kmod: return -EBUSY if modprobe limit is reached Luis R. Rodriguez
2016-12-08 19:48 ` [RFC 06/10] kmod: provide sanity check on kmod_concurrent access Luis R. Rodriguez
2016-12-14 16:08   ` Petr Mladek
2016-12-14 17:12     ` Luis R. Rodriguez
2016-12-15 12:57   ` Petr Mladek
2017-01-10 20:00     ` Luis R. Rodriguez
2016-12-08 19:49 ` [RFC 07/10] kmod: use simplified rate limit printk Luis R. Rodriguez
2016-12-14 16:23   ` Petr Mladek
2016-12-14 16:41     ` Joe Perches
2016-12-16  8:44     ` Luis R. Rodriguez
2016-12-08 19:49 ` [RFC 08/10] sysctl: add support for unsigned int properly Luis R. Rodriguez
2016-12-08 19:49 ` [RFC 09/10] kmod: add helpers for getting kmod count and limit Luis R. Rodriguez
2016-12-15 16:56   ` Petr Mladek
2016-12-16  7:57     ` Luis R. Rodriguez
2017-01-11 18:27       ` Luis R. Rodriguez
2016-12-08 19:49 ` [RFC 10/10] kmod: add a sanity check on module loading Luis R. Rodriguez
2016-12-09 20:03   ` Martin Wilck
2016-12-09 20:56     ` Linus Torvalds
2016-12-15 18:08       ` Luis R. Rodriguez
2016-12-15  0:27   ` Rusty Russell
2016-12-16  8:31     ` Luis R. Rodriguez
2016-12-17  3:54       ` Rusty Russell
     [not found]         ` <CAB=NE6VvuA9a6hf6yoopGfUxVJQM5HyV5bNzUdsEtUV0UhbG-g@mail.gmail.com>
2016-12-20  0:53           ` Rusty Russell
2016-12-20 18:52             ` Luis R. Rodriguez
2016-12-21  2:21               ` Rusty Russell
2016-12-21 13:08                 ` Luis R. Rodriguez
2017-01-03  0:04                   ` Rusty Russell
2017-01-06 20:36                     ` Luis R. Rodriguez
2017-01-06 21:53                       ` Jessica Yu
2017-01-09 20:27                         ` Luis R. Rodriguez
     [not found]                       ` <87bmvgax51.fsf@rustcorp.com.au>
2017-01-09 19:56                         ` [RFC 10/10] " Luis R. Rodriguez
2017-01-06 21:03                     ` Jessica Yu
2017-01-04  2:47   ` Jessica Yu [this message]
2017-01-11 19:10 ` [RFC 00/10] kmod: stress test driver, few fixes and enhancements Luis R. Rodriguez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170104024745.t3selvpcqcbtfxfs@jeyu \
    --to=jeyu@redhat.com \
    --cc=DSterba@suse.com \
    --cc=acme@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=atomlin@redhat.com \
    --cc=corbet@lwn.net \
    --cc=dan.j.williams@intel.com \
    --cc=davem@davemloft.net \
    --cc=dmitry.torokhov@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=fdmanana@suse.com \
    --cc=hare@suse.com \
    --cc=jeffm@suse.com \
    --cc=jpoimboe@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=martin.wilck@suse.com \
    --cc=mbenes@suse.cz \
    --cc=mcgrof@kernel.org \
    --cc=mingo@redhat.com \
    --cc=mmarek@suse.com \
    --cc=neilb@suse.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=pmladek@suse.com \
    --cc=rgoldwyn@suse.com \
    --cc=rusty@rustcorp.com.au \
    --cc=rwright@hpe.com \
    --cc=shuah@kernel.org \
    --cc=subashab@codeaurora.org \
    --cc=torvalds@linux-foundation.org \
    --cc=xypron.glpk@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.