linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Brauner <christian@brauner.io>
To: gregkh@linuxfoundation.org, tkjos@android.com,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Cc: arve@android.com, maco@android.com, joel@joelfernandes.org,
	tkjos@google.com, Christian Brauner <christian@brauner.io>
Subject: [PATCH 2/2] binderfs: make each binderfs mount a new instance
Date: Sun,  6 Jan 2019 15:05:41 +0100	[thread overview]
Message-ID: <20190106140541.16442-3-christian@brauner.io> (raw)
In-Reply-To: <20190106140541.16442-1-christian@brauner.io>

When currently mounting binderfs in the same ipc namespace twice:

mount -t binder binder /A
mount -t binder binder /B

then the binderfs instances mounted on /A and /B will be the same, i.e.
they will have the same superblock. This was the first approach that seemed
reasonable. However, this leads to some problems and inconsistencies:

/* private binderfs instance in same ipc namespace */
There is no way for a user to request a private binderfs instance in the
same ipc namespace.
This request has been made in a private mail to me by two independent
people.

/* bind-mounts */
If users want the same binderfs instance to appear in multiple places they
can use bind mounts. So there is no value in having a request for a new
binderfs mount giving them the same instance.

/* unexpected behavior */
It's surprising that request to mount binderfs is not giving the user a new
instance like tmpfs, devpts, ramfs, and others do.

/* past mistakes */
Other pseudo-filesystems once made the same mistakes of giving back the
same superblock when actually requesting a new mount (cf. devpts's
deprecated "newinstance" option).
We should not make the same mistake. Once we've committed to always giving
back the same superblock in the same IPC namespace with the next kernel
release we will not be able to make that change so better to do it now.

/* kdbusfs */
It was pointed out to me that kdbusfs - which is conceptually closely
related to binderfs - also allowed users to get a private kdbusfs instance
in the same IPC namespace by making each mount of kdbusfs a separate
instance. I think that makes a lot of sense.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 drivers/android/binderfs.c | 41 ++------------------------------------
 1 file changed, 2 insertions(+), 39 deletions(-)

diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index 6f68d6217eb3..4990d65d4850 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -379,7 +379,7 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent)
 	struct binderfs_info *info;
 	int ret = -ENOMEM;
 	struct inode *inode = NULL;
-	struct ipc_namespace *ipc_ns = sb->s_fs_info;
+	struct ipc_namespace *ipc_ns = current->nsproxy->ipc_ns;
 
 	get_ipc_ns(ipc_ns);
 
@@ -450,48 +450,11 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent)
 	return ret;
 }
 
-static int binderfs_test_super(struct super_block *sb, void *data)
-{
-	struct binderfs_info *info = sb->s_fs_info;
-
-	if (info)
-		return info->ipc_ns == data;
-
-	return 0;
-}
-
-static int binderfs_set_super(struct super_block *sb, void *data)
-{
-	sb->s_fs_info = data;
-	return set_anon_super(sb, NULL);
-}
-
 static struct dentry *binderfs_mount(struct file_system_type *fs_type,
 				     int flags, const char *dev_name,
 				     void *data)
 {
-	struct super_block *sb;
-	struct ipc_namespace *ipc_ns = current->nsproxy->ipc_ns;
-
-	if (!ns_capable(ipc_ns->user_ns, CAP_SYS_ADMIN))
-		return ERR_PTR(-EPERM);
-
-	sb = sget_userns(fs_type, binderfs_test_super, binderfs_set_super,
-			 flags, ipc_ns->user_ns, ipc_ns);
-	if (IS_ERR(sb))
-		return ERR_CAST(sb);
-
-	if (!sb->s_root) {
-		int ret = binderfs_fill_super(sb, data, flags & SB_SILENT ? 1 : 0);
-		if (ret) {
-			deactivate_locked_super(sb);
-			return ERR_PTR(ret);
-		}
-
-		sb->s_flags |= SB_ACTIVE;
-	}
-
-	return dget(sb->s_root);
+	return mount_nodev(fs_type, flags, data, binderfs_fill_super);
 }
 
 static void binderfs_kill_super(struct super_block *sb)
-- 
2.19.1


      parent reply	other threads:[~2019-01-06 14:06 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-06 14:05 [PATCH 0/2] binderfs: fixes Christian Brauner
2019-01-06 14:05 ` [PATCH 1/2] binderfs: remove wrong kern_mount() call Christian Brauner
2019-01-06 14:05 ` Christian Brauner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190106140541.16442-3-christian@brauner.io \
    --to=christian@brauner.io \
    --cc=arve@android.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=joel@joelfernandes.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maco@android.com \
    --cc=tkjos@android.com \
    --cc=tkjos@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).