From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761022AbdADCyv (ORCPT ); Tue, 3 Jan 2017 21:54:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44986 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751014AbdADCym (ORCPT ); Tue, 3 Jan 2017 21:54:42 -0500 Date: Tue, 3 Jan 2017 18:47:45 -0800 From: Jessica Yu To: "Luis R. Rodriguez" 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 Message-ID: <20170104024745.t3selvpcqcbtfxfs@jeyu> References: <20161208184801.1689-1-mcgrof@kernel.org> <20161208194930.2816-1-mcgrof@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20161208194930.2816-1-mcgrof@kernel.org> X-OS: Linux jeyu 4.7.9-100.fc23.x86_64 x86_64 User-Agent: NeoMutt/20161126 (1.7.1) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Wed, 04 Jan 2017 02:47:48 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.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 >Reported-by: Randy Wright >Signed-off-by: Luis R. Rodriguez Back from travel today, apologies for the delay. Will be able to give this a proper look this week. Jessica