[CRIU] [PATCH v4] fsnotify: skip non-direcory mounts

[Andrew Vagin]

Applied!

On Thu, Mar 09, 2017 at 11:38:29AM +0300, Pavel Tikhomirov wrote:
> To restore fsnotify's watches on files we need to find paths for each
> of them using handle we have in /proc/<pid>/fdinfo/<fsnotifyfd>.
> These handle is valid to open the file with open_by_handle_at if
> you have mount fd where the file lays. So we try open_by_handle_at
> for all possible mount fds we have.
> 
> But we can not do so for 'file' bind-mounts, as the way we open
> mount fd opens file instead and can hang on fifos or fail on sockets.
> (see check_open_handle->open_handle->open_mount code path, imagine
> lookup_mnt_sdev() found 'file' bind-mount, open_mount() failed(hanged)
> in __open_mountpoint() and if irmap_lookup() also was not successful the
> whole dump fails too)
> 
> So if we have file bindmount of fifo file, and we restore some
> inotify on other file on other mount with same s_dev, we hang forever
> on open.
> 
> So just skip non-directory mounts from inotify search we will find
> path for them on other mount(e.g. non-bindmount) with same s_dev.
> 
> https://jira.sw.ru/browse/PSBM-57362
> 
> v2: remove isdir hashing, improve commit message
> v3: make lookup_mnt_sdev handle only nondir mounts, add comment,
> move more expensive notdir_mountpoint check after s_dev
> v4: inverse notdir_mountpoint to be mnt_is_dir, now on error
> in mnt_is_dir mount is also skipped
> 
> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> ---
>  criu/fsnotify.c      |  2 ++
>  criu/include/mount.h |  1 +
>  criu/mount.c         | 27 ++++++++++++++++++++++++++-
>  3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/criu/fsnotify.c b/criu/fsnotify.c
> index 0dcb07c..65833ca 100644
> --- a/criu/fsnotify.c
> +++ b/criu/fsnotify.c
> @@ -132,6 +132,8 @@ static char *alloc_openable(unsigned int s_dev, unsigned long i_ino, FhEntry *f_
>  
>  		if (m->s_dev != s_dev)
>  			continue;
> +		if (!mnt_is_dir(m))
> +			continue;
>  
>  		mntfd = __open_mountpoint(m, -1);
>  		pr_debug("\t\tTrying via mntid %d root %s ns_mountpoint @%s (%d)\n",
> diff --git a/criu/include/mount.h b/criu/include/mount.h
> index a692b55..e60dd34 100644
> --- a/criu/include/mount.h
> +++ b/criu/include/mount.h
> @@ -87,6 +87,7 @@ extern struct ns_id *lookup_nsid_by_mnt_id(int mnt_id);
>  
>  extern int open_mount(unsigned int s_dev);
>  extern int __open_mountpoint(struct mount_info *pm, int mnt_fd);
> +extern int mnt_is_dir(struct mount_info *pm);
>  extern int open_mountpoint(struct mount_info *pm);
>  
>  extern struct mount_info *collect_mntinfo(struct ns_id *ns, bool for_dump);
> diff --git a/criu/mount.c b/criu/mount.c
> index 2973d27..147001b 100644
> --- a/criu/mount.c
> +++ b/criu/mount.c
> @@ -228,7 +228,11 @@ struct mount_info *lookup_mnt_sdev(unsigned int s_dev)
>  	struct mount_info *m;
>  
>  	for (m = mntinfo; m != NULL; m = m->next)
> -		if (m->s_dev == s_dev)
> +		/*
> +		 * We should not provide notdir bindmounts to open_mount as
> +		 * opening them can fail/hang for binds of unix sockets/fifos
> +		 */
> +		if (m->s_dev == s_dev && mnt_is_dir(m))
>  			return m;
>  
>  	return NULL;
> @@ -957,6 +961,27 @@ static struct mount_info *mnt_build_tree(struct mount_info *list,
>  	return tree;
>  }
>  
> +int mnt_is_dir(struct mount_info *pm)
> +{
> +	int mntns_root;
> +	struct stat st;
> +
> +	mntns_root = mntns_get_root_fd(pm->nsid);
> +	if (mntns_root < 0) {
> +		pr_perror("Can't get root fd of mntns for %d", pm->mnt_id);
> +		return 0;
> +	}
> +
> +	if (fstatat(mntns_root, pm->ns_mountpoint, &st, 0)) {
> +		pr_perror("Can't fstatat on %s", pm->ns_mountpoint);
> +		return 0;
> +	}
> +
> +	if (S_ISDIR(st.st_mode))
> +		return 1;
> +	return 0;
> +}
> +
>  /*
>   * mnt_fd is a file descriptor on the mountpoint, which is closed in an error case.
>   * If mnt_fd is -1, the mountpoint will be opened by this function.
> -- 
> 2.9.3
>