pstree: Restore task group leader when inherit SID

Submitted by Radostin Stoyanov on Jan. 18, 2019, 12:43 a.m.

Details

Message ID 20190118004314.32680-1-rstoyanov1@gmail.com
State Accepted
Series "pstree: Restore task group leader when inherit SID"
Headers show

Commit Message

Radostin Stoyanov Jan. 18, 2019, 12:43 a.m.
The current behaviour of CRIU is to inherit the group leader of the
parent process when migrating a session leader (with --shell-job).
However, it is possible for a process to be group leader without being
a session leader (e.g. a shell process). In this case CRIU should
restore the original group ID of the process and inherit only the
session ID.

Closes #593

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
---
 criu/pstree.c | 27 ++++++++++++++++++---------
 criu/tty.c    | 13 +++++++++----
 2 files changed, 27 insertions(+), 13 deletions(-)

--
2.20.1

Patch hide | download patch | download mbox

diff --git a/criu/pstree.c b/criu/pstree.c
index 695110c45..92b4167aa 100644
--- a/criu/pstree.c
+++ b/criu/pstree.c
@@ -367,22 +367,31 @@  static int prepare_pstree_for_shell_job(pid_t pid)
 	 */

 	old_sid = root_item->sid;
-	old_gid = root_item->pgid;

-	pr_info("Migrating process tree (GID %d->%d SID %d->%d)\n",
-		old_gid, current_gid, old_sid, current_sid);
+	pr_info("Migrating process tree (SID %d->%d)\n",
+		old_sid, current_sid);

 	for_each_pstree_item(pi) {
-		if (pi->pgid == old_gid)
-			pi->pgid = current_gid;
 		if (pi->sid == old_sid)
 			pi->sid = current_sid;
 	}

-	if (lookup_create_item(current_sid) == NULL)
-		return -1;
-	if (lookup_create_item(current_gid) == NULL)
-		return -1;
+	old_gid = root_item->pgid;
+	if (old_gid != vpid(root_item)) {
+		if (lookup_create_item(current_sid) == NULL)
+			return -1;
+
+		pr_info("Migrating process tree (GID %d->%d)\n",
+			old_gid, current_gid);
+
+		for_each_pstree_item(pi) {
+			if (pi->pgid == old_gid)
+				pi->pgid = current_gid;
+		}
+
+		if (lookup_create_item(current_gid) == NULL)
+			return -1;
+	}

 	return 0;
 }
diff --git a/criu/tty.c b/criu/tty.c
index 38e1cab33..5aaca633a 100644
--- a/criu/tty.c
+++ b/criu/tty.c
@@ -1068,10 +1068,15 @@  out:
 		 * the process which keeps the master peer.
 		 */
 		if (root_item->sid != vpid(root_item)) {
-			pr_debug("Restore inherited group %d\n",
-				 getpgid(getppid()));
-			if (tty_set_prgp(fd, getpgid(getppid())))
-				goto err;
+			if (root_item->pgid == vpid(root_item)) {
+				if (tty_set_prgp(fd, root_item->pgid))
+					goto err;
+			} else {
+				pr_debug("Restore inherited group %d\n",
+					getpgid(getppid()));
+				if (tty_set_prgp(fd, getpgid(getppid())))
+					goto err;
+			}
 		}
 	}


Comments

Cyrill Gorcunov Jan. 18, 2019, 7:30 a.m.
On Fri, Jan 18, 2019 at 12:43:14AM +0000, Radostin Stoyanov wrote:
> The current behaviour of CRIU is to inherit the group leader of the
> parent process when migrating a session leader (with --shell-job).
> However, it is possible for a process to be group leader without being
> a session leader (e.g. a shell process). In this case CRIU should
> restore the original group ID of the process and inherit only the
> session ID.
> 
> Closes #593
> 
> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>

Good catch! Will try to review more carefully today.
Cyrill Gorcunov Jan. 18, 2019, 5:38 p.m.
On Fri, Jan 18, 2019 at 12:43:14AM +0000, Radostin Stoyanov wrote:
> The current behaviour of CRIU is to inherit the group leader of the
> parent process when migrating a session leader (with --shell-job).
> However, it is possible for a process to be group leader without being
> a session leader (e.g. a shell process). In this case CRIU should
> restore the original group ID of the process and inherit only the
> session ID.
> 
> Closes #593
> 
> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>
Andrei Vagin Jan. 19, 2019, 4:11 a.m.
Applied, thanks!

On Fri, Jan 18, 2019 at 12:43:14AM +0000, Radostin Stoyanov wrote:
> The current behaviour of CRIU is to inherit the group leader of the
> parent process when migrating a session leader (with --shell-job).
> However, it is possible for a process to be group leader without being
> a session leader (e.g. a shell process). In this case CRIU should
> restore the original group ID of the process and inherit only the
> session ID.
> 
> Closes #593
> 
> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
> ---
>  criu/pstree.c | 27 ++++++++++++++++++---------
>  criu/tty.c    | 13 +++++++++----
>  2 files changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/criu/pstree.c b/criu/pstree.c
> index 695110c45..92b4167aa 100644
> --- a/criu/pstree.c
> +++ b/criu/pstree.c
> @@ -367,22 +367,31 @@ static int prepare_pstree_for_shell_job(pid_t pid)
>  	 */
> 
>  	old_sid = root_item->sid;
> -	old_gid = root_item->pgid;
> 
> -	pr_info("Migrating process tree (GID %d->%d SID %d->%d)\n",
> -		old_gid, current_gid, old_sid, current_sid);
> +	pr_info("Migrating process tree (SID %d->%d)\n",
> +		old_sid, current_sid);
> 
>  	for_each_pstree_item(pi) {
> -		if (pi->pgid == old_gid)
> -			pi->pgid = current_gid;
>  		if (pi->sid == old_sid)
>  			pi->sid = current_sid;
>  	}
> 
> -	if (lookup_create_item(current_sid) == NULL)
> -		return -1;
> -	if (lookup_create_item(current_gid) == NULL)
> -		return -1;
> +	old_gid = root_item->pgid;
> +	if (old_gid != vpid(root_item)) {
> +		if (lookup_create_item(current_sid) == NULL)
> +			return -1;
> +
> +		pr_info("Migrating process tree (GID %d->%d)\n",
> +			old_gid, current_gid);
> +
> +		for_each_pstree_item(pi) {
> +			if (pi->pgid == old_gid)
> +				pi->pgid = current_gid;
> +		}
> +
> +		if (lookup_create_item(current_gid) == NULL)
> +			return -1;
> +	}
> 
>  	return 0;
>  }
> diff --git a/criu/tty.c b/criu/tty.c
> index 38e1cab33..5aaca633a 100644
> --- a/criu/tty.c
> +++ b/criu/tty.c
> @@ -1068,10 +1068,15 @@ out:
>  		 * the process which keeps the master peer.
>  		 */
>  		if (root_item->sid != vpid(root_item)) {
> -			pr_debug("Restore inherited group %d\n",
> -				 getpgid(getppid()));
> -			if (tty_set_prgp(fd, getpgid(getppid())))
> -				goto err;
> +			if (root_item->pgid == vpid(root_item)) {
> +				if (tty_set_prgp(fd, root_item->pgid))
> +					goto err;
> +			} else {
> +				pr_debug("Restore inherited group %d\n",
> +					getpgid(getppid()));
> +				if (tty_set_prgp(fd, getpgid(getppid())))
> +					goto err;
> +			}
>  		}
>  	}
> 
> --
> 2.20.1
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu