[user-cr][PATCH 1/2] restart: remount /proc for new tasks created with CLONE_NEWPID

[user-cr][PATCH 1/2] restart: remount /proc for new tasks created with CLONE_NEWPID

[Oren Laadan]

Not doing this can be a pain for restarted software which relies
on /proc...

This builds on a patch by Serge Hallyn, but also aims to address the
future cases of hierarchical pid-ns:

1) Before mounting the new /proc, first umount the old one, which
isn't necessary anymore.

2) Perform the unshare() together with the remount of /proc, so it
will occur for every new pid-ns and not only for the first one.

Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
 restart.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/restart.c b/restart.c
index f3d33de..f42b456 100644
--- a/restart.c
+++ b/restart.c
@@ -30,6 +30,7 @@
 #include <asm/unistd.h>
 #include <sys/syscall.h>
 #include <sys/prctl.h>
+#include <sys/mount.h>
 
 #include <linux/sched.h>
 #include <linux/checkpoint.h>
@@ -273,6 +274,8 @@ int global_child_collected;
 int global_send_sigint = -1;
 int global_sent_sigint;
 
+static int ckpt_remount_proc(void);
+
 static int ckpt_build_tree(struct ckpt_ctx *ctx);
 static int ckpt_init_tree(struct ckpt_ctx *ctx);
 static int ckpt_set_creator(struct ckpt_ctx *ctx, struct task *task);
@@ -981,11 +984,36 @@ static int ckpt_probe_child(pid_t pid, char *str)
 	return 0;
 }
 
+/*
+ * Remount the /proc with a new instance: tasks that start a new
+ * pid-ns need a fresh mount of /proc to reflect their pid-ns.
+ */
+static int ckpt_remount_proc(void)
+{
+	if (unshare(CLONE_NEWNS | CLONE_FS) < 0) {
+		perror("unshare");
+		return -1;
+	}
+	if (umount2("/proc", MNT_DETACH) < 0) {
+		perror("umount -l /proc");
+		return -1;
+	}
+	if (mount("proc", "/proc", "proc", 0, NULL) < 0) {
+		perror("mount -t proc");
+		return -1;
+	}
+
+	return 0;
+}
+
 #ifdef CLONE_NEWPID
 static int __ckpt_coordinator(void *arg)
 {
 	struct ckpt_ctx *ctx = (struct ckpt_ctx *) arg;
 
+	if (ckpt_remount_proc() < 0)
+		return -1;
+
 	if (!ctx->args->wait)
 		close(ctx->pipe_coord[0]);
 
@@ -1850,6 +1878,10 @@ int ckpt_fork_stub(void *data)
 	struct task *task = (struct task *) data;
 	struct ckpt_ctx *ctx = task->ctx;
 
+	/* tasks with new pid-ns need new /proc mount */
+	if ((task->flags & TASK_NEWPID) && ckpt_remount_proc() < 0)
+		return -1;
+
 	/*
 	 * In restart into a new pid namespace (--pidns), coordinator
 	 * is the container init, hence if it terminated permatutely
-- 
1.6.3.3

[user-cr][PATCH 2/2] Add --mount-pty option to mount new devpts

[Oren Laadan]

Based on Serge Hallyn's patch:

  "Trivial patch, and I'm not sure whether we want this or want to do
  it this way.  But it saves me having to do it during my restart.sh
  wrapper shell-script."

Make sure that the old /dev/pts is un-mounted prior to the mount of
the new one.

Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
 restart.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/restart.c b/restart.c
index f42b456..e3c4a5c 100644
--- a/restart.c
+++ b/restart.c
@@ -53,6 +53,7 @@ static char usage_str[] =
 "     --signal=SIG       send SIG to root task on SIGINT (default: SIGKILL\n"
 "                        to container root, SIGINT otherwise)\n"
 "     --mntns            restart under a private mounts namespace\n"
+"     --mount-pty        start in a new devpts namespace to supprt ptys\n"
 "  -w,--wait             wait for root task to termiate (default)\n"
 "     --show-status      show exit status of root task (implies -w)\n"
 "     --copy-status      imitate exit status of root task (implies -w)\n"
@@ -275,6 +276,7 @@ int global_send_sigint = -1;
 int global_sent_sigint;
 
 static int ckpt_remount_proc(void);
+static int ckpt_remount_devpts(void);
 
 static int ckpt_build_tree(struct ckpt_ctx *ctx);
 static int ckpt_init_tree(struct ckpt_ctx *ctx);
@@ -342,6 +344,7 @@ struct args {
 	char *root;
 	int wait;
 	int mntns;
+	int mnt_pty;
 	int show_status;
 	int copy_status;
 	char *freezer;
@@ -432,6 +435,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		{ "debug",	no_argument,		NULL, 'd' },
 		{ "warn-pidzero",	no_argument,	NULL, 9 },
 		{ "fail-pidzero",	no_argument,	NULL, 10 },
+		{ "mount-pty",	no_argument,		NULL, 12 },
 		{ NULL,		0,			NULL, 0 }
 	};
 	static char optc[] = "hdvkpPwWF:r:i:l:";
@@ -538,6 +542,9 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		case 11:
 			args->mntns = 1;
 			break;
+		case 12:
+			args->mnt_pty = 1;
+			break;
 		default:
 			usage(usage_str);
 		}
@@ -591,6 +598,9 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		printf("Invalid used of both -l/--logfile and --logfile-fd\n");
 		exit(1);
 	}
+
+	if (args->mnt_pty)
+		args->mntns = 1;
 }
 
 static void report_exit_status(int status, char *str, int debug)
@@ -785,6 +795,10 @@ int main(int argc, char *argv[])
 		exit(1);
 	}
 
+	/* remount /dev/pts ? */
+	if (args.mnt_pty && ckpt_remount_devpts() < 0)
+		exit(1);
+
 	/* self-restart ends here: */
 	if (args.self) {
 		restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args.logfd);
@@ -916,6 +930,32 @@ static int ckpt_collect_child(struct ckpt_ctx *ctx)
 	return ckpt_parse_status(status, mimic, verbose);
 }
 
+static int ckpt_remount_devpts(void)
+{
+	struct stat ptystat;
+
+	/* make sure /dev/ptmx is a link else we'll just break */
+	if (lstat("/dev/ptmx", &ptystat) < 0) {
+		perror("stat /dev/ptmx");
+		return -1;
+	}
+	if ((ptystat.st_mode & S_IFMT) != S_IFLNK) {
+		ckpt_err("Error: /dev/ptmx must be a link to /dev/pts/ptmx\n");
+		return -1;
+	}
+
+	if (umount2("/dev/pts", MNT_DETACH) < 0) {
+		perror("umount -l /dev/pts");
+		return -1;
+	}
+	if (mount("pts", "/dev/pts", "devpts", 0, "newinstance") < 0) {
+		perror("mount -t devpts -o newinstance");
+		return -1;
+	}
+
+	return 0;
+}
+
 static int ckpt_close_files(void)
 {
 	char fdpath[64], *endp;
-- 
1.6.3.3

Re: [user-cr][PATCH 1/2] restart: remount /proc for new tasks created with CLONE_NEWPID

[Serge E. Hallyn]

Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
> Not doing this can be a pain for restarted software which relies
> on /proc...
> 
> This builds on a patch by Serge Hallyn, but also aims to address the
> future cases of hierarchical pid-ns:
> 
> 1) Before mounting the new /proc, first umount the old one, which
> isn't necessary anymore.
> 
> 2) Perform the unshare() together with the remount of /proc, so it
> will occur for every new pid-ns and not only for the first one.
> 
> Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
> ---
>  restart.c |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/restart.c b/restart.c
> index f3d33de..f42b456 100644
> --- a/restart.c
> +++ b/restart.c
> @@ -30,6 +30,7 @@
>  #include <asm/unistd.h>
>  #include <sys/syscall.h>
>  #include <sys/prctl.h>
> +#include <sys/mount.h>
> 
>  #include <linux/sched.h>
>  #include <linux/checkpoint.h>
> @@ -273,6 +274,8 @@ int global_child_collected;
>  int global_send_sigint = -1;
>  int global_sent_sigint;
> 
> +static int ckpt_remount_proc(void);
> +
>  static int ckpt_build_tree(struct ckpt_ctx *ctx);
>  static int ckpt_init_tree(struct ckpt_ctx *ctx);
>  static int ckpt_set_creator(struct ckpt_ctx *ctx, struct task *task);
> @@ -981,11 +984,36 @@ static int ckpt_probe_child(pid_t pid, char *str)
>  	return 0;
>  }
> 
> +/*
> + * Remount the /proc with a new instance: tasks that start a new
> + * pid-ns need a fresh mount of /proc to reflect their pid-ns.
> + */
> +static int ckpt_remount_proc(void)
> +{
> +	if (unshare(CLONE_NEWNS | CLONE_FS) < 0) {
> +		perror("unshare");
> +		return -1;
> +	}
> +	if (umount2("/proc", MNT_DETACH) < 0) {
> +		perror("umount -l /proc");
> +		return -1;
> +	}

I don't expect this to ever happen in practice, but *if*
somehow /proc were already unmounted, you'd fail restart
for no good reason.  I don't know that we care about a
failure to umount here.

In fact I suppose in a 'restart -r /opt/container1' /proc
might not yet be mounted after chroot?  So it's not all
that unlikely.

> +	if (mount("proc", "/proc", "proc", 0, NULL) < 0) {
> +		perror("mount -t proc");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  #ifdef CLONE_NEWPID
>  static int __ckpt_coordinator(void *arg)
>  {
>  	struct ckpt_ctx *ctx = (struct ckpt_ctx *) arg;
> 
> +	if (ckpt_remount_proc() < 0)
> +		return -1;
> +
>  	if (!ctx->args->wait)
>  		close(ctx->pipe_coord[0]);
> 
> @@ -1850,6 +1878,10 @@ int ckpt_fork_stub(void *data)
>  	struct task *task = (struct task *) data;
>  	struct ckpt_ctx *ctx = task->ctx;
> 
> +	/* tasks with new pid-ns need new /proc mount */
> +	if ((task->flags & TASK_NEWPID) && ckpt_remount_proc() < 0)
> +		return -1;
> +
>  	/*
>  	 * In restart into a new pid namespace (--pidns), coordinator
>  	 * is the container init, hence if it terminated permatutely
> -- 
> 1.6.3.3

Re: [user-cr][PATCH 2/2] Add --mount-pty option to mount new devpts

[Serge E. Hallyn]

Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
> Based on Serge Hallyn's patch:
> 
>   "Trivial patch, and I'm not sure whether we want this or want to do
>   it this way.  But it saves me having to do it during my restart.sh
>   wrapper shell-script."
> 
> Make sure that the old /dev/pts is un-mounted prior to the mount of
> the new one.
> 
> Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>

Thanks, Oren.  For both patches:

Signed-off-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

(regardless of which way you decide to go with the /proc umount error
handling)

thanks,
-serge

> ---
>  restart.c |   40 ++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 40 insertions(+), 0 deletions(-)
> 
> diff --git a/restart.c b/restart.c
> index f42b456..e3c4a5c 100644
> --- a/restart.c
> +++ b/restart.c
> @@ -53,6 +53,7 @@ static char usage_str[] =
>  "     --signal=SIG       send SIG to root task on SIGINT (default: SIGKILL\n"
>  "                        to container root, SIGINT otherwise)\n"
>  "     --mntns            restart under a private mounts namespace\n"
> +"     --mount-pty        start in a new devpts namespace to supprt ptys\n"
>  "  -w,--wait             wait for root task to termiate (default)\n"
>  "     --show-status      show exit status of root task (implies -w)\n"
>  "     --copy-status      imitate exit status of root task (implies -w)\n"
> @@ -275,6 +276,7 @@ int global_send_sigint = -1;
>  int global_sent_sigint;
> 
>  static int ckpt_remount_proc(void);
> +static int ckpt_remount_devpts(void);
> 
>  static int ckpt_build_tree(struct ckpt_ctx *ctx);
>  static int ckpt_init_tree(struct ckpt_ctx *ctx);
> @@ -342,6 +344,7 @@ struct args {
>  	char *root;
>  	int wait;
>  	int mntns;
> +	int mnt_pty;
>  	int show_status;
>  	int copy_status;
>  	char *freezer;
> @@ -432,6 +435,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
>  		{ "debug",	no_argument,		NULL, 'd' },
>  		{ "warn-pidzero",	no_argument,	NULL, 9 },
>  		{ "fail-pidzero",	no_argument,	NULL, 10 },
> +		{ "mount-pty",	no_argument,		NULL, 12 },
>  		{ NULL,		0,			NULL, 0 }
>  	};
>  	static char optc[] = "hdvkpPwWF:r:i:l:";
> @@ -538,6 +542,9 @@ static void parse_args(struct args *args, int argc, char *argv[])
>  		case 11:
>  			args->mntns = 1;
>  			break;
> +		case 12:
> +			args->mnt_pty = 1;
> +			break;
>  		default:
>  			usage(usage_str);
>  		}
> @@ -591,6 +598,9 @@ static void parse_args(struct args *args, int argc, char *argv[])
>  		printf("Invalid used of both -l/--logfile and --logfile-fd\n");
>  		exit(1);
>  	}
> +
> +	if (args->mnt_pty)
> +		args->mntns = 1;
>  }
> 
>  static void report_exit_status(int status, char *str, int debug)
> @@ -785,6 +795,10 @@ int main(int argc, char *argv[])
>  		exit(1);
>  	}
> 
> +	/* remount /dev/pts ? */
> +	if (args.mnt_pty && ckpt_remount_devpts() < 0)
> +		exit(1);
> +
>  	/* self-restart ends here: */
>  	if (args.self) {
>  		restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args.logfd);
> @@ -916,6 +930,32 @@ static int ckpt_collect_child(struct ckpt_ctx *ctx)
>  	return ckpt_parse_status(status, mimic, verbose);
>  }
> 
> +static int ckpt_remount_devpts(void)
> +{
> +	struct stat ptystat;
> +
> +	/* make sure /dev/ptmx is a link else we'll just break */
> +	if (lstat("/dev/ptmx", &ptystat) < 0) {
> +		perror("stat /dev/ptmx");
> +		return -1;
> +	}
> +	if ((ptystat.st_mode & S_IFMT) != S_IFLNK) {
> +		ckpt_err("Error: /dev/ptmx must be a link to /dev/pts/ptmx\n");
> +		return -1;
> +	}
> +
> +	if (umount2("/dev/pts", MNT_DETACH) < 0) {
> +		perror("umount -l /dev/pts");
> +		return -1;
> +	}
> +	if (mount("pts", "/dev/pts", "devpts", 0, "newinstance") < 0) {
> +		perror("mount -t devpts -o newinstance");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  static int ckpt_close_files(void)
>  {
>  	char fdpath[64], *endp;
> -- 
> 1.6.3.3

Re: [user-cr][PATCH 2/2] Add --mount-pty option to mount new devpts

[Serge E. Hallyn]

Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
> Based on Serge Hallyn's patch:
> 
>   "Trivial patch, and I'm not sure whether we want this or want to do
>   it this way.  But it saves me having to do it during my restart.sh
>   wrapper shell-script."
> 
> Make sure that the old /dev/pts is un-mounted prior to the mount of
> the new one.
> 
> Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
> ---
>  restart.c |   40 ++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 40 insertions(+), 0 deletions(-)
> 
> diff --git a/restart.c b/restart.c
> index f42b456..e3c4a5c 100644
> --- a/restart.c
> +++ b/restart.c
> @@ -53,6 +53,7 @@ static char usage_str[] =
>  "     --signal=SIG       send SIG to root task on SIGINT (default: SIGKILL\n"
>  "                        to container root, SIGINT otherwise)\n"
>  "     --mntns            restart under a private mounts namespace\n"
> +"     --mount-pty        start in a new devpts namespace to supprt ptys\n"
>  "  -w,--wait             wait for root task to termiate (default)\n"
>  "     --show-status      show exit status of root task (implies -w)\n"
>  "     --copy-status      imitate exit status of root task (implies -w)\n"
> @@ -275,6 +276,7 @@ int global_send_sigint = -1;
>  int global_sent_sigint;
> 
>  static int ckpt_remount_proc(void);
> +static int ckpt_remount_devpts(void);
> 
>  static int ckpt_build_tree(struct ckpt_ctx *ctx);
>  static int ckpt_init_tree(struct ckpt_ctx *ctx);
> @@ -342,6 +344,7 @@ struct args {
>  	char *root;
>  	int wait;
>  	int mntns;
> +	int mnt_pty;
>  	int show_status;
>  	int copy_status;
>  	char *freezer;
> @@ -432,6 +435,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
>  		{ "debug",	no_argument,		NULL, 'd' },
>  		{ "warn-pidzero",	no_argument,	NULL, 9 },
>  		{ "fail-pidzero",	no_argument,	NULL, 10 },
> +		{ "mount-pty",	no_argument,		NULL, 12 },
>  		{ NULL,		0,			NULL, 0 }
>  	};
>  	static char optc[] = "hdvkpPwWF:r:i:l:";
> @@ -538,6 +542,9 @@ static void parse_args(struct args *args, int argc, char *argv[])
>  		case 11:
>  			args->mntns = 1;
>  			break;
> +		case 12:
> +			args->mnt_pty = 1;
> +			break;
>  		default:
>  			usage(usage_str);
>  		}
> @@ -591,6 +598,9 @@ static void parse_args(struct args *args, int argc, char *argv[])
>  		printf("Invalid used of both -l/--logfile and --logfile-fd\n");
>  		exit(1);
>  	}
> +
> +	if (args->mnt_pty)
> +		args->mntns = 1;
>  }
> 
>  static void report_exit_status(int status, char *str, int debug)
> @@ -785,6 +795,10 @@ int main(int argc, char *argv[])
>  		exit(1);
>  	}
> 
> +	/* remount /dev/pts ? */
> +	if (args.mnt_pty && ckpt_remount_devpts() < 0)
> +		exit(1);
> +
>  	/* self-restart ends here: */
>  	if (args.self) {
>  		restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args.logfd);
> @@ -916,6 +930,32 @@ static int ckpt_collect_child(struct ckpt_ctx *ctx)
>  	return ckpt_parse_status(status, mimic, verbose);
>  }
> 
> +static int ckpt_remount_devpts(void)
> +{
> +	struct stat ptystat;
> +
> +	/* make sure /dev/ptmx is a link else we'll just break */
> +	if (lstat("/dev/ptmx", &ptystat) < 0) {
> +		perror("stat /dev/ptmx");
> +		return -1;
> +	}
> +	if ((ptystat.st_mode & S_IFMT) != S_IFLNK) {
> +		ckpt_err("Error: /dev/ptmx must be a link to /dev/pts/ptmx\n");
> +		return -1;
> +	}
> +
> +	if (umount2("/dev/pts", MNT_DETACH) < 0) {
> +		perror("umount -l /dev/pts");
> +		return -1;
> +	}
> +	if (mount("pts", "/dev/pts", "devpts", 0, "newinstance") < 0) {

How about making that

	if (mount("pts", "/dev/pts", "devpts", 0, "ptmxmode=666,newinstance")
				< 0) {

(or take that as another arg if you must)?  Otherwise restarting
non-root-owned tasks not only ends up with tasks not able to create
a new terminal, but you can't externally fix that since the mount
is private...

> +		perror("mount -t devpts -o newinstance");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  static int ckpt_close_files(void)
>  {
>  	char fdpath[64], *endp;
> -- 
> 1.6.3.3

Re: [user-cr][PATCH 2/2] Add --mount-pty option to mount new devpts

[Oren Laadan]

Serge E. Hallyn wrote:
> Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
>> Based on Serge Hallyn's patch:
>>
>>   "Trivial patch, and I'm not sure whether we want this or want to do
>>   it this way.  But it saves me having to do it during my restart.sh
>>   wrapper shell-script."
>>
>> Make sure that the old /dev/pts is un-mounted prior to the mount of
>> the new one.
>>
>> Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
>> ---
>>  restart.c |   40 ++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 40 insertions(+), 0 deletions(-)
>>
>> diff --git a/restart.c b/restart.c
>> index f42b456..e3c4a5c 100644
>> --- a/restart.c
>> +++ b/restart.c
>> @@ -53,6 +53,7 @@ static char usage_str[] =
>>  "     --signal=SIG       send SIG to root task on SIGINT (default: SIGKILL\n"
>>  "                        to container root, SIGINT otherwise)\n"
>>  "     --mntns            restart under a private mounts namespace\n"
>> +"     --mount-pty        start in a new devpts namespace to supprt ptys\n"
>>  "  -w,--wait             wait for root task to termiate (default)\n"
>>  "     --show-status      show exit status of root task (implies -w)\n"
>>  "     --copy-status      imitate exit status of root task (implies -w)\n"
>> @@ -275,6 +276,7 @@ int global_send_sigint = -1;
>>  int global_sent_sigint;
>>
>>  static int ckpt_remount_proc(void);
>> +static int ckpt_remount_devpts(void);
>>
>>  static int ckpt_build_tree(struct ckpt_ctx *ctx);
>>  static int ckpt_init_tree(struct ckpt_ctx *ctx);
>> @@ -342,6 +344,7 @@ struct args {
>>  	char *root;
>>  	int wait;
>>  	int mntns;
>> +	int mnt_pty;
>>  	int show_status;
>>  	int copy_status;
>>  	char *freezer;
>> @@ -432,6 +435,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
>>  		{ "debug",	no_argument,		NULL, 'd' },
>>  		{ "warn-pidzero",	no_argument,	NULL, 9 },
>>  		{ "fail-pidzero",	no_argument,	NULL, 10 },
>> +		{ "mount-pty",	no_argument,		NULL, 12 },
>>  		{ NULL,		0,			NULL, 0 }
>>  	};
>>  	static char optc[] = "hdvkpPwWF:r:i:l:";
>> @@ -538,6 +542,9 @@ static void parse_args(struct args *args, int argc, char *argv[])
>>  		case 11:
>>  			args->mntns = 1;
>>  			break;
>> +		case 12:
>> +			args->mnt_pty = 1;
>> +			break;
>>  		default:
>>  			usage(usage_str);
>>  		}
>> @@ -591,6 +598,9 @@ static void parse_args(struct args *args, int argc, char *argv[])
>>  		printf("Invalid used of both -l/--logfile and --logfile-fd\n");
>>  		exit(1);
>>  	}
>> +
>> +	if (args->mnt_pty)
>> +		args->mntns = 1;
>>  }
>>
>>  static void report_exit_status(int status, char *str, int debug)
>> @@ -785,6 +795,10 @@ int main(int argc, char *argv[])
>>  		exit(1);
>>  	}
>>
>> +	/* remount /dev/pts ? */
>> +	if (args.mnt_pty && ckpt_remount_devpts() < 0)
>> +		exit(1);
>> +
>>  	/* self-restart ends here: */
>>  	if (args.self) {
>>  		restart(getpid(), STDIN_FILENO, RESTART_TASKSELF, args.logfd);
>> @@ -916,6 +930,32 @@ static int ckpt_collect_child(struct ckpt_ctx *ctx)
>>  	return ckpt_parse_status(status, mimic, verbose);
>>  }
>>
>> +static int ckpt_remount_devpts(void)
>> +{
>> +	struct stat ptystat;
>> +
>> +	/* make sure /dev/ptmx is a link else we'll just break */
>> +	if (lstat("/dev/ptmx", &ptystat) < 0) {
>> +		perror("stat /dev/ptmx");
>> +		return -1;
>> +	}
>> +	if ((ptystat.st_mode & S_IFMT) != S_IFLNK) {
>> +		ckpt_err("Error: /dev/ptmx must be a link to /dev/pts/ptmx\n");
>> +		return -1;
>> +	}
>> +
>> +	if (umount2("/dev/pts", MNT_DETACH) < 0) {
>> +		perror("umount -l /dev/pts");
>> +		return -1;
>> +	}
>> +	if (mount("pts", "/dev/pts", "devpts", 0, "newinstance") < 0) {
> 
> How about making that
> 
> 	if (mount("pts", "/dev/pts", "devpts", 0, "ptmxmode=666,newinstance")
> 				< 0) {
> 
> (or take that as another arg if you must)?  Otherwise restarting
> non-root-owned tasks not only ends up with tasks not able to create
> a new terminal, but you can't externally fix that since the mount
> is private...

Ok. And also either umount() won't fail by default.

Oren.

> 
>> +		perror("mount -t devpts -o newinstance");
>> +		return -1;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int ckpt_close_files(void)
>>  {
>>  	char fdpath[64], *endp;
>> -- 
>> 1.6.3.3
>