Ok so here is the patch to do unix98 pty restore by making
invasive changes to ptmx_open() instead of using
task_struct->required_id to pass an index. Patch generated
against the cr tree with the original pty patches, as I
assume that will be easier to read.
Break ptmx_open() into a ptmx_create() which is used both
by ptmx_open itself, and fs/devpts/inode.c:open_create_pty().
The latter function is used only during sys_restart() when
a unix98 pty must be restored.
Either with or without this patch, the pty test at
git://git.sr71.net/~hallyn/cr_tests.git
under the pty/ directory. The question then is which is
a more likely approach to be acceptable upstream. The
task->required_id approach is imo 'hacky', but it actually
does a much better job of re-using existing helpers, and
therefore is more maintainable. So part of me DOES prefer
the original required_id approach.
This is purely RFC, so comments very much appreciated.
Signed-off-by: Serge Hallyn <serue@xxxxxxxxxx>
--
checkpoint/sys.c | 8 ---
drivers/char/pty.c | 16 +++---
drivers/char/tty_io.c | 112 +++++++++++++++++++++++++++++++++++----------
fs/devpts/inode.c | 55 ++++++++++++++++++++--
include/linux/checkpoint.h | 2
include/linux/devpts_fs.h | 2
include/linux/sched.h | 1
7 files changed, 151 insertions(+), 45 deletions(-)
diff --git a/checkpoint/sys.c b/checkpoint/sys.c
index 3db18f7..b1fdbd7 100644
--- a/checkpoint/sys.c
+++ b/checkpoint/sys.c
@@ -339,14 +339,6 @@ SYSCALL_DEFINE3(restart, pid_t, pid, int, fd, unsigned long, flags)
return ret;
}
-static int __init checkpoint_init(void)
-{
- init_task.required_id = CKPT_REQUIRED_NONE;
- return 0;
-}
-__initcall(checkpoint_init);
-
-
/* 'ckpt_debug_level' controls the verbosity level of c/r code */
#ifdef CONFIG_CHECKPOINT_DEBUG
diff --git a/drivers/char/pty.c b/drivers/char/pty.c
index afdab5e..79e86e4 100644
--- a/drivers/char/pty.c
+++ b/drivers/char/pty.c
@@ -28,6 +28,8 @@
#include <linux/uaccess.h>
#include <linux/bitops.h>
#include <linux/devpts_fs.h>
+#include <linux/file.h>
+#include <linux/mount.h>
#include <asm/system.h>
@@ -629,20 +631,13 @@ static const struct tty_operations pty_unix98_ops = {
* allocated_ptys_lock handles the list of free pty numbers
*/
-static int __ptmx_open(struct inode *inode, struct file *filp)
+int ptmx_create(struct inode *inode, struct file *filp, int index)
{
struct tty_struct *tty;
int retval;
- int index = -1;
nonseekable_open(inode, filp);
-#ifdef CONFIG_CHECKPOINT
- /* when restarting, request specific index */
- if (current->flags & PF_RESTARTING)
- index = (int) current->required_id;
-#endif
- /* find a device that is not in use. */
index = devpts_new_index(inode, index);
if (index < 0)
return index;
@@ -675,6 +670,11 @@ out:
return retval;
}
+static int __ptmx_open(struct inode *inode, struct file *filp)
+{
+ return ptmx_create(inode, filp, UNSPECIFIED_PTY_INDEX);
+}
+
static int ptmx_open(struct inode *inode, struct file *filp)
{
int ret;
diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index b8f8d79..d27ec36 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -79,6 +79,7 @@
#include <linux/devpts_fs.h>
#include <linux/file.h>
#include <linux/fdtable.h>
+#include <linux/namei.h>
#include <linux/console.h>
#include <linux/timer.h>
#include <linux/ctype.h>
@@ -2982,13 +2983,97 @@ static int restore_tty_ldisc(struct ckpt_ctx *ctx,
return 0;
}
-#define CKPT_PTMX_PATH "/dev/ptmx"
+#define PTMXNAME "ptmx"
+struct tty_struct *pty_open_by_master_in_ns(struct ckpt_ctx *ctx,
+ struct ckpt_hdr_tty *h, struct file *fdir)
+{
+ struct tty_struct *tty;
+ struct file *file;
+ int ret;
+ struct qstr ptmxname;
+ struct dentry *ptmxdentry;
+ struct nameidata nd;
+
+ ret = file_permission(fdir, MAY_EXEC);
+ if (ret)
+ return ERR_PTR(ret);
+ nd.path = fdir->f_path;
+ path_get(&nd.path);
+ ptmxname.name = PTMXNAME;
+ ptmxname.len = strlen(ptmxname.name);
+ mutex_lock(&fdir->f_dentry->d_inode->i_mutex);
+ ptmxdentry = d_hash_and_lookup(fdir->f_dentry, &ptmxname);
+ mutex_unlock(&fdir->f_dentry->d_inode->i_mutex);
+ if (!ptmxdentry) {
+ path_put(&nd.path);
+ return ERR_PTR(-ENOENT);
+ }
+
+ /* should get mode from the file handle... */
+ file = alloc_file(nd.path.mnt, ptmxdentry, FMODE_READ|FMODE_WRITE, &tty_fops);
+ path_put(&nd.path);
+ if (!file) {
+ dput(ptmxdentry);
+ return ERR_PTR(-EINVAL);
+ }
+
+ ret = security_dentry_open(file, current_cred());
+ if (ret) {
+ fput(file);
+ return ERR_PTR(ret);
+ }
+ /* check write access perms to file */
+
+ lock_kernel(); /* tty_open does it... */
+ ret = open_create_pty(fdir, h->index, file);
+ unlock_kernel();
+ if (ret) {
+ fput(file);
+ return ERR_PTR(ret);
+ }
+ ckpt_debug("master file %p (obj %d)\n", file, h->file_objref);
+
+ /*
+ * Add file to objhash to ensure proper cleanup later
+ * (it isn't referenced elsewhere). Use h->file_objref
+ * which was explicitly during checkpoint for this.
+ */
+ ret = ckpt_obj_insert(ctx, file, h->file_objref, CKPT_OBJ_FILE);
+ if (ret < 0) {
+ fput(file);
+ return ERR_PTR(ret);
+ }
+
+ tty = file->private_data;
+ fput(file); /* objhash took a ref */
+
+ return tty;
+}
+
+struct tty_struct *pty_open_by_master(struct ckpt_ctx *ctx,
+ struct ckpt_hdr_tty *h)
+{
+ struct file *fdir;
+ struct tty_struct *tty;
+ /*
+ * we need to pick a way to specify which devpts
+ * mountpoint to use. For now, we'll just use
+ * whatever /dev/ptmx points to
+ */
+ fdir = filp_open("/dev/pts", O_RDONLY, 0);
+ if (IS_ERR(fdir))
+ return ERR_PTR(PTR_ERR(fdir));
+
+ tty = pty_open_by_master_in_ns(ctx, h, fdir);
+ fput(fdir);
+
+ return tty;
+}
static struct tty_struct *do_restore_tty(struct ckpt_ctx *ctx)
{
struct ckpt_hdr_tty *h;
struct tty_struct *tty = ERR_PTR(-EINVAL);
- struct file *file = NULL;
int master, ret;
h = ckpt_read_obj_type(ctx, sizeof(*h), CKPT_HDR_TTY);
@@ -3025,28 +3110,9 @@ static struct tty_struct *do_restore_tty(struct ckpt_ctx *ctx)
* specific (and probably called via tty_operations).
*/
if (master) {
- current->required_id = h->index;
- file = filp_open(CKPT_PTMX_PATH, O_RDWR | O_NOCTTY, 0);
- current->required_id = CKPT_REQUIRED_NONE;
- ckpt_debug("master file %p (obj %d)\n", file, h->file_objref);
- if (IS_ERR(file)) {
- tty = (struct tty_struct *) file;
- goto out;
- }
-
- /*
- * Add file to objhash to ensure proper cleanup later
- * (it isn't referenced elsewhere). Use h->file_objref
- * which was explicitly during checkpoint for this.
- */
- ret = ckpt_obj_insert(ctx, file, h->file_objref, CKPT_OBJ_FILE);
- if (ret < 0) {
- tty = ERR_PTR(ret);
- fput(file);
+ tty = pty_open_by_master(ctx, h);
+ if (IS_ERR(tty))
goto out;
- }
-
- tty = file->private_data;
} else {
tty = ckpt_obj_fetch(ctx, h->link_objref, CKPT_OBJ_TTY);
if (IS_ERR(tty))
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index 8921726..f76083f 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -14,6 +14,9 @@
#include <linux/init.h>
#include <linux/fs.h>
#include <linux/sched.h>
+#include <linux/fs.h>
+#include <linux/smp_lock.h>
+#include <linux/file.h>
#include <linux/namei.h>
#include <linux/mount.h>
#include <linux/tty.h>
@@ -431,13 +434,18 @@ static struct file_system_type devpts_fs_type = {
/*
* The normal naming convention is simply /dev/pts/<number>; this conforms
* to the System V naming convention
+ *
+ * @ptmx_inode: inode of the /dev/ptmx file - each pty namepace has its
+ * own
+ * @req_idx: requested specific index (i.e. 5 for /dev/pts/5). If -1,
+ * then return first available.
*/
int devpts_new_index(struct inode *ptmx_inode, int req_idx)
{
struct super_block *sb = pts_sb_from_inode(ptmx_inode);
struct pts_fs_info *fsi = DEVPTS_SB(sb);
- int index;
+ int index = req_idx;
int ida_ret;
retry:
@@ -445,7 +453,8 @@ retry:
return -ENOMEM;
mutex_lock(&allocated_ptys_lock);
- index = (req_idx >= 0 ? req_idx : 0);
+ if (index == UNSPECIFIED_PTY_INDEX)
+ index = 0;
ida_ret = ida_get_new_above(&fsi->allocated_ptys, index, &index);
if (ida_ret < 0) {
mutex_unlock(&allocated_ptys_lock);
@@ -454,7 +463,7 @@ retry:
return -EIO;
}
- if (req_idx >= 0 && index != req_idx) {
+ if (req_idx != UNSPECIFIED_PTY_INDEX && index != req_idx) {
ida_remove(&fsi->allocated_ptys, index);
mutex_unlock(&allocated_ptys_lock);
return -EBUSY;
@@ -557,6 +566,46 @@ out:
mutex_unlock(&root->d_inode->i_mutex);
}
+struct inode *get_ptmx_inode(struct super_block *sb)
+{
+ struct pts_fs_info *fsi = DEVPTS_SB(sb);
+ struct dentry *dentry = fsi->ptmx_dentry;
+ struct inode *inode = dentry->d_inode;
+
+ return igrab(inode);
+}
+
+/* defined in drivers/char/pty.c */
+extern int ptmx_create(struct inode *inode, struct file *filp, int index);
+
+/*
+ * fn caller (in restart code) has checked for the rights on the
+ * part of the userspace task to open the ptmx file.
+ * @pdir: an open file handle to the /dev/pts directory, which we
+ * will use to determine the devpts namespace, and to confirm that
+ * the resulting file is in the right directory.
+ * @file is a file for /dev/ptmx
+ */
+int open_create_pty(struct file *fdir, int idx, struct file *file)
+{
+ struct inode *ptmx_ino;
+ struct dentry *pdir = fdir->f_dentry;
+ struct super_block *sb;
+ int ret;
+
+ if (!pdir || !pdir->d_inode)
+ return -EINVAL;
+
+ sb = pts_sb_from_inode(pdir->d_inode);
+ ptmx_ino = get_ptmx_inode(sb);
+ if (!ptmx_ino)
+ return -EINVAL;
+
+ ret = ptmx_create(ptmx_ino, file, idx);
+ iput(ptmx_ino);
+ return ret;
+}
+
static int __init init_devpts_fs(void)
{
int err = register_filesystem(&devpts_fs_type);
diff --git a/include/linux/checkpoint.h b/include/linux/checkpoint.h
index 5e90ef9..0cbe8c4 100644
--- a/include/linux/checkpoint.h
+++ b/include/linux/checkpoint.h
@@ -46,8 +46,6 @@
#define CHECKPOINT_USER_FLAGS CHECKPOINT_SUBTREE
#define RESTART_USER_FLAGS (RESTART_TASKSELF | RESTART_FROZEN)
-#define CKPT_REQUIRED_NONE ((unsigned long) -1L)
-
extern void exit_checkpoint(struct task_struct *tsk);
extern int ckpt_kwrite(struct ckpt_ctx *ctx, void *buf, int count);
diff --git a/include/linux/devpts_fs.h b/include/linux/devpts_fs.h
index 14948ee..a15d23c 100644
--- a/include/linux/devpts_fs.h
+++ b/include/linux/devpts_fs.h
@@ -15,10 +15,12 @@
#include <linux/errno.h>
+#define UNSPECIFIED_PTY_INDEX -1
#ifdef CONFIG_UNIX98_PTYS
int devpts_new_index(struct inode *ptmx_inode, int req_idx);
void devpts_kill_index(struct inode *ptmx_inode, int idx);
+int open_create_pty(struct file *fdir, int idx, struct file *file);
/* mknod in devpts */
int devpts_pty_new(struct inode *ptmx_inode, struct tty_struct *tty);
/* get tty structure */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f6f1350..0e67de7 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1481,7 +1481,6 @@ struct task_struct {
#endif /* CONFIG_TRACING */
#ifdef CONFIG_CHECKPOINT
struct ckpt_ctx *checkpoint_ctx;
- unsigned long required_id;
#endif
};
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
