[CRIU] [PATCH] irmap: Get root mntfd before releasing tasks on predump

[Andrew Vagin]

On Tue, Sep 30, 2014 at 11:57:54PM +0400, Pavel Emelyanov wrote:
> We have a use-after-free in predump code:
> 
> 1st the free_pstree() is called in pre_dump_tasks(), then we
> go to irmap_predump_run() which may call the lookup_irmap()
> which, in turn, dereferences the root_item to get the root
> mount ns fd.
> 
> But the problem is bigger than that. After we've released the
> tasks (done before freeing pstree on predump) we can no longer
> access them by PIDs, so keeping the root-item after irmap
> scan is not a fix.
> 
> Fix is to get the root fd before releasing the tasks and using
> one in irmap scanner.
> 
> Caught recently on iterative inotify_irmap test.
> 
> Signed-off-by: Pavel Emelyanov <xemul at parallels.com>

Acked-by: Andrew Vagin <avagin at parallels.com>

> 
> ---
> 
> diff --git a/cr-dump.c b/cr-dump.c
> index 04c18fa..e82f576 100644
> --- a/cr-dump.c
> +++ b/cr-dump.c
> @@ -1693,6 +1693,9 @@ int cr_pre_dump_tasks(pid_t pid)
>  		if (pre_dump_one_task(item, &ctls))
>  			goto err;
>  
> +	if (irmap_predump_prep())
> +		goto err;
> +
>  	ret = 0;
>  err:
>  	pstree_switch_state(root_item,
> diff --git a/include/irmap.h b/include/irmap.h
> index b5b495d..3938d07 100644
> --- a/include/irmap.h
> +++ b/include/irmap.h
> @@ -4,6 +4,7 @@ char *irmap_lookup(unsigned int s_dev, unsigned long i_ino);
>  struct _FhEntry;
>  int irmap_queue_cache(unsigned int dev, unsigned long ino,
>  		struct _FhEntry *fh);
> +int irmap_predump_prep(void);
>  int irmap_predump_run(void);
>  int check_open_handle(unsigned int s_dev, unsigned long i_ino,
>  		struct _FhEntry *f_handle);
> diff --git a/irmap.c b/irmap.c
> index ab8247e..10ebb0a 100644
> --- a/irmap.c
> +++ b/irmap.c
> @@ -217,6 +217,8 @@ invalid:
>  	return 1;
>  }
>  
> +static bool doing_predump = false;
> +
>  char *irmap_lookup(unsigned int s_dev, unsigned long i_ino)
>  {
>  	struct irmap *c, *h, **p;
> @@ -227,7 +229,14 @@ char *irmap_lookup(unsigned int s_dev, unsigned long i_ino)
>  
>  	pr_debug("Resolving %x:%lx path\n", s_dev, i_ino);
>  
> -	if (__mntns_get_root_fd(root_item->pid.real) < 0)
> +	/*
> +	 * If we're in predump, then processes already run
> +	 * and the root_item is already freed by that time.
> +	 * But the root service fd is already set by the
> +	 * irmap_predump_prep, so we just go ahead and scan.
> +	 */
> +	if (!doing_predump &&
> +			__mntns_get_root_fd(root_item->pid.real) < 0)
>  		goto out;
>  
>  	timing_start(TIME_IRMAP_RESOLVE);
> @@ -296,6 +305,22 @@ int irmap_queue_cache(unsigned int dev, unsigned long ino,
>  	return 0;
>  }
>  
> +int irmap_predump_prep(void)
> +{
> +	/*
> +	 * Tasks are about to get released soon, but
> +	 * we'll need to do FS scan for irmaps. In this
> +	 * scan we will need to know the root dir tasks
> +	 * live in. Need to make sure the respective fd
> +	 * (service) is set to that root, so that the
> +	 * scan works and doesn't race with the tasks
> +	 * dying or changind root.
> +	 */
> +
> +	doing_predump = true;
> +	return __mntns_get_root_fd(root_item->pid.real) < 0 ? -1 : 0;
> +}
> +
>  int irmap_predump_run(void)
>  {
>  	int ret = 0;
> 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu