[PATCH user-cr] fix sid swap buglet

[PATCH user-cr] fix sid swap buglet

[Serge E. Hallyn]

In the case of a restart of a subtree checkpoint, some sids
may be recorded as 0.  If so, then we substitute the coordinator's
sid.  The code to do that wasn't quite right, and in fact writes
past the end of the pids_arr, which happens to be into the
copy_arr.  As a result, the kernel was getting bogus data for
substituted pids.  In my own case that lead to sys_restart()
waiting on a bogus vpid (in particular, the pid of my bash shell).

Fix that.

Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 restart.c |   22 ++++++++++++----------
 1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/restart.c b/restart.c
index 20f5055..f0b7862 100644
--- a/restart.c
+++ b/restart.c
@@ -2029,17 +2029,19 @@ static int ckpt_adjust_pids(struct ckpt_ctx *ctx)
 			if (ctx->pids_arr[m].vpgid == swap.old)
 				ctx->copy_arr[m].vpgid = swap.new;
 		}
+	}
 
-		/*
-		 * If the task's {sid,pgid} was zeroed out (inside
-		 * ckpt_init_tree) then substitute the coordinator's
-		 * sid for it now. (This should leave no more 0's in
-		 * restart of a subtree-checkpoint).
-		 */
-		if (ctx->tasks_arr[n].flags & TASK_ZERO_SID)
-			ctx->pids_arr[m].vsid = coord_sid;
-		if (ctx->tasks_arr[n].flags & TASK_ZERO_PGID)
-			ctx->pids_arr[m].vpgid = coord_sid;
+	/*
+	 * If the task's {sid,pgid} was zeroed out (inside
+	 * ckpt_init_tree) then substitute the coordinator's
+	 * sid for it now. (This should leave no more 0's in
+	 * restart of a subtree-checkpoint).
+	 */
+	for (m = 0; m < ctx->pids_nr; m++) {
+		if (ctx->tasks_arr[m].flags & TASK_ZERO_SID)
+			ctx->copy_arr[m].vsid = coord_sid;
+		if (ctx->tasks_arr[m].flags & TASK_ZERO_PGID)
+			ctx->copy_arr[m].vpgid = coord_sid;
 	}
 
 	memcpy(ctx->pids_arr, ctx->copy_arr, len);
-- 
1.6.1.1

Re: [PATCH user-cr] fix sid swap buglet

[Oren Laadan]

Oops ...already patched and pushed as per our discussion on IRC.
The patch is functionally identical but adds a comment.

Oren.

Serge E. Hallyn wrote:
> In the case of a restart of a subtree checkpoint, some sids
> may be recorded as 0.  If so, then we substitute the coordinator's
> sid.  The code to do that wasn't quite right, and in fact writes
> past the end of the pids_arr, which happens to be into the
> copy_arr.  As a result, the kernel was getting bogus data for
> substituted pids.  In my own case that lead to sys_restart()
> waiting on a bogus vpid (in particular, the pid of my bash shell).
> 
> Fix that.
> 
> Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> ---
>  restart.c |   22 ++++++++++++----------
>  1 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/restart.c b/restart.c
> index 20f5055..f0b7862 100644
> --- a/restart.c
> +++ b/restart.c
> @@ -2029,17 +2029,19 @@ static int ckpt_adjust_pids(struct ckpt_ctx *ctx)
>  			if (ctx->pids_arr[m].vpgid == swap.old)
>  				ctx->copy_arr[m].vpgid = swap.new;
>  		}
> +	}
>  
> -		/*
> -		 * If the task's {sid,pgid} was zeroed out (inside
> -		 * ckpt_init_tree) then substitute the coordinator's
> -		 * sid for it now. (This should leave no more 0's in
> -		 * restart of a subtree-checkpoint).
> -		 */
> -		if (ctx->tasks_arr[n].flags & TASK_ZERO_SID)
> -			ctx->pids_arr[m].vsid = coord_sid;
> -		if (ctx->tasks_arr[n].flags & TASK_ZERO_PGID)
> -			ctx->pids_arr[m].vpgid = coord_sid;
> +	/*
> +	 * If the task's {sid,pgid} was zeroed out (inside
> +	 * ckpt_init_tree) then substitute the coordinator's
> +	 * sid for it now. (This should leave no more 0's in
> +	 * restart of a subtree-checkpoint).
> +	 */
> +	for (m = 0; m < ctx->pids_nr; m++) {
> +		if (ctx->tasks_arr[m].flags & TASK_ZERO_SID)
> +			ctx->copy_arr[m].vsid = coord_sid;
> +		if (ctx->tasks_arr[m].flags & TASK_ZERO_PGID)
> +			ctx->copy_arr[m].vpgid = coord_sid;
>  	}
>  
>  	memcpy(ctx->pids_arr, ctx->copy_arr, len);