MIME-Version: 1.0
In-Reply-To: <20170802001200.GD18884@wotan.suse.de>
References: <1500645920-28490-1-git-send-email-matt.redfearn@imgtec.com> <20170802001200.GD18884@wotan.suse.de>
From: Kees Cook <keescook@chromium.org>
Date: Tue, 1 Aug 2017 19:28:20 -0700
Message-ID: <CAGXu5jJw74M0hTL8JGUtshgZpGjzWia2d=oK3t8oJF6qo9Xp_A@mail.gmail.com>
Subject: Re: [RFC PATCH] exec: Avoid recursive modprobe for binary format handlers
To: "Luis R. Rodriguez" <mcgrof@kernel.org>
Cc: Matt Redfearn <matt.redfearn@imgtec.com>,
        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>,
        Petr Mladek <pmladek@suse.com>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1348
Lines: 31

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.

-Kees

-- 
Kees Cook
Pixel Security