From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from h2.hallyn.com ([78.46.35.8]:53814 "EHLO h2.hallyn.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751063AbcFXG4d (ORCPT <rfc822;linux-fsdevel@vger.kernel.org>);
	Fri, 24 Jun 2016 02:56:33 -0400
Date: Fri, 24 Jun 2016 01:56:31 -0500
From: "Serge E. Hallyn" <serge@hallyn.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Andy Lutomirski <luto@amacapital.net>,
	Linux Containers <containers@lists.linux-foundation.org>,
	Linux FS Devel <linux-fsdevel@vger.kernel.org>,
	Miklos Szeredi <miklos@szeredi.hu>,
	James Bottomley <James.Bottomley@hansenpartnership.com>,
	Djalal Harouni <tixxdz@gmail.com>,
	Seth Forshee <seth.forshee@canonical.com>,
	"Serge E. Hallyn" <serge@hallyn.com>
Subject: Re: [PATCH review 02/13] mnt: Refactor fs_fully_visible into
 mount_too_revealing
Message-ID: <20160624065631.GA403@mail.hallyn.com>
References: <87fus77pns.fsf@x220.int.ebiederm.org>
 <20160620172130.15712-1-ebiederm@xmission.com>
 <20160620172130.15712-2-ebiederm@xmission.com>
 <CALCETrXv2aeP38AmUaRVMC+O-oeWKwDcy8fPfsOCu1f8mncZEA@mail.gmail.com>
 <874m8m4bky.fsf@x220.int.ebiederm.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <874m8m4bky.fsf@x220.int.ebiederm.org>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Tue, Jun 21, 2016 at 01:54:21PM -0500, Eric W. Biederman wrote:
> Andy Lutomirski <luto@amacapital.net> writes:
> 
> > On Mon, Jun 20, 2016 at 10:21 AM, Eric W. Biederman
> > <ebiederm@xmission.com> wrote:
> >> Replace the call of fs_fully_visible in do_new_mount from before the
> >> new superblock is allocated with a call of mount_too_revealing after
> >> the superblock is allocated.   This winds up being a much better location
> >> for maintainability of the code.
> >>
> >> The first change this enables is the replacement of FS_USERNS_VISIBLE
> >> with SB_I_USERNS_VISIBLE.  Moving the flag from struct filesystem_type
> >> to sb_iflags on the superblock.
> >
> > Why is this useful?
> 
> A couple of reasons.
> - It helps clean up do_new_mount which is currently so overloaded by
>   special cases that it is difficult to see the core logic.

Agreed, i find this easier to read and reason about.

> - It makes the check about the actual superblock that is being mounted
>   rather than the superblock that might be mounted.
> 
> - The practical place where being about the actual superblock that is
>   being mounted helps is that in "11/13 mnt: Simplify mount_too_revealing"
>   that removes the MNT_LOCK_NOSUID MNT_LOCK_NOEXEC and MNT_LOCK_NODEV
>   tests from the code, while verify that those tests are not needed
>   because the sb_iflags contains SB_I_NOEXEC and SB_I_NODEV.
> 
> - The conceptual change of testing once the superblock has been
>   generated makes changes like the one above much more sensible
>   and it helps untangle mount namespace versus superblock concerns.
> 
> That last is a big part of what this patchset is about.  When do we care
> about the superblock and when do we care about the mount namespace.
> 
> Eric