[CRIU] [PATCH] crtools: Fix partially opened file set in cr_fdset helpers (v2)

[Cyrill Gorcunov]

The caller expects to obtain NULL in case of any
errors happened but in turn we might return some
garbage instead.

Moreover prep_cr_fdset_for_restore might treat magic
number as file descriptor in case of magic number
mismatch, which is bloody wrong.

Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
---
 crtools.c |   54 +++++++++++++++++++++++++++++-------------------------
 1 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/crtools.c b/crtools.c
index 926d26b..22619a2 100644
--- a/crtools.c
+++ b/crtools.c
@@ -125,17 +125,19 @@ static struct cr_fdset *alloc_cr_fdset(void)
 
 struct cr_fdset *cr_fdset_open(int pid, unsigned long use_mask, struct cr_fdset *cr_fdset)
 {
+	char path[PATH_MAX];
 	unsigned int i;
 	int ret = -1;
-	char path[PATH_MAX];
 
-	if (cr_fdset == NULL) {
+	if (!cr_fdset) {
 		cr_fdset = alloc_cr_fdset();
 		if (!cr_fdset)
 			goto err;
 	}
 
 	for (i = 0; i < CR_FD_MAX; i++) {
+		int fd;
+
 		if (!(use_mask & CR_FD_DESC_USE(i)))
 			continue;
 		if (cr_fdset->fds[i] != -1)
@@ -146,41 +148,40 @@ struct cr_fdset *cr_fdset_open(int pid, unsigned long use_mask, struct cr_fdset
 		if (ret)
 			goto err;
 
-		ret = unlink(path);
-		if (ret && errno != ENOENT) {
-			pr_perror("Unable to unlink %s\n", path);
-			goto err;
-		}
-
-		ret = open(path, O_RDWR | O_CREAT | O_EXCL, CR_FD_PERM);
-		if (ret < 0) {
+		fd = open(path, O_RDWR | O_CREAT | O_EXCL | O_TRUNC, CR_FD_PERM);
+		if (fd < 0) {
 			pr_perror("Unable to open %s\n", path);
 			goto err;
 		}
+		cr_fdset->fds[i] = fd;
 
-		pr_debug("Opened %s with %d\n", path, ret);
-		if (write_img(ret, &fdset_template[i].magic))
+		pr_debug("Opened %s with %d\n", path, fd);
+		if (write_img(fd, &fdset_template[i].magic))
 			goto err;
-
-		cr_fdset->fds[i] = ret;
 	}
-err:
+
 	return cr_fdset;
+
+err:
+	close_cr_fdset(&cr_fdset);
+	return NULL;
 }
 
 struct cr_fdset *prep_cr_fdset_for_restore(int pid, unsigned long use_mask)
 {
+	struct cr_fdset *cr_fdset;
+	char path[PATH_MAX];
 	unsigned int i;
 	int ret = -1;
-	char path[PATH_MAX];
 	u32 magic;
-	struct cr_fdset *cr_fdset;
 
 	cr_fdset = alloc_cr_fdset();
 	if (!cr_fdset)
 		goto err;
 
 	for (i = 0; i < CR_FD_MAX; i++) {
+		int fd;
+
 		if (!(use_mask & CR_FD_DESC_USE(i)))
 			continue;
 
@@ -189,25 +190,28 @@ struct cr_fdset *prep_cr_fdset_for_restore(int pid, unsigned long use_mask)
 		if (ret)
 			goto err;
 
-		ret = open(path, O_RDWR, CR_FD_PERM);
-		if (ret < 0) {
+		fd = open(path, O_RDWR, CR_FD_PERM);
+		if (fd < 0) {
 			pr_perror("Unable to open %s\n", path);
 			goto err;
 		}
+		cr_fdset->fds[i] = fd;
 
-		pr_debug("Opened %s with %d\n", path, ret);
-		if (read_img(ret, &magic) < 0)
+		pr_debug("Opened %s with %d\n", path, fd);
+		if (read_img(fd, &magic) < 0)
 			goto err;
 		if (magic != fdset_template[i].magic) {
-			close(ret);
+			close(fd);
 			pr_err("Magic doesn't match for %s\n", path);
 			goto err;
 		}
-
-		cr_fdset->fds[i] = ret;
 	}
-err:
+
 	return cr_fdset;
+
+err:
+	close_cr_fdset(&cr_fdset);
+	return NULL;
 }
 
 void close_cr_fdset(struct cr_fdset **cr_fdset)
-- 
1.7.7.6