From mboxrd@z Thu Jan  1 00:00:00 1970
From: ebiederm@xmission.com (Eric W. Biederman)
Subject: Re: [PATCH 03/11] vfs: Don't allow overwriting mounts in the current mount namespace
Date: Tue, 18 Feb 2014 13:28:36 -0800
Message-ID: <87ha7wrtyz.fsf@xmission.com>
References: <CAJfpegsxgnSRUW-E5HM3uT5QfGyUtn_v=i4Ppkkkutp34287AA@mail.gmail.com>
	<87a9kkax0j.fsf@xmission.com>
	<CAJfpeguv+7giYNpAuXE9Ja_9BEwB0-fZBVgRSeVqpzSXgQYZ6Q@mail.gmail.com>
	<8761v7h2pt.fsf@tw-ebiederman.twitter.com>
	<CAJfpegtT7y-1HhbEVAMKkdQugTG_w7G_epGtQHGvQLpcZB5FVA@mail.gmail.com>
	<87li281wx6.fsf_-_@xmission.com> <87ob28kqks.fsf_-_@xmission.com>
	<8761ogkqhl.fsf_-_@xmission.com>
	<20140218171252.GC4026@tucsk.piliscsaba.szeredi.hu>
Mime-Version: 1.0
Content-Type: text/plain
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	Linux-Fsdevel <linux-fsdevel@vger.kernel.org>,
	Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andy Lutomirski <luto@amacapital.net>,
	Rob Landley <rob@landley.net>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Christoph Hellwig <hch@infradead.org>,
	Karel Zak <kzak@redhat.com>,
	"J. Bruce Fields" <bfields@fieldses.org>
To: Miklos Szeredi <miklos@szeredi.hu>
Return-path: <linux-kernel-owner@vger.kernel.org>
In-Reply-To: <20140218171252.GC4026@tucsk.piliscsaba.szeredi.hu> (Miklos
	Szeredi's message of "Tue, 18 Feb 2014 18:12:52 +0100")
Sender: linux-kernel-owner@vger.kernel.org
List-Id: linux-fsdevel.vger.kernel.org


>> diff --git a/fs/namespace.c b/fs/namespace.c
>> index 22e536705c45..6dc23614d44d 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -631,6 +631,41 @@ struct vfsmount *lookup_mnt(struct path *path)
>>  	return m;
>>  }
>>  
>> +/*
>> + * __is_local_mountpoint - Test to see if dentry is a mountpoint in the
>> + *                         current mount namespace.
>> + *
>> + * The common case is dentries are not mountpoints at all and that
>> + * test is handled inline.  For the slow case when we are actually
>> + * dealing with a mountpoint of some kind, walk through all of the
>> + * mounts in the current mount namespace and test to see if the dentry
>> + * is a mountpoint.
>> + *
>> + * The mount_hashtable is not usable in the context because we
>> + * need to identify all mounts that may be in the current mount
>> + * namespace not just a mount that happens to have some specified
>> + * parent mount.
>> + */
>> +int __is_local_mountpoint(struct dentry *dentry)
>
> Minor nit: return value of any is_* function is either true or false, so why not
> declare it bool?

Because I am working on the core of the kernel and C compilers do weird
things with bool variables (storing them in bytes...).  I expected a
type that the C compiler does not do weird things with would be more
readily received on a path whose performance people are interested in.

>> +{
>> +	struct mnt_namespace *ns = current->nsproxy->mnt_ns;
>> +	struct mount *mnt;
>> +	int is_covered = 0;
>
> And the same for this var.
>
>> +
>> +	if (!d_mountpoint(dentry))
>> +		goto out;
>> +
>> +	down_read(&namespace_sem);
>> +	list_for_each_entry(mnt, &ns->list, mnt_list) {
>> +		is_covered = (mnt->mnt_mountpoint == dentry);
>> +		if (is_covered)
>> +			break;
>> +	}
>> +	up_read(&namespace_sem);
>> +out:
>> +	return is_covered;
>> +}
>> +
>>  static struct mountpoint *new_mountpoint(struct dentry *dentry)
>>  {
>>  	struct list_head *chain = mountpoint_hashtable + hash(NULL, dentry);
>> -- 
>> 1.7.5.4

Eric