[CRIU] [PATCH] criu: fix segfault in pre-dump

[Andrei Vagin]

On Tue, Sep 11, 2018 at 09:51:15PM +0200, Adrian Reber wrote:
> From: Adrian Reber <areber at redhat.com>
> 
> By accident I found a segfault using pre-dump in combination with the
> page-server. Doing the following I was able to trigger it:
> 
>  * criu page-server -D /tmp/1
>  * criu pre-dump -t PID -D /tmp/3 --track-mem
>  * criu page-server -D /tmp/4 --prev-images-dir ../1
>  * criu pre-dump -t PID -D /tmp/3 --track-mem
>  --> segfault
> 
> ...
> (00.010090) Warn  (criu/image.c:134): Failed to open parent directory
> ...
> (00.012984) Error (criu/mem.c:318): Pid-reuse detection failed: no parent inventory, check warnings in get_parent_stats
> ...
> (00.013037) Error (criu/mem.c:544): Can't dump page with parasite
> ...
> (00.013955) Pre-dumping tasks' memory
> (00.013966) 	Pre-dumping 8793
> (00.014380) Transferring pages:
> Segmentation fault (core dumped)
> 
> Looking in cr-dump.c at cr_pre_dump_finish(int ret) the function gets
> the return code of the previous operations in 'ret' but it is
> immediately overwritten and never used.
> 
> In older CRIU versions it used to be:
> 
> 	if (ret < 0)
> 		goto err;
> 
> but that is gone now. So this reintroduces the check for the int
> parameter given to cr_pre_dump_finish() by the function caller.
> 
> As the commands used to trigged the segfault do not make much sense the
> result is still not usable and the same 'Warn' and 'Error' messages are
> printed, but the segfault is gone.
> 
> Signed-off-by: Adrian Reber <areber at redhat.com>
> ---
>  criu/cr-dump.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
> index ec5e1e45b..df31de1a9 100644
> --- a/criu/cr-dump.c
> +++ b/criu/cr-dump.c
> @@ -1474,10 +1474,11 @@ static int setup_alarm_handler()
>  	return 0;
>  }
>  
> -static int cr_pre_dump_finish(int ret)
> +static int cr_pre_dump_finish(int status)
>  {
>  	InventoryEntry he = INVENTORY_ENTRY__INIT;
>  	struct pstree_item *item;
> +	int ret;
>  
>  	/*
>  	 * Restore registers for tasks only. The threads have not been
> @@ -1495,6 +1496,9 @@ static int cr_pre_dump_finish(int ret)
>  
>  	timing_stop(TIME_FROZEN);
>  
> +	if (status < 0)

I think we need to set ret in this case

		ret = status;
> +		goto err;
> +
>  	pr_info("Pre-dumping tasks' memory\n");
>  	for_each_pstree_item(item) {
>  		struct parasite_ctl *ctl = dmpi(item)->parasite_ctl;
> -- 
> 2.17.1
>