From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933758AbcJERis (ORCPT ); Wed, 5 Oct 2016 13:38:48 -0400 Received: from mx2.suse.de ([195.135.220.15]:34758 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933677AbcJERik (ORCPT ); Wed, 5 Oct 2016 13:38:40 -0400 Date: Wed, 5 Oct 2016 19:38:35 +0200 From: "Luis R. Rodriguez" To: Linus Torvalds Cc: "Luis R. Rodriguez" , Dmitry Torokhov , "Herbert, Marc" , "open list:DOCUMENTATION" , Jacek Anaszewski , David Woodhouse , Christian Lamparter , Julia Lawall , Andrew Morton , linuxppc-dev , Mimi Zohar , Andy Lutomirski , Richard Purdie , Wu Fengguang , Johannes Berg , Michal Marek , Hauke Mehrtens , Mark Brown , Jiri Slaby , Ming Lei , Daniel Vetter , Bjorn Andersson , Felix Fietkau , Roman Pen , Greg KH , Linux Kernel Mailing List , Vikram Mulukutla , Stephen Boyd , Takashi Iwai , Jeff Mahoney , Hariprasad S , Benjamin Poirier , Josh Triplett , Kees Cook Subject: Re: [RFC] fs: add userspace critical mounts event support Message-ID: <20161005173835.GC3296@wotan.suse.de> References: <20160903174939.GB32345@dtor-ws> <2deae6da-dd43-7bff-e1fd-ffd26946b928@intel.com> <20161005000008.GY3296@wotan.suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 04, 2016 at 05:32:22PM -0700, Linus Torvalds wrote: > On Tue, Oct 4, 2016 at 5:24 PM, Luis R. Rodriguez wrote: > > > > Note that the races are beyond firmware, so all > > kernel_read_file_from_path() users, as such re-using such old /sys/ > > interafeces for firmware will not suffice to cover all ground now for > > the same race for other possible users. > > Blah blah blah. > > The reason I've hated this whole discussion is that it's full of > "let's re-architect everything", and then it has these horribly warty > interfaces. To be clear, kernel_read_file_from_path() was an agreed upon strategy about 1 year ago at the Linux Security summit as we found different kernel implementations for the same exact task, reading files from the filesystem -- my point here was simply that acknowledging that the race on early init and driver's init / probe for firmware is implicating that the race is *also* possible for the other kernel-read-from-fs points. Its not clear to me what your grudge here is other than the proposal for a solution in this patch is not what we want. > It's classic second-system syndrome. > > Just do *one* thing, and do it well. Don't change anything else. Don't > force existign drivers to use new interfaces. Don't over-architect, > and don't do stupid interfaces. If there is a race for the other users and we want to avoid wrapping a solution for it to the other callers without doing any vetting for correctness then so be it, but to disregard completely seems error-prone. I accept that thinking about such other users may complicate a solution for firmware and if you prefer we just separate the race solution for both that's fine. > If user-space mounts a new filesystem (or just unpacks files from a > tar-file that has firmware images in it, for chissake), that is not > some magical "critical mount event". The whole concept is just stupid. > Is it a "mount event" when the user downloads a new firmware image > from the internet? > > HELL NO. We've gotten passed that the original implementation proposed is not what we want, let's move on. > But what is equally stupid is to then dismiss simple models because > some totally unrelated "beyond firmware" issue. I have not heard back from the other stakeholders using kernel_read_file_from_path() and possible races for them. You seem to suggest to ignore those possible theoretical races in the name of a simple solution for firmware. Fine. > Anything that is "beyond firmware" shouldn't even be discussed, for > chrissake! It has nothing what-so-ever to do with firmware loading. If > there ends up being some common helper functions, and shared code, > that *still* doesn't make it so. My point was to raise the flag of the possible races on the other call sites where we read files directly from the kernel, that's all, if we agree we really don't care for that fine. > Basic rules of thumb: > > (a) don't over-design > > (b) don't have stupid illogical interfaces > > (c) don't conflate different issues just because you think they may > have shared code. > > (4) be consistent. Don't make up new interfaces, and most certainly > do *NOT* dismiss something just because it's what we have done before. > > That's it. OK.. Luis