mount: Fix cleanup_mnt_ns() when opts.root is set

Submitted by Radostin Stoyanov on Jan. 11, 2019, 12:23 p.m.

Details

Message ID 20190111122352.29395-1-rstoyanov1@gmail.com
State Accepted
Series "mount: Fix cleanup_mnt_ns() when opts.root is set"
Headers show

Commit Message

Radostin Stoyanov Jan. 11, 2019, 12:23 p.m.
In commit

    c1404f66712e7bacd7cbc7cef1bb85c7c6ad698f
    mount: restore cwd after creating a roots yard (v2)

were introduced changes to the function cleanup_mnt_ns() that handle
the case when opts.root is set.

However, after commit

    2e8970beda5bfb7d56fd3351ae57d8dd1c69b2c7
    mount: create a mount point for the root mount namespace in the roots yard

the cleanup_mnt_ns() function always results in error:

    mnt: Can't remove the directory ...: No such file or directory

when restore fails and opts.root has been set.

Resolves #467

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
---
 criu/mount.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/mount.c b/criu/mount.c
index f578c7b6a..55c264a9c 100644
--- a/criu/mount.c
+++ b/criu/mount.c
@@ -3263,13 +3263,7 @@  int depopulate_roots_yard(int mntns_fd, bool only_ghosts)
 
 void cleanup_mnt_ns(void)
 {
-	char path[PATH_MAX], *root = opts.root ? : "/";
-
-	if (mnt_roots == NULL)
-		return;
-
-	snprintf(path, sizeof(path), "%s/%s", root, mnt_roots);
-	if (rmdir(path))
+	if (rmdir(mnt_roots))
 		pr_perror("Can't remove the directory %s", mnt_roots);
 }
 

Comments

Andrey Vagin Jan. 13, 2019, 5:30 a.m.
Applied, thanks. Good catch!

On Fri, Jan 11, 2019 at 12:23:52PM +0000, Radostin Stoyanov wrote:
> In commit
> 
>     c1404f66712e7bacd7cbc7cef1bb85c7c6ad698f
>     mount: restore cwd after creating a roots yard (v2)
> 
> were introduced changes to the function cleanup_mnt_ns() that handle
> the case when opts.root is set.
> 
> However, after commit
> 
>     2e8970beda5bfb7d56fd3351ae57d8dd1c69b2c7
>     mount: create a mount point for the root mount namespace in the roots yard
> 
> the cleanup_mnt_ns() function always results in error:
> 
>     mnt: Can't remove the directory ...: No such file or directory
> 
> when restore fails and opts.root has been set.
> 
> Resolves #467
> 
> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
> ---
>  criu/mount.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/criu/mount.c b/criu/mount.c
> index f578c7b6a..55c264a9c 100644
> --- a/criu/mount.c
> +++ b/criu/mount.c
> @@ -3263,13 +3263,7 @@ int depopulate_roots_yard(int mntns_fd, bool only_ghosts)
>  
>  void cleanup_mnt_ns(void)
>  {
> -	char path[PATH_MAX], *root = opts.root ? : "/";
> -
> -	if (mnt_roots == NULL)
> -		return;
> -
> -	snprintf(path, sizeof(path), "%s/%s", root, mnt_roots);
> -	if (rmdir(path))
> +	if (rmdir(mnt_roots))
>  		pr_perror("Can't remove the directory %s", mnt_roots);
>  }
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Andrey Vagin Jan. 13, 2019, 5:56 a.m.
On Fri, Jan 11, 2019 at 12:23:52PM +0000, Radostin Stoyanov wrote:
> In commit
> 
>     c1404f66712e7bacd7cbc7cef1bb85c7c6ad698f
>     mount: restore cwd after creating a roots yard (v2)
> 
> were introduced changes to the function cleanup_mnt_ns() that handle
> the case when opts.root is set.
> 
> However, after commit
> 
>     2e8970beda5bfb7d56fd3351ae57d8dd1c69b2c7
>     mount: create a mount point for the root mount namespace in the roots yard
> 
> the cleanup_mnt_ns() function always results in error:
> 
>     mnt: Can't remove the directory ...: No such file or directory
> 
> when restore fails and opts.root has been set.
> 
> Resolves #467
> 
> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
> ---
>  criu/mount.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/criu/mount.c b/criu/mount.c
> index f578c7b6a..55c264a9c 100644
> --- a/criu/mount.c
> +++ b/criu/mount.c
> @@ -3263,13 +3263,7 @@ int depopulate_roots_yard(int mntns_fd, bool only_ghosts)
>  
>  void cleanup_mnt_ns(void)
>  {
> -	char path[PATH_MAX], *root = opts.root ? : "/";
> -
> -	if (mnt_roots == NULL)
> -		return;

we still need to check mnt_roots == NULL

[root@fc24 criu]# python test/zdtm.py run -t zdtm/static/env00 -f h
=== Run 1/1 ================ zdtm/static/env00
========================== Run zdtm/static/env00 in h ==========================
Start test
./env00 --pidfile=env00.pid --outfile=env00.out --envname=ENV_00_TEST
Run criu dump
Run criu restore
=[log]=> dump/zdtm/static/env00/36/1/restore.log
------------------------ grep Error ------------------------
(00.005509) pie: 36: vdso: Remap rt-vvar 0x29000 -> 0x7ffe91dd3000
(00.005555) pie: 36: Restoring scheduler params 0.0.0
(00.005589) pie: 36: 36: Restored
(00.005690) Running post-restore scripts
(00.005722) Error (criu/mount.c:3267): mnt: Can't remove the directory (null): Bad address
------------------------ ERROR OVER ------------------------
Send the 15 signal to  36
Wait for zdtm/static/env00(36) to die for 0.100000
Removing dump/zdtm/static/env00/36
========================= Test zdtm/static/env00 PASS ==========================


> -
> -	snprintf(path, sizeof(path), "%s/%s", root, mnt_roots);
> -	if (rmdir(path))
> +	if (rmdir(mnt_roots))
>  		pr_perror("Can't remove the directory %s", mnt_roots);
>  }
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Radostin Stoyanov Jan. 13, 2019, 8:45 a.m.
On 13/01/2019 05:56, Andrei Vagin wrote:
> On Fri, Jan 11, 2019 at 12:23:52PM +0000, Radostin Stoyanov wrote:
>> In commit
>>
>>     c1404f66712e7bacd7cbc7cef1bb85c7c6ad698f
>>     mount: restore cwd after creating a roots yard (v2)
>>
>> were introduced changes to the function cleanup_mnt_ns() that handle
>> the case when opts.root is set.
>>
>> However, after commit
>>
>>     2e8970beda5bfb7d56fd3351ae57d8dd1c69b2c7
>>     mount: create a mount point for the root mount namespace in the roots yard
>>
>> the cleanup_mnt_ns() function always results in error:
>>
>>     mnt: Can't remove the directory ...: No such file or directory
>>
>> when restore fails and opts.root has been set.
>>
>> Resolves #467
>>
>> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
>> ---
>>  criu/mount.c | 8 +-------
>>  1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/criu/mount.c b/criu/mount.c
>> index f578c7b6a..55c264a9c 100644
>> --- a/criu/mount.c
>> +++ b/criu/mount.c
>> @@ -3263,13 +3263,7 @@ int depopulate_roots_yard(int mntns_fd, bool only_ghosts)
>>  
>>  void cleanup_mnt_ns(void)
>>  {
>> -	char path[PATH_MAX], *root = opts.root ? : "/";
>> -
>> -	if (mnt_roots == NULL)
>> -		return;
> we still need to check mnt_roots == NULL
Sorry, I've missed that. Thank you for fixing it!
> [root@fc24 criu]# python test/zdtm.py run -t zdtm/static/env00 -f h
> === Run 1/1 ================ zdtm/static/env00
> ========================== Run zdtm/static/env00 in h ==========================
> Start test
> ./env00 --pidfile=env00.pid --outfile=env00.out --envname=ENV_00_TEST
> Run criu dump
> Run criu restore
> =[log]=> dump/zdtm/static/env00/36/1/restore.log
> ------------------------ grep Error ------------------------
> (00.005509) pie: 36: vdso: Remap rt-vvar 0x29000 -> 0x7ffe91dd3000
> (00.005555) pie: 36: Restoring scheduler params 0.0.0
> (00.005589) pie: 36: 36: Restored
> (00.005690) Running post-restore scripts
> (00.005722) Error (criu/mount.c:3267): mnt: Can't remove the directory (null): Bad address
> ------------------------ ERROR OVER ------------------------
> Send the 15 signal to  36
> Wait for zdtm/static/env00(36) to die for 0.100000
> Removing dump/zdtm/static/env00/36
> ========================= Test zdtm/static/env00 PASS ==========================
>
>
>> -
>> -	snprintf(path, sizeof(path), "%s/%s", root, mnt_roots);
>> -	if (rmdir(path))
>> +	if (rmdir(mnt_roots))
>>  		pr_perror("Can't remove the directory %s", mnt_roots);
>>  }
>>  
>> -- 
>> 2.20.1
>>
>> _______________________________________________
>> CRIU mailing list
>> CRIU@openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu