[CRIU] Re: [PATCH] restorer: close log file before detaching from crtools

[Andrew Vagin]

On Mon, Feb 20, 2012 at 05:12:13PM +0300, Kinsbursky Stanislav wrote:
> There is a race between log close by process being restoring and opened file
> desctriptors check in zdtm test suite - crtools can exit and compare file
> descriptors before detached restored process will perform all the rest tasks
> (including close of the log) and execute final system call:
> 
> --- dump/sleeping00/8578/dump.fd        2012-02-20 14:31:31.246096000 +0300
> +++ dump/sleeping00/8578/restore.fd     2012-02-20 14:31:31.418095999 +0300
> @@ -1,4 +1,5 @@
> 
>  0 -> /dev/null
>  1 -> /dev/null
> +1023 -> /root/crtools/test/dump/sleeping00/8578/restore.log
>  2 -> /dev/null
> 
> The solution is to close log in restorer before final command received. But
> this leads to another problem: we have to inform somehow about possible errors
> afterwards This is done by forced segmentation fault and looks like this
> (dmesg):
> 
> pipe00[17373]: segfault at 2ed ip 00000000000002ed sp ffffffffffffffff error
> 14
> 
> Where "ip" is restorer's line number and "sp" is the reason why we failed.
> 
> Signed-off-by: Stanislav Kinsbursky <skinsbursky at openvz.org>
> 
> ---
>  restorer.c |   21 +++++++++++++++------
>  1 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/restorer.c b/restorer.c
> index ea4af04..d25a571 100644
> --- a/restorer.c
> +++ b/restorer.c
> @@ -336,7 +336,7 @@ static u64 restore_mapping(const struct vma_entry *vma_entry)
>   */
>  long restore_task(struct task_restore_core_args *args)
>  {
> -	long ret = -1;
> +	long ret = -1, line;
>  	struct task_entry *task_entry;
>  	struct core_entry *core_entry;
>  	struct vma_entry *vma_entry;
> @@ -722,6 +722,9 @@ long restore_task(struct task_restore_core_args *args)
>  	sys_sigaction(SIGCHLD, &args->sigchld_act, NULL);
>  
>  	cr_wait_dec(&args->task_entries->nr_in_progress);
> +
> +	sys_close(args->logfd);
> +
>  	cr_wait_while(&args->task_entries->start, CR_STATE_RESTORE_SIGCHLD);
>  
>  	/*
> @@ -742,13 +745,10 @@ long restore_task(struct task_restore_core_args *args)
>  
>  	ret = sys_munmap(args->task_entries, TASK_ENTRIES_SIZE);
>  	if (ret < 0) {
> -		write_num_n(__LINE__);
> -		write_num_n(ret);
> -		goto core_restore_end;
> +		line = __LINE__;
> +		goto core_restore_failed;
>  	}
>  
> -	sys_close(args->logfd);
> -
>  	/*
>  	 * Sigframe stack.
>  	 */
> @@ -773,4 +773,13 @@ core_restore_end:
>  	write_num_n(sys_getpid());
>  	sys_exit(-1);
>  	return -1;
> +
> +core_restore_failed:
> +	asm volatile(
> +		"movq %0, %%rsp				\n"
> +		"jmp *%1				\n"
> +		:
> +		: "r"(ret), "r"(line)
> +		: );
> +	return ret;
We have a similar code in BUG_ON_HANDLER, but this code is better, so
I think you can improve BUG_ON_HANDLER and use it.
>  }
>