cr: pts: detect use of multiple devpts mounts in container
RFC PATCH 1/1 cr: pts: detect use of multiple devpts mounts in container
We don't support multiple devpts mounts in a container. So bail on checkpoint if a container has them. This will cause checkpoint to fail of a container which still has a pty from parent container in use.
Note one issue with this is the register_checkpoint_obj(&&ckpt_obj_ptsns_ops) done from module_init(). We either need to add a matching unregister, or maybe move this into drivers/char/tty_io.c.
--- drivers/char/tty_io.c | 27 +++++++++++++++++++++++++++ fs/devpts/inode.c | 9 ++++++++- include/linux/checkpoint_hdr.h | 2 ++ include/linux/checkpoint_types.h | 2 ++ include/linux/devpts_fs.h | 22 ++++++++++++++++++++-- 5 files changed, 59 insertions(+), 3 deletions(-)
drivers/char/tty_io.c +27 -0
@@ -96,6 +96,7 @@#include <linux/bitops.h> #include <linux/delay.h> #include <linux/seq_file.h> +#include <linux/mount.h> #include <linux/uaccess.h> #include <asm/system.h>
@@ -2687,6 +2688,27 @@ static int tty_can_checkpoint(struct ckpt_ctx *ctx, struct tty_struct *tty)return 1 ; } +/* +* If tty_file_checkpoint() ever supports more than unix98 ptys we'll have +* to check for that +*/ +static int detect_multiple_ptsns (struct ckpt_ctx *ctx,struct file *file) +{ +int objref; +int new; + + objref =ckpt_obj_lookup_add (ctx, file->f_path.mnt->mnt_sb, +CKPT_OBJ_PTS_NS , &new); +if (objref <0 ) +return objref; +if (new && ctx->num_devpts_ns) +return -EBUSY ; +if (file->f_path.mnt->mnt_ns != ctx->root_nsproxy->mnt_ns) +return -EBUSY ; + ctx->num_devpts_ns++; +return 0 ; +} +static int tty_file_checkpoint (struct ckpt_ctx *ctx,struct file *file) {struct ckpt_hdr_file_tty *h;
@@ -2732,6 +2754,11 @@ static int tty_file_checkpoint(struct ckpt_ctx *ctx, struct file *file) ret =ckpt_write_obj (ctx, &h->common.h);ckpt_hdr_put (ctx, h); + + ret =detect_multiple_ptsns (ctx, file); +if (ret) +ckpt_err (ctx, ret,"Open pty from alien ptsns\n" ); +return ret; }
fs/devpts/inode.c +8 -1
@@ -572,7 +572,14 @@ out:static int __init init_devpts_fs (void ) { -int err =register_filesystem (&devpts_fs_type); +int err; +#ifdef CONFIG_CHECKPOINT + err =register_checkpoint_obj (&ckpt_obj_ptsns_ops); +if (err) +return err; +#endif + + err =register_filesystem (&devpts_fs_type);if (!err) { devpts_mnt =kern_mount (&devpts_fs_type);if (IS_ERR (devpts_mnt)) {
include/linux/checkpoint_hdr.h +2 -0
@@ -271,6 +271,8 @@ enum obj_type {#define CKPT_OBJ_NET_NS CKPT_OBJ_NET_NS CKPT_OBJ_NETDEV,#define CKPT_OBJ_NETDEV CKPT_OBJ_NETDEV + CKPT_OBJ_PTS_NS, +#define CKPT_OBJ_PTS_NS CKPT_OBJ_PTS_NS CKPT_OBJ_MAX#define CKPT_OBJ_MAX CKPT_OBJ_MAX };
include/linux/checkpoint_types.h +2 -0
@@ -86,6 +86,8 @@ struct ckpt_ctx {struct cred *realcred, *ecred;/* tmp storage for cred at restart */ struct list_head listen_sockets;/* listening parent sockets */ +int num_devpts_ns;/* must not become > 1 at the moment */ +struct ckpt_stats stats;/* statistics */ #define CKPT_MSG_LEN 1024
include/linux/devpts_fs.h +20 -2
@@ -14,6 +14,7 @@#define _LINUX_DEVPTS_FS_H#include <linux/errno.h> +#include <linux/checkpoint.h> #define UNSPECIFIED_PTY_INDEX -1
@@ -30,7 +31,24 @@ struct tty_struct *devpts_get_tty(struct inode *pts_inode, int number);/* unlink */ void devpts_pty_kill (struct tty_struct *tty); -#else +#ifdef CONFIG_CHECKPOINT +/* +* We don't yet support multiple pts mounts in a container, so +* all we're doing here is detecting pty's in >1 ptsns. We'll +* actually stick the sb on the objhash because that's all there +* is. Note that means that when we start checkpointing mounts +* and filesystems we'll need to change this. +* +* We don't need to pin these bc they are pinned by the pty, +* which we do have pinned. +*/ +static const struct ckpt_obj_ops ckpt_obj_ptsns_ops = { + .obj_name ="PTS NS" , + .obj_type =CKPT_OBJ_PTS_NS , +}; +#endif /* CONFIG_CHECKPOINT */ + +#else /* CONFIG_UNIX98_PTYS */ /* Dummy stubs in the no-pty case */ static inline int devpts_new_index (struct inode *ptmx_inode) {return -EINVAL ; }
@@ -47,7 +65,7 @@ static inline struct tty_struct *devpts_get_tty(struct inode *pts_inode, }static inline void devpts_pty_kill (struct tty_struct *tty) { } -#endif +#endif /* CONFIG_UNIX98_PTYS */ #endif /* _LINUX_DEVPTS_FS_H */
Signature · 1 line
1.7.0.4
A message that is not in the archive (sent off-list or not yet received): 20100429152848.GA7090-r/[email protected]
Quoted 7 lines · Serge E. Hallyn
We don't support multiple devpts mounts in a container. So bail on checkpoint if a container has them. This will cause checkpoint to fail of a container which still has a pty from parent container in use.
Note one issue with this is the
register_checkpoint_obj(&&ckpt_obj_ptsns_ops)done frommodule_init(). We either need to add a matching unregister,
That's a rather significant problem. We need a matching unregister_checkpoint_obj() and we need to be sure that none of the functions specified in the ckpt_obj_ops are executing when the module is finally removed.
Is there a way to do that without holding any locks during ckpt_obj_op calls? I've been thinking about how to do it with RCU:
struct ckpt_obj_ops *ops; rcu_read_lock(); ops = ckpt_obj_ops[type]; get_module(ops->owner); /* try_module_get() */ rcu_read_unlock(); ops->op(...); put_module(ops->owner);
That should also ensure that the module won't be removed until the operation is complete. However it seems quite absurd to grab a reference to a module just to use its grab and drop ops. We'll probably spend more time in the module code...
Regardless, the register side requires a spinlock to protect writes to the ckpt_obj_ops array (inside register_checkpoint_obj()).
Cheers,
-Matt