All of lore.kernel.org
 help / color / mirror / Atom feed
From: sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org
To: hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org,
	alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org
Cc: kyle-hoO6YkzgTuCM0SS3m2neIg@public.gmane.org,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org,
	bastian-yyjItF7Rl6lg9hUCZPvPmw@public.gmane.org,
	containers-qjLDD68F18O7TbgM5vRIOg@public.gmane.org,
	sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org,
	xemul-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org
Subject: [PATCH 09/10] Enable multiple instances of devpts
Date: Fri, 12 Sep 2008 10:53:22 -0700	[thread overview]
Message-ID: <20080912175322.GJ17350@us.ibm.com> (raw)
In-Reply-To: <20080912174845.GA17350-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>


From 4567a37856205a04cc0617e3fcc8ede36b25bcf5 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Date: Tue, 9 Sep 2008 18:52:56 -0700
Subject: [PATCH 09/10] Enable multiple instances of devpts

To support containers, allow multiple instances of devpts filesystem, such
that indices of ptys allocated in one instance are independent of ptys
allocated in other instances of devpts.

But to preserve backward compatibility, enable this support for multiple
instances only if:

	- CONFIG_DEVPTS_MULTIPLE_INSTANCES is set to Y, and
	- '-o newinstance' mount option is specified while mounting devpts

See Documentation/fs/devpts.txt (next patch in series) for details.

To use multi-instance mount, a container startup script could:

	$ ns_exec -cm /bin/bash
	$ umount /dev/pts
	$ mount -t devpts -o newinstance lxcpts /dev/pts
	$ mount -o bind /dev/pts/ptmx /dev/ptmx
	$ sshd -p 1234

where 'ns_exec -cm /bin/bash' is calls clone() with CLONE_NEWNS flag and execs
/bin/bash in the child process. A pty created by the sshd is not visible in
the original mount of /dev/pts.

USER-SPACE-IMPACT:

	See Documentation/fs/devpts.txt (included in next patch) for user-space
	impact in multi-instance and mixed-mode operation.
TODO:
	- Update mount(8), pts(4) man pages. Highlight impact of not
	  redirecting /dev/ptmx to /dev/pts/ptmx after a multi-instance mount.

Implementation note:

	See comments in new get_sb_ref() function in fs/super.c on why
	get_sb_single() cannot be directly used.

Changelog[v5]:
	- Move get_sb_ref() definition to earlier patch

	- Move usage info to Documentation/filesystems/devpts.txt (next patch)

	- Make ptmx node even in init_pts_ns, now that default mode is 0000
	  (defined in earlier patch, enabled here).

	- Cache ptmx dentry and use to update mode during remount
	  (defined in earlier patch, enabled here).

	- Bugfix: explicitly ignore newinstance on remount (if newinstance was
	  specified on remount of initial mount, it would be ignored but
	  /proc/mounts would imply that the option was set)

Changelog[v4]:

	- Update patch description to address H. Peter Anvin's comments

	- Consolidate multi-instance mode code under new config token,
	  CONFIG_DEVPTS_MULTIPLE_INSTANCE.

	- Move usage-details from patch description to
	  Documentation/fs/devpts.txt

Changelog[v3]:
	- Rename new mount option to 'newinstance'

	- Create ptmx nodes only in 'newinstance' mounts

	- Bugfix: parse_mount_options() modifies @data but since we need to
	  parse the @data twice (once in devpts_get_sb() and once during
	  do_remount_sb()), parse a local copy of @data in devpts_get_sb().
	  (restructured code in devpts_get_sb() to fix this)

Changelog[v2]:
	- Support both single-mount and multiple-mount semantics and
	  provide '-onewmnt' option to select the semantics.

Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 fs/devpts/inode.c |  168 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 163 insertions(+), 5 deletions(-)

diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index 6b56255..c54b010 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -48,10 +48,11 @@ struct pts_mount_opts {
 	gid_t   gid;
 	umode_t mode;
 	umode_t ptmxmode;
+	int newinstance;
 };
 
 enum {
-	Opt_uid, Opt_gid, Opt_mode, Opt_ptmxmode,
+	Opt_uid, Opt_gid, Opt_mode, Opt_ptmxmode, Opt_newinstance,
 	Opt_err
 };
 
@@ -61,6 +62,7 @@ static match_table_t tokens = {
 	{Opt_mode, "mode=%o"},
 #ifdef CONFIG_DEVPTS_MULTIPLE_INSTANCES
 	{Opt_ptmxmode, "ptmxmode=%o"},
+	{Opt_newinstance, "newinstance"},
 #endif
 	{Opt_err, NULL}
 };
@@ -78,13 +80,15 @@ static inline struct pts_fs_info *DEVPTS_SB(struct super_block *sb)
 
 static inline struct super_block *pts_sb_from_inode(struct inode *inode)
 {
+#ifdef CONFIG_DEVPTS_MULTIPLE_INSTANCES
 	if (inode->i_sb->s_magic == DEVPTS_SUPER_MAGIC)
 		return inode->i_sb;
-
+#endif
 	return devpts_mnt->mnt_sb;
 }
 
-static int parse_mount_options(char *data, struct pts_mount_opts *opts)
+static int parse_mount_options(char *data, int remount,
+		struct pts_mount_opts *opts)
 {
 	char *p;
 
@@ -95,6 +99,10 @@ static int parse_mount_options(char *data, struct pts_mount_opts *opts)
 	opts->mode    = DEVPTS_DEFAULT_MODE;
 	opts->ptmxmode = DEVPTS_DEFAULT_PTMX_MODE;
 
+	/* ignore newinstance on remount to avoid confusing show_options */
+	if (!remount)
+		opts->newinstance = 0;
+
 	while ((p = strsep(&data, ",")) != NULL) {
 		substring_t args[MAX_OPT_ARGS];
 		int token;
@@ -128,6 +136,10 @@ static int parse_mount_options(char *data, struct pts_mount_opts *opts)
 				return -EINVAL;
 			opts->ptmxmode = option & S_IALLUGO;
 			break;
+		case Opt_newinstance:
+			if (!remount)
+				opts->newinstance = 1;
+			break;
 #endif
 		default:
 			printk(KERN_ERR "devpts: called with bogus options\n");
@@ -180,6 +192,8 @@ static int mknod_ptmx(struct super_block *sb)
 
 	d_add(dentry, inode);
 
+	fsi->ptmx_dentry = dentry;
+
 	printk(KERN_DEBUG "Created ptmx node in devpts ino %lu\n",
 			inode->i_ino);
 
@@ -207,7 +221,7 @@ static int devpts_remount(struct super_block *sb, int *flags, char *data)
 	struct pts_fs_info *fsi = DEVPTS_SB(sb);
 	struct pts_mount_opts *opts = &fsi->mount_opts;
 
-	err = parse_mount_options(data, opts);
+	err = parse_mount_options(data, 1, opts);
 
 	/*
 	 * parse_mount_options() restores options to default values
@@ -232,6 +246,8 @@ static int devpts_show_options(struct seq_file *seq, struct vfsmount *vfs)
 	seq_printf(seq, ",mode=%03o", opts->mode);
 #ifdef CONFIG_DEVPTS_MULTIPLE_INSTANCES
 	seq_printf(seq, ",ptmxmode=%03o", opts->ptmxmode);
+	if (opts->newinstance)
+		seq_printf(seq, ",newinstance");
 #endif
 
 	return 0;
@@ -298,12 +314,153 @@ fail:
 	return -ENOMEM;
 }
 
+#ifdef CONFIG_DEVPTS_MULTIPLE_INSTANCES
+/*
+ * Safely parse the mount options in @data and update @opts.
+ *
+ * devpts ends up parsing options two times during mount, due to the
+ * two modes of operation it supports. The first parse occurs in
+ * devpts_get_sb() when determining the mode (single-instance or
+ * multi-instance mode). The second parse happens in devpts_remount()
+ * or new_pts_mount() depending on the mode.
+ *
+ * Parsing of options modifies the @data making subsequent parsing
+ * incorrect. So make a local copy of @data and parse it.
+ *
+ * Return: 0 On success, -errno on error
+ */
+static int safe_parse_mount_options(void *data, struct pts_mount_opts *opts)
+{
+	int rc;
+	void *datacp;
+
+	if (!data)
+		return 0;
+
+	/* Use kstrdup() ?  */
+	datacp = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!datacp)
+		return -ENOMEM;
+
+	memcpy(datacp, data, PAGE_SIZE);
+	rc = parse_mount_options((char *)datacp, 0, opts);
+	kfree(datacp);
+
+	return rc;
+}
+
+/*
+ * Mount a new (private) instance of devpts.  PTYs created in this
+ * instance are independent of the PTYs in other devpts instances.
+ */
+static int new_pts_mount(struct file_system_type *fs_type, int flags,
+		void *data, struct vfsmount *mnt)
+{
+	int err;
+	struct pts_fs_info *fsi;
+	struct pts_mount_opts *opts;
+
+	printk(KERN_NOTICE "devpts: newinstance mount\n");
+
+	err = get_sb_nodev(fs_type, flags, data, devpts_fill_super, mnt);
+	if (err)
+		return err;
+
+	fsi = DEVPTS_SB(mnt->mnt_sb);
+	opts = &fsi->mount_opts;
+
+	err = parse_mount_options(data, 0, opts);
+	if (err)
+		goto fail;
+
+	err = mknod_ptmx(mnt->mnt_sb);
+	if (err)
+		goto fail;
+
+	return 0;
+
+fail:
+	dput(mnt->mnt_sb->s_root);
+	deactivate_super(mnt->mnt_sb);
+	return err;
+}
+
+/*
+ * Check if 'newinstance' mount option was specified in @data.
+ *
+ * Return: -errno  	on error (eg: invalid mount options specified)
+ * 	 : 1 		if 'newinstance' mount option was specified
+ * 	 : 0 		if 'newinstance' mount option was NOT specified
+ */
+static int is_new_instance_mount(void *data)
+{
+	int rc;
+	struct pts_mount_opts opts;
+
+	if (!data)
+		return 0;
+
+	rc = safe_parse_mount_options(data, &opts);
+	if (!rc)
+		rc = opts.newinstance;
+
+	return rc;
+}
+
+/*
+ * Mount or remount the initial kernel mount of devpts. This type of
+ * mount maintains the legacy, single-instance semantics, while the
+ * kernel still allows multiple-instances.
+ */
+static int init_pts_mount(struct file_system_type *fs_type, int flags,
+		void *data, struct vfsmount *mnt)
+{
+	int err;
+
+	if (!devpts_mnt) {
+		err = get_sb_single(fs_type, flags, data, devpts_fill_super,
+				mnt);
+
+		err = mknod_ptmx(mnt->mnt_sb);
+		if (err) {
+			dput(mnt->mnt_sb->s_root);
+			deactivate_super(mnt->mnt_sb);
+		} else
+			devpts_mnt = mnt;
+
+		return err;
+	}
+
+	return get_sb_ref(devpts_mnt->mnt_sb, flags, data, mnt);
+}
+
 static int devpts_get_sb(struct file_system_type *fs_type,
 	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
 {
+	int new;
+
+	new = is_new_instance_mount(data);
+	if (new < 0)
+		return new;
+
+	if (new)
+		return new_pts_mount(fs_type, flags, data, mnt);
+
+	return init_pts_mount(fs_type, flags, data, mnt);
+}
+#else
+/*
+ * This supports only the legacy single-instance semantics (no
+ * multiple-instance semantics)
+ */
+static int devpts_get_sb(struct file_system_type *fs_type, int flags,
+		const char *dev_name, void *data, struct vfsmount *mnt)
+{
 	return get_sb_single(fs_type, flags, data, devpts_fill_super, mnt);
 }
 
+#endif
+
 static void devpts_kill_sb(struct super_block *sb)
 {
 	struct pts_fs_info *fsi = DEVPTS_SB(sb);
@@ -431,8 +588,9 @@ void devpts_pty_kill(struct tty_struct *tty)
 	if (dentry && !IS_ERR(dentry)) {
 		inode->i_nlink--;
 		d_delete(dentry);
-		dput(dentry);
+		dput(dentry);		// d_lookup in devpts_pty_new
 	}
+	dput(dentry);			// d_find_alias above
 
 	mutex_unlock(&root->d_inode->i_mutex);
 }
-- 
1.5.2.5

  parent reply	other threads:[~2008-09-12 17:53 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-12 17:48 [PATCH 0/10][v4]: Enable multiple devpts instances sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found] ` <20080912174845.GA17350-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-12 17:50   ` [PATCH 01/10] Remove devpts_root global sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found]     ` <20080912175057.GB17350-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-24 17:04       ` Serge E. Hallyn
2008-09-12 17:51   ` [PATCH 02/10] Per-mount allocated_ptys sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found]     ` <20080912175116.GC17350-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-24 17:14       ` Serge E. Hallyn
     [not found]         ` <20080924171408.GB25255-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-27  1:12           ` sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
2008-09-12 17:51   ` [PATCH 03/10] Per-mount 'config' object sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found]     ` <20080912175135.GD17350-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-24 17:20       ` Serge E. Hallyn
2008-09-12 17:51   ` [PATCH 04/10] Extract option parsing to new function sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found]     ` <20080912175153.GE17350-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-24 17:23       ` Serge E. Hallyn
2008-09-12 17:52   ` [PATCH 05/10] Add DEVPTS_MULTIPLE_INSTANCES config token sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found]     ` <20080912175210.GF17350-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-24 17:24       ` Serge E. Hallyn
2008-09-12 17:52   ` [PATCH 06/10] Define mknod_ptmx() sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found]     ` <20080912175237.GG17350-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-24 18:21       ` Serge E. Hallyn
     [not found]         ` <20080924182125.GF25255-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-26 21:32           ` sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
2008-09-24 18:50       ` Serge E. Hallyn
     [not found]         ` <20080924185046.GA31535-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-26 21:29           ` sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
     [not found]             ` <20080926212954.GE31505-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-29 13:14               ` Serge E. Hallyn
2008-09-12 17:52   ` [PATCH 07/10] Update ptmx permissions during remount sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found]     ` <20080912175252.GH17350-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-24 18:30       ` Serge E. Hallyn
2008-09-12 17:53   ` [PATCH 08/10] Define get_sb_ref() sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found]     ` <20080912175308.GI17350-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-24 19:20       ` Serge E. Hallyn
2008-09-24 19:55       ` Dave Hansen
2008-09-26 21:21         ` sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
     [not found]           ` <20080926212115.GD31505-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-26 21:31             ` Dave Hansen
2008-09-27  0:47               ` sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
     [not found]                 ` <20080927004727.GA2161-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-29 14:00                   ` Cedric Le Goater
     [not found]                     ` <48E0DF71.2070007-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org>
2008-09-30 15:13                       ` Serge E. Hallyn
     [not found]                         ` <20080930151325.GA26713-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-10-01 12:38                           ` Cedric Le Goater
2008-09-27 20:29               ` sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
     [not found]                 ` <20080927202924.GA16208-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-10-04  3:09                   ` sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
2008-09-12 17:53   ` sukadev-r/Jw6+rmf7HQT0dZR+AlfA [this message]
     [not found]     ` <20080912175322.GJ17350-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-24 20:26       ` [PATCH 09/10] Enable multiple instances of devpts Serge E. Hallyn
     [not found]         ` <20080924202616.GB31664-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-26 21:03           ` sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
     [not found]             ` <20080926210347.GB31505-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-29 13:01               ` Serge E. Hallyn
     [not found]                 ` <20080929130131.GA12531-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-29 15:18                   ` sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
     [not found]                     ` <20080929151828.GA10202-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-29 15:29                       ` Serge E. Hallyn
     [not found]                         ` <20080929152951.GA32518-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-29 15:58                           ` Cedric Le Goater
2008-09-29 14:06               ` Cedric Le Goater
2008-09-12 17:53   ` [PATCH 10/10] Document usage of multiple-instances " sukadev-r/Jw6+rmf7HQT0dZR+AlfA
     [not found]     ` <20080912175347.GK17350-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-19 15:33       ` Alan Cox
     [not found]         ` <20080919163311.626b715f-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
2008-09-19 16:53           ` H. Peter Anvin
2008-09-20 16:17           ` sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
     [not found]             ` <20080920161717.GA23693-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-22 13:29               ` Serge E. Hallyn
     [not found]                 ` <20080922132937.GA11932-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-22 19:25                   ` Serge E. Hallyn
2008-09-22 16:16               ` Serge E. Hallyn
     [not found]                 ` <20080922161658.GA27087-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-22 16:33                   ` sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
2008-09-24 20:36       ` Serge E. Hallyn
     [not found]         ` <20080924203650.GC31664-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
2008-09-26 21:05           ` sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8

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=20080912175322.GJ17350@us.ibm.com \
    --to=sukadev-r/jw6+rmf7hqt0dzr+alfa@public.gmane.org \
    --cc=alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org \
    --cc=bastian-yyjItF7Rl6lg9hUCZPvPmw@public.gmane.org \
    --cc=containers-qjLDD68F18O7TbgM5vRIOg@public.gmane.org \
    --cc=ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org \
    --cc=hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org \
    --cc=kyle-hoO6YkzgTuCM0SS3m2neIg@public.gmane.org \
    --cc=sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org \
    --cc=xemul-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.