[CRIU] [PATCH] restore_root_task(): don't clean remaps if mnt_ns_fd is closed

Kir Kolyshkin kir at openvz.org
Tue Oct 13 11:21:49 PDT 2015


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.

Also, as pointed out by Pavel, "calling the whole try_clean_remaps()
is not required once we've passed the cleanup_mnt_ns() point".

So, let's just set mnt_ns_fd to -1 after closing it, and
return from try_clean_remaps() if the argument is negative.

Signed-off-by: Kir Kolyshkin <kir at openvz.org>
---
 cr-restore.c |  4 +++-
 files-reg.c  | 28 ++++++++++++++--------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/cr-restore.c b/cr-restore.c
index 5d1b501..6677edf 100644
--- a/cr-restore.c
+++ b/cr-restore.c
@@ -1854,12 +1854,14 @@ static int restore_root_task(struct pstree_item *init)
 	 */
 	task_entries->nr_threads -= atomic_read(&task_entries->nr_zombies);
 
-	if (mnt_ns_fd >= 0)
+	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);
+		mnt_ns_fd = -1;
+	}
 	cleanup_mnt_ns();
 
 	ret = stop_usernsd();
diff --git a/files-reg.c b/files-reg.c
index 59d88ca..82e4742 100644
--- a/files-reg.c
+++ b/files-reg.c
@@ -423,23 +423,24 @@ void try_clean_remaps(int ns_fd)
 	struct remap_info *ri;
 	int old_ns = -1;
 
+	if (ns_fd < 0)
+		return;
+
 	if (list_empty(&remaps))
 		goto out;
 
-	if (ns_fd >= 0) {
-		pr_info("Switching to new ns to clean ghosts\n");
+	pr_info("Switching to new ns to clean ghosts\n");
 
-		old_ns = open_proc(PROC_SELF, "ns/mnt");
-		if (old_ns < 0) {
-			pr_perror("`- Can't keep old ns");
-			return;
-		}
+	old_ns = open_proc(PROC_SELF, "ns/mnt");
+	if (old_ns < 0) {
+		pr_perror("`- Can't keep old ns");
+		return;
+	}
 
-		if (setns(ns_fd, CLONE_NEWNS) < 0) {
-			close(old_ns);
-			pr_perror("`- Can't switch");
-			return;
-		}
+	if (setns(ns_fd, CLONE_NEWNS) < 0) {
+		close(old_ns);
+		pr_perror("`- Can't switch");
+		return;
 	}
 
 	list_for_each_entry(ri, &remaps, list)
@@ -453,8 +454,7 @@ void try_clean_remaps(int ns_fd)
 	}
 
 out:
-	if (ns_fd >= 0)
-		close(ns_fd);
+	close(ns_fd);
 }
 
 static struct collect_image_info remap_cinfo = {
-- 
2.4.3



More information about the CRIU mailing list