From: "Luis R. Rodriguez" <mcgrof@kernel.org>
To: Matt Redfearn <matt.redfearn@imgtec.com>
Cc: "Luis R. Rodriguez" <mcgrof@kernel.org>,
Kees Cook <keescook@chromium.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Andrew Morton <akpm@linux-foundation.org>,
David Howells <dhowells@redhat.com>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
Dan Carpenter <dan.carpenter@oracle.com>,
Jessica Yu <jeyu@redhat.com>, Michal Marek <mmarek@suse.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Linux MIPS Mailing List <linux-mips@linux-mips.org>,
ebiederm@xmission.com, mingo@redhat.com, peterz@infradead.org,
Petr Mladek <pmladek@suse.com>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH] exec: Avoid recursive modprobe for binary format handlers
Date: Tue, 8 Aug 2017 21:23:52 +0200 [thread overview]
Message-ID: <20170808192352.GU27873@wotan.suse.de> (raw)
In-Reply-To: <757118c9-45e2-9680-dca2-079d928d9b1c@imgtec.com>
On Mon, Aug 07, 2017 at 11:26:09AM +0100, Matt Redfearn wrote:
> Hi Luis,
> On 03/08/17 00:23, Luis R. Rodriguez wrote:
> > On Tue, Aug 01, 2017 at 07:28:20PM -0700, Kees Cook wrote:
> > > On Tue, Aug 1, 2017 at 5:12 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> > > > On Fri, Jul 21, 2017 at 03:05:20PM +0100, Matt Redfearn wrote:
> > > > > Commit 6d7964a722af ("kmod: throttle kmod thread limit") which was
> > > > > merged in v4.13-rc1 broke this behaviour since the recursive modprobe is
> > > > > no longer caught, it just ends up waiting indefinitely for the kmod_wq
> > > > > wait queue. Hence the kernel appears to hang silently when starting
> > > > > userspace.
> > > > Indeed, the recursive issue were no longer expected to exist.
> > > Errr, yeah, recursive binfmt loads can still happen.
> > >
> > > > The *old* implementation would also prevent a set of binaries to daisy chain
> > > > a set of 50 different binaries which require different binfmt loaders. The
> > > > current implementation enables this and we'd just wait. There's a bound to
> > > > the number of binfmd loaders though, so this would be bounded. If however
> > > > a 2nd loader loaded the first binary we'd run into the same issue I think.
> > > >
> > > > If we can't think of a good way to resolve this we'll just have to revert
> > > > 6d7964a722af for now.
> > > The weird but "normal" recursive case is usually a script calling a
> > > script calling a misc format. Getting a chain of modprobes running,
> > > though, seems unlikely. I *think* Matt's patch is okay, but I agree,
> > > it'd be better for the request_module() to fail.
> > In that case how about we just have each waiter only wait max X seconds,
> > if the number of concurrent ongoing modprobe calls hasn't reduced by
> > a single digit in X seconds we give up on request_module() for the
> > module and clearly indicate what happened.
> >
> > Matt, can you test?
>
> Sure - I've tested patch this on Cavium Octeon under the same conditions as
> before (64 bit kernel with 32bit userspace & no binfmt handler builtin).
>
> The failing modprobe is now caught and rather than silence we get the
> expected kernel panic, albeit all the warnings look quite noisy.
Thanks for testing! I agree its all too verbose, I'll clean that up and
resubmit with a cleaner log.
I tried to devise a test case for this but for the life of me I could not. If
you happen to come up with something please feel free to submit one for
lib/test_kmod.c!
> In any case, this is better than the 4.13-rc1 behavior, so
>
> Tested-by: Matt Redfearn <matt.redfearn@imgetc.com>
Luis
next prev parent reply other threads:[~2017-08-08 19:23 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-21 14:05 [RFC PATCH] exec: Avoid recursive modprobe for binary format handlers Matt Redfearn
2017-08-02 0:12 ` Luis R. Rodriguez
2017-08-02 2:28 ` Kees Cook
2017-08-02 23:23 ` Luis R. Rodriguez
2017-08-04 0:02 ` Kees Cook
2017-08-04 0:10 ` Luis R. Rodriguez
2017-08-07 10:26 ` Matt Redfearn
2017-08-08 19:23 ` Luis R. Rodriguez [this message]
2017-08-09 0:09 ` Luis R. Rodriguez
2017-09-08 21:23 ` Lucas De Marchi
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=20170808192352.GU27873@wotan.suse.de \
--to=mcgrof@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=dan.carpenter@oracle.com \
--cc=dhowells@redhat.com \
--cc=dmitry.torokhov@gmail.com \
--cc=ebiederm@xmission.com \
--cc=gregkh@linuxfoundation.org \
--cc=jeyu@redhat.com \
--cc=keescook@chromium.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@linux-mips.org \
--cc=matt.redfearn@imgtec.com \
--cc=mingo@redhat.com \
--cc=mmarek@suse.com \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).