Message-Id: <20170809234635.13443-1-mcgrof@kernel.org>
Date:   Wed,  9 Aug 2017 16:46:32 -0700
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     akpm@...ux-foundation.org
Cc:     mingo@...hat.com, peterz@...radead.org, keescook@...omium.org,
        dmitry.torokhov@...il.com, jeyu@...hat.com, rusty@...tcorp.com.au,
        mmarek@...e.com, pmladek@...e.com, mbenes@...e.cz,
        jpoimboe@...hat.com, ebiederm@...ssion.com, shuah@...nel.org,
        matt.redfearn@...tec.com, dan.carpenter@...cle.com,
        colin.king@...onical.com, danielmentz@...gle.com,
        dcb314@...mail.com, linux-kselftest@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        "Luis R. Rodriguez" <mcgrof@...nel.org>
Subject: [PATCH 0/3] kmod: pending fixes for v4.13-final

Andrew,

These are the few pending fixes I have queued up for v4.13-final. One is a
a generic regression fix for recursive loops on kmod and the other one is a
trivial print out correction.

During the v4.13 development we assumed that recursive kmod loops were no
longer possible. Clearly that is not true. The regression fix makes use of a
new killable wait. We use a killable wait to be paranoid in how signals might
be sent to modprobe and only accept a proper SIGKILL. The signal will only be
available to userspace to issue *iff* a thread has already entered a wait
state, and that happens only if we've already throttled after 50 kmod threads
have been hit.

These fixes are also available on my linux git tree branch
20170809-kmod-for-v4.13-final [0] which also has contains other fixes I had
previously sent to you and Shuah for inclusion for v4.13-final.

Note that although it may seem excessive to trigger a failure afer 5 seconds
if all kmod thread remain busy, prior to the series of changes that went into
v4.13 we would actually *always* fatally fail any request which came in if
the limit was already reached. The new waiting implemented in v4.13 actually
gives us *more* breathing room -- the wait for 5 seconds is a wait for *any*
kmod thread to finish. We give up and fail *iff* no kmod thread has finished
and they're *all* running straight for 5 consecutive seconds. If 50 kmod
threads are running consecutively for 5 seconds something else must be really
bad.

Recursive loops with kmod are bad but they're also hard to implement properly
as a selftest without currently fooling current userspace tools like kmod [1].
For instance kmod will complain when you run depmod if it finds a recursive
loop with symbol dependency between modules as such this type of recursive loop
cannot go upstream as the modules_install target will fail after running
depmod. These tests already exist on userspace kmod upstream though (refer to
the testsuite/module-playground/mod-loop-*.c files). The same is not true if
request_module() is used though, or worst if aliases are used. Likewise the
issue with 64-bit kernels booting 32-bit userspace without a binfmt handler
built-in is also currently not detected and proactively avoided by userspace
kmod tools, or kconfig for all architectures. Although we could complain in the
kernel when some of these individual recursive issues creep up, proactively
avoiding these situations in userspace at build time is what we should keep
striving for.

Lastly, since recursive loops could happen with kmod it may mean recursive
loops may also be possible with other kernel usermode helpers, this should
be investigated and long term if we can come up with a more sensible generic
solution even better!

If there are any questions please let me know.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=20170809-kmod-for-v4.13-final
[1] https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git

  Luis

Luis R. Rodriguez (3):
  wait: add wait_event_killable_timeout()
  kmod: fix wait on recursive loop
  test_kmod: fix description for -s -and -c parameters

 include/linux/wait.h                 | 37 ++++++++++++++++++++++++++++++++++++
 kernel/kmod.c                        | 25 ++++++++++++++++++++++--
 tools/testing/selftests/kmod/kmod.sh |  4 ++--
 3 files changed, 62 insertions(+), 4 deletions(-)

-- 
2.14.0

Powered by blists - more mailing lists