[4/5] files: open files on overmounted mounts are not supported

Submitted by Pavel Tikhomirov on Sept. 17, 2018, 11:47 a.m.

Details

Message ID 20180917114757.21453-5-ptikhomirov@virtuozzo.com
State New
Series "support restoring ghost files on readonly mounts"
Headers show

Commit Message

Pavel Tikhomirov Sept. 17, 2018, 11:47 a.m.
Files from such mounts can switch on restore to different files on the
overmounting mount, as we yet don't fully control the mount on which
the file is restored.

Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 criu/files-reg.c     | 13 +++++++++----
 criu/include/mount.h |  1 +
 criu/mount.c         |  2 +-
 3 files changed, 11 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/files-reg.c b/criu/files-reg.c
index e3ed01f55..f7247e488 100644
--- a/criu/files-reg.c
+++ b/criu/files-reg.c
@@ -1288,7 +1288,7 @@  static bool should_check_size(int flags)
 int dump_one_reg_file(int lfd, u32 id, const struct fd_parms *p)
 {
 	struct fd_link _link, *link;
-	struct ns_id *nsid;
+	struct mount_info *mi;
 	struct cr_img *rimg;
 	char ext_id[64];
 	FileEntry fe = FILE_ENTRY__INIT;
@@ -1312,13 +1312,18 @@  int dump_one_reg_file(int lfd, u32 id, const struct fd_parms *p)
 		goto ext;
 	}
 
-	nsid = lookup_nsid_by_mnt_id(p->mnt_id);
-	if (nsid == NULL) {
+	mi = lookup_mnt_id(p->mnt_id);
+	if (mi == NULL) {
 		pr_err("Can't lookup mount=%d for fd=%d path=%s\n",
 			p->mnt_id, p->fd, link->name + 1);
 		return -1;
 	}
 
+	if (mnt_is_overmounted(mi)) {
+		pr_err("Open files on overmounted mounts are not supported yet\n");
+		return -1;
+	}
+
 	if (p->mnt_id >= 0 && (root_ns_mask & CLONE_NEWNS)) {
 		rfe.mnt_id = p->mnt_id;
 		rfe.has_mnt_id = true;
@@ -1335,7 +1340,7 @@  int dump_one_reg_file(int lfd, u32 id, const struct fd_parms *p)
 		return -1;
 	}
 
-	if (check_path_remap(link, p, lfd, id, nsid))
+	if (check_path_remap(link, p, lfd, id, mi->nsid))
 		return -1;
 	rfe.name	= &link->name[1];
 ext:
diff --git a/criu/include/mount.h b/criu/include/mount.h
index 776c348cd..43cfe788a 100644
--- a/criu/include/mount.h
+++ b/criu/include/mount.h
@@ -129,5 +129,6 @@  extern struct mount_info *parse_mountinfo(pid_t pid, struct ns_id *nsid, bool fo
 extern int check_mnt_id(void);
 
 extern int remount_readonly_mounts(void);
+extern int mnt_is_overmounted(struct mount_info *mi);
 
 #endif /* __CR_MOUNT_H__ */
diff --git a/criu/mount.c b/criu/mount.c
index 3c19c280b..c28af0fe2 100644
--- a/criu/mount.c
+++ b/criu/mount.c
@@ -1152,7 +1152,7 @@  static int get_clean_fd(struct mount_info *mi)
  * root of our mount namespace as it is covered by other mount.
  * mnt_is_overmounted() checks if mount is not visible.
  */
-static int mnt_is_overmounted(struct mount_info *mi)
+int mnt_is_overmounted(struct mount_info *mi)
 {
 	struct mount_info *t, *c, *m = mi;
 

Comments

Andrey Vagin Sept. 26, 2018, 8:16 p.m.
On Mon, Sep 17, 2018 at 02:47:56PM +0300, Pavel Tikhomirov wrote:
> Files from such mounts can switch on restore to different files on the
> overmounting mount, as we yet don't fully control the mount on which
> the file is restored.

I don't understand what is going on here with overmounted mounts. Could
you explain with more details.

> 
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
>  criu/files-reg.c     | 13 +++++++++----
>  criu/include/mount.h |  1 +
>  criu/mount.c         |  2 +-
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/criu/files-reg.c b/criu/files-reg.c
> index e3ed01f55..f7247e488 100644
> --- a/criu/files-reg.c
> +++ b/criu/files-reg.c
> @@ -1288,7 +1288,7 @@ static bool should_check_size(int flags)
>  int dump_one_reg_file(int lfd, u32 id, const struct fd_parms *p)
>  {
>  	struct fd_link _link, *link;
> -	struct ns_id *nsid;
> +	struct mount_info *mi;
>  	struct cr_img *rimg;
>  	char ext_id[64];
>  	FileEntry fe = FILE_ENTRY__INIT;
> @@ -1312,13 +1312,18 @@ int dump_one_reg_file(int lfd, u32 id, const struct fd_parms *p)
>  		goto ext;
>  	}
>  
> -	nsid = lookup_nsid_by_mnt_id(p->mnt_id);
> -	if (nsid == NULL) {
> +	mi = lookup_mnt_id(p->mnt_id);
> +	if (mi == NULL) {
>  		pr_err("Can't lookup mount=%d for fd=%d path=%s\n",
>  			p->mnt_id, p->fd, link->name + 1);
>  		return -1;
>  	}
>  
> +	if (mnt_is_overmounted(mi)) {
> +		pr_err("Open files on overmounted mounts are not supported yet\n");
> +		return -1;
> +	}
> +
>  	if (p->mnt_id >= 0 && (root_ns_mask & CLONE_NEWNS)) {
>  		rfe.mnt_id = p->mnt_id;
>  		rfe.has_mnt_id = true;
> @@ -1335,7 +1340,7 @@ int dump_one_reg_file(int lfd, u32 id, const struct fd_parms *p)
>  		return -1;
>  	}
>  
> -	if (check_path_remap(link, p, lfd, id, nsid))
> +	if (check_path_remap(link, p, lfd, id, mi->nsid))
>  		return -1;
>  	rfe.name	= &link->name[1];
>  ext:
> diff --git a/criu/include/mount.h b/criu/include/mount.h
> index 776c348cd..43cfe788a 100644
> --- a/criu/include/mount.h
> +++ b/criu/include/mount.h
> @@ -129,5 +129,6 @@ extern struct mount_info *parse_mountinfo(pid_t pid, struct ns_id *nsid, bool fo
>  extern int check_mnt_id(void);
>  
>  extern int remount_readonly_mounts(void);
> +extern int mnt_is_overmounted(struct mount_info *mi);
>  
>  #endif /* __CR_MOUNT_H__ */
> diff --git a/criu/mount.c b/criu/mount.c
> index 3c19c280b..c28af0fe2 100644
> --- a/criu/mount.c
> +++ b/criu/mount.c
> @@ -1152,7 +1152,7 @@ static int get_clean_fd(struct mount_info *mi)
>   * root of our mount namespace as it is covered by other mount.
>   * mnt_is_overmounted() checks if mount is not visible.
>   */
> -static int mnt_is_overmounted(struct mount_info *mi)
> +int mnt_is_overmounted(struct mount_info *mi)
>  {
>  	struct mount_info *t, *c, *m = mi;
>  
> -- 
> 2.17.1
>
Pavel Tikhomirov Sept. 27, 2018, 7:26 a.m.
On 09/26/2018 11:16 PM, Andrey Vagin wrote:
> On Mon, Sep 17, 2018 at 02:47:56PM +0300, Pavel Tikhomirov wrote:
>> Files from such mounts can switch on restore to different files on the
>> overmounting mount, as we yet don't fully control the mount on which
>> the file is restored.
> 
> I don't understand what is going on here with overmounted mounts. Could
> you explain with more details.

If before restore the file was open on overmounted mount, dentry is 
invisible, on dump and at restore stage we try to access the file by 
path on visible overmount instead of looking under it, so we actually 
work with wrong file or wrongly detect link-remap.

See the test (patch 5) where mount of open file is changed from 
overmounted to overmount, which is wrong.

> 
>>
>> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
>> ---
>>   criu/files-reg.c     | 13 +++++++++----
>>   criu/include/mount.h |  1 +
>>   criu/mount.c         |  2 +-
>>   3 files changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/criu/files-reg.c b/criu/files-reg.c
>> index e3ed01f55..f7247e488 100644
>> --- a/criu/files-reg.c
>> +++ b/criu/files-reg.c
>> @@ -1288,7 +1288,7 @@ static bool should_check_size(int flags)
>>   int dump_one_reg_file(int lfd, u32 id, const struct fd_parms *p)
>>   {
>>   	struct fd_link _link, *link;
>> -	struct ns_id *nsid;
>> +	struct mount_info *mi;
>>   	struct cr_img *rimg;
>>   	char ext_id[64];
>>   	FileEntry fe = FILE_ENTRY__INIT;
>> @@ -1312,13 +1312,18 @@ int dump_one_reg_file(int lfd, u32 id, const struct fd_parms *p)
>>   		goto ext;
>>   	}
>>   
>> -	nsid = lookup_nsid_by_mnt_id(p->mnt_id);
>> -	if (nsid == NULL) {
>> +	mi = lookup_mnt_id(p->mnt_id);
>> +	if (mi == NULL) {
>>   		pr_err("Can't lookup mount=%d for fd=%d path=%s\n",
>>   			p->mnt_id, p->fd, link->name + 1);
>>   		return -1;
>>   	}
>>   
>> +	if (mnt_is_overmounted(mi)) {
>> +		pr_err("Open files on overmounted mounts are not supported yet\n");
>> +		return -1;
>> +	}
>> +
>>   	if (p->mnt_id >= 0 && (root_ns_mask & CLONE_NEWNS)) {
>>   		rfe.mnt_id = p->mnt_id;
>>   		rfe.has_mnt_id = true;
>> @@ -1335,7 +1340,7 @@ int dump_one_reg_file(int lfd, u32 id, const struct fd_parms *p)
>>   		return -1;
>>   	}
>>   
>> -	if (check_path_remap(link, p, lfd, id, nsid))
>> +	if (check_path_remap(link, p, lfd, id, mi->nsid))
>>   		return -1;
>>   	rfe.name	= &link->name[1];
>>   ext:
>> diff --git a/criu/include/mount.h b/criu/include/mount.h
>> index 776c348cd..43cfe788a 100644
>> --- a/criu/include/mount.h
>> +++ b/criu/include/mount.h
>> @@ -129,5 +129,6 @@ extern struct mount_info *parse_mountinfo(pid_t pid, struct ns_id *nsid, bool fo
>>   extern int check_mnt_id(void);
>>   
>>   extern int remount_readonly_mounts(void);
>> +extern int mnt_is_overmounted(struct mount_info *mi);
>>   
>>   #endif /* __CR_MOUNT_H__ */
>> diff --git a/criu/mount.c b/criu/mount.c
>> index 3c19c280b..c28af0fe2 100644
>> --- a/criu/mount.c
>> +++ b/criu/mount.c
>> @@ -1152,7 +1152,7 @@ static int get_clean_fd(struct mount_info *mi)
>>    * root of our mount namespace as it is covered by other mount.
>>    * mnt_is_overmounted() checks if mount is not visible.
>>    */
>> -static int mnt_is_overmounted(struct mount_info *mi)
>> +int mnt_is_overmounted(struct mount_info *mi)
>>   {
>>   	struct mount_info *t, *c, *m = mi;
>>   
>> -- 
>> 2.17.1
>>