Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752205AbdHHTX4 (ORCPT ); Tue, 8 Aug 2017 15:23:56 -0400 Received: from mx2.suse.de ([195.135.220.15]:57642 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752030AbdHHTXz (ORCPT ); Tue, 8 Aug 2017 15:23:55 -0400 Date: Tue, 8 Aug 2017 21:23:52 +0200 From: "Luis R. Rodriguez" To: Matt Redfearn Cc: "Luis R. Rodriguez" , Kees Cook , Alexander Viro , Andrew Morton , David Howells , Dmitry Torokhov , Dan Carpenter , Jessica Yu , Michal Marek , Linus Torvalds , Greg Kroah-Hartman , Linux MIPS Mailing List , ebiederm@xmission.com, mingo@redhat.com, peterz@infradead.org, Petr Mladek , "linux-fsdevel@vger.kernel.org" , LKML Subject: Re: [RFC PATCH] exec: Avoid recursive modprobe for binary format handlers Message-ID: <20170808192352.GU27873@wotan.suse.de> References: <1500645920-28490-1-git-send-email-matt.redfearn@imgtec.com> <20170802001200.GD18884@wotan.suse.de> <20170802232331.GO18884@wotan.suse.de> <757118c9-45e2-9680-dca2-079d928d9b1c@imgtec.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <757118c9-45e2-9680-dca2-079d928d9b1c@imgtec.com> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2619 Lines: 51 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 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 Luis