[CRIU] [PATCH v3 1/6] files-reg: Account number of unlinked parent components of ghost file

[Pavel Emelyanov]

On 12/29/2015 05:05 PM, Kirill Tkhai wrote:
> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
> ---
>  files-reg.c               |   69 ++++++++++++++++++++++++++++++++++++++-------
>  protobuf/ghost-file.proto |    1 +
>  2 files changed, 60 insertions(+), 10 deletions(-)
> 
> diff --git a/files-reg.c b/files-reg.c
> index 0f89be5..3e09ae0 100644
> --- a/files-reg.c
> +++ b/files-reg.c
> @@ -48,6 +48,7 @@ struct ghost_file {
>  
>  	u32			dev;
>  	u32			ino;
> +	u32			nr_up;
>  
>  	struct file_remap	remap;
>  };
> @@ -223,6 +224,7 @@ static int open_remap_ghost(struct reg_file_info *rfi,
>  	gf->dev = gfe->dev;
>  	gf->ino = gfe->ino;
>  	gf->remap.rmnt_id = rfi->rfe->mnt_id;
> +	gf->nr_up = gfe->has_nr_up ? gfe->nr_up : 0;
>  
>  	if (S_ISDIR(gfe->mode))
>  		strncpy(gf->remap.rpath, rfi->path, PATH_MAX);
> @@ -526,7 +528,50 @@ static struct collect_image_info remap_cinfo = {
>  	.collect = collect_one_remap,
>  };
>  
> -static int dump_ghost_file(int _fd, u32 id, const struct stat *st, dev_t phys_dev)
> +static int count_unlinked_parents(GhostFileEntry *gfe, const char *path, int mntns_root)
> +{
> +	struct stat st;
> +	char buf[PATH_MAX], *p;
> +	int count = 0;
> +
> +	strcpy(buf, path);
> +
> +	p = strrchr(buf, '/');
> +	if (!p) {
> +		pr_err("No parent for %s", buf);
> +		count = -1;
> +		goto out;
> +	}
> +
> +	while (p > buf) {
> +		*p = '\0';
> +		if (fstatat(mntns_root, buf, &st, 0) == 0)

This place looks dangerous. Consider we get here after we've fstat()-ed the fd
and found out that its n_link is 0. Then we go on and stat() it by path. But
the path in question may refer to some other existing file and thus this whole
logic becomes flawed.

-- Pavel

> +			goto populate;
> +
> +		if (errno != ENOENT) {
> +			pr_perror("Can't stat %s", buf);
> +			count = -1;
> +			goto out;
> +		}
> +
> +		count++;
> +
> +		while (--p > buf && *p != '/')
> +			continue;
> +	}
> +
> +populate:
> +	if (count) {
> +		gfe->has_nr_up = true;
> +		gfe->nr_up = count;
> +		pr_debug("Ghost file %s has %d unlinked parent component(s)", path, count);
> +	}
> +out:
> +	return count;
> +}
> +
> +static int dump_ghost_file(char *path, int _fd, u32 id, const struct stat *st,
> +			   dev_t phys_dev, int mntns_root)
>  {
>  	struct cr_img *img;
>  	GhostFileEntry gfe = GHOST_FILE_ENTRY__INIT;
> @@ -553,6 +598,9 @@ static int dump_ghost_file(int _fd, u32 id, const struct stat *st, dev_t phys_de
>  	gfe.dev = phys_dev;
>  	gfe.ino = st->st_ino;
>  
> +	if (count_unlinked_parents(&gfe, path, mntns_root) < 0)
> +		return -1;
> +
>  	if (S_ISCHR(st->st_mode) || S_ISBLK(st->st_mode)) {
>  		gfe.has_rdev = true;
>  		gfe.rdev = st->st_rdev;
> @@ -616,8 +664,8 @@ struct file_remap *lookup_ghost_remap(u32 dev, u32 ino)
>  	return NULL;
>  }
>  
> -static int dump_ghost_remap(char *path, const struct stat *st,
> -				int lfd, u32 id, struct ns_id *nsid)
> +static int dump_ghost_remap(char *path, const struct stat *st, int lfd,
> +			    u32 id, struct ns_id *nsid, int mntns_root)
>  {
>  	struct ghost_file *gf;
>  	RemapFilePathEntry rpe = REMAP_FILE_PATH_ENTRY__INIT;
> @@ -631,7 +679,7 @@ static int dump_ghost_remap(char *path, const struct stat *st,
>  		return -1;
>  	}
>  
> -	phys_dev = phys_stat_resolve_dev(nsid, st->st_dev, path);
> +	phys_dev = phys_stat_resolve_dev(nsid, st->st_dev, path + 1);
>  	list_for_each_entry(gf, &ghost_files, list)
>  		if ((gf->dev == phys_dev) && (gf->ino == st->st_ino))
>  			goto dump_entry;
> @@ -645,7 +693,7 @@ static int dump_ghost_remap(char *path, const struct stat *st,
>  	gf->id = ghost_file_ids++;
>  	list_add_tail(&gf->list, &ghost_files);
>  
> -	if (dump_ghost_file(lfd, gf->id, st, phys_dev))
> +	if (dump_ghost_file(path, lfd, gf->id, st, phys_dev, mntns_root))
>  		return -1;
>  
>  dump_entry:
> @@ -918,6 +966,10 @@ static int check_path_remap(struct fd_link *link, const struct fd_parms *parms,
>  		return 0;
>  	}
>  
> +	mntns_root = mntns_get_root_fd(nsid);
> +	if (mntns_root < 0)
> +		return -1;
> +
>  	if (ost->st_nlink == 0) {
>  		/*
>  		 * Unpleasant, but easy case. File is completely invisible
> @@ -926,7 +978,8 @@ static int check_path_remap(struct fd_link *link, const struct fd_parms *parms,
>  		 * also open.
>  		 */
>  		strip_deleted(link);
> -		return dump_ghost_remap(rpath + 1, ost, lfd, id, nsid);
> +
> +		return dump_ghost_remap(rpath, ost, lfd, id, nsid, mntns_root);
>  	}
>  
>  	if (nfs_silly_rename(rpath, parms)) {
> @@ -941,10 +994,6 @@ static int check_path_remap(struct fd_link *link, const struct fd_parms *parms,
>  		return dump_linked_remap(rpath + 1, plen - 1, ost, lfd, id, nsid);
>  	}
>  
> -	mntns_root = mntns_get_root_fd(nsid);
> -	if (mntns_root < 0)
> -		return -1;
> -
>  	ret = fstatat(mntns_root, rpath, &pst, 0);
>  	if (ret < 0) {
>  		/*
> diff --git a/protobuf/ghost-file.proto b/protobuf/ghost-file.proto
> index db056db..6325c99 100644
> --- a/protobuf/ghost-file.proto
> +++ b/protobuf/ghost-file.proto
> @@ -11,4 +11,5 @@ message ghost_file_entry {
>  	optional uint32		rdev		= 6 [(criu).dev = true, (criu).odev = true];
>  	optional timeval	atim		= 7;
>  	optional timeval	mtim		= 8;
> +	optional uint32		nr_up		= 9; /* Number of unlinked parent directories */
>  }
> 
> .
>