[CRIU] [PATCH v3] restore_root_task(): fix calling try_clean_remaps

Kir Kolyshkin kir at openvz.org
Wed Oct 14 16:45:36 PDT 2015


1. As pointed out by Coverity (CID 114629), mnt_ns_fd is closed,
but then the function calls try_clean_remaps(mnt_ns_fd)
which tries to close the file descriptor which is already closed.

To address this, let's use safe_close() which sets closed fd to -1.
As it also checks its argument, there's no need for explicit check
so let's remove "if" check before close().

2. As Pavel pointed out, "calling the whole try_clean_remaps()
is not required once we've passed the cleanup_mnt_ns() point".
This could be addressed by introducing yet another label, but
it's cleaner to just use a flag variable.

Note that since the second issue is being addressed, the first one
goes away, but let's keep the fix for it anyway, it might help in
the future.

Signed-off-by: Kir Kolyshkin <kir at openvz.org>
---
 cr-restore.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/cr-restore.c b/cr-restore.c
index 4a835af..2c812cd 100644
--- a/cr-restore.c
+++ b/cr-restore.c
@@ -1736,6 +1736,7 @@ static int restore_root_task(struct pstree_item *init)
 {
 	enum trace_flags flag = TRACE_ALL;
 	int ret, fd, mnt_ns_fd = -1;
+	int clean_remaps = 1;
 
 	ret = run_scripts(ACT_PRE_RESTORE);
 	if (ret != 0) {
@@ -1854,12 +1855,12 @@ static int restore_root_task(struct pstree_item *init)
 	 */
 	task_entries->nr_threads -= atomic_read(&task_entries->nr_zombies);
 
-	if (mnt_ns_fd >= 0)
-		/*
-		 * Don't try_clean_remaps here, since restore went OK
-		 * and all ghosts were removed by the openers.
-		 */
-		close(mnt_ns_fd);
+	/*
+	 * There is no need to call try_clean_remaps() after this point,
+	 * as restore went OK and all ghosts were removed by the openers.
+	 */
+	clean_remaps = 0;
+	close_safe(&mnt_ns_fd);
 	cleanup_mnt_ns();
 
 	ret = stop_usernsd();
@@ -1952,7 +1953,8 @@ out_kill:
 
 out:
 	fini_cgroup();
-	try_clean_remaps(mnt_ns_fd);
+	if (clean_remaps)
+		try_clean_remaps(mnt_ns_fd);
 	cleanup_mnt_ns();
 	stop_usernsd();
 	__restore_switch_stage(CR_STATE_FAIL);
-- 
2.4.3



More information about the CRIU mailing list