Blob Blame History Raw
From 2894e242562f3c639b6cb63f502d7445076d3db1 Mon Sep 17 00:00:00 2001
From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
Date: Thu, 13 Feb 2020 13:03:12 +0300
Subject: [PATCH 056/120] files-reg: fix error handling of rm_parent_dirs

If unlinkat fails it means that fs is in "corrupted" state - spoiled
with non-unlinked auxiliary directories.

While on it add fixme note as this function can be racy and BUG_ON if
path contains double slashes.

Cherry-picked from Virtuozzo criu:
https://src.openvz.org/projects/OVZ/repos/criu/commits/b7b4e69fd

Changes: simplify while loop condition, remove confusing FIXME, remove
excess !count check in favour of while loop condition check

Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 criu/files-reg.c | 45 ++++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/criu/files-reg.c b/criu/files-reg.c
index 4560f253e..0e126a32e 100644
--- a/criu/files-reg.c
+++ b/criu/files-reg.c
@@ -1792,30 +1792,42 @@ out:
 	return ret;
 }
 
-static void rm_parent_dirs(int mntns_root, char *path, int count)
+static int rm_parent_dirs(int mntns_root, char *path, int count)
 {
 	char *p, *prev = NULL;
+	int ret = -1;
 
-	if (!count)
-		return;
-
-	while (count > 0) {
-		count -= 1;
+	while (count-- > 0) {
 		p = strrchr(path, '/');
-		if (p)
+		if (p) {
+			/* We don't handle "//" in path */
+			BUG_ON(prev && (prev - p == 1));
 			*p = '\0';
+		} else {
+			/* Inconsistent path and count */
+			pr_perror("Can't strrchr \"/\" in \"%s\"/\"%s\"]"
+				  " left count=%d\n",
+				  path, prev ? prev + 1 : "", count + 1);
+			goto err;
+		}
+
 		if (prev)
 			*prev = '/';
+		prev = p;
 
-		if (unlinkat(mntns_root, path, AT_REMOVEDIR))
+		if (unlinkat(mntns_root, path, AT_REMOVEDIR)) {
 			pr_perror("Can't remove %s AT %d", path, mntns_root);
-		else
-			pr_debug("Unlinked parent dir: %s AT %d\n", path, mntns_root);
-		prev = p;
+			goto err;
+		}
+		pr_debug("Unlinked parent dir: %s AT %d\n", path, mntns_root);
 	}
 
+	ret = 0;
+err:
 	if (prev)
 		*prev = '/';
+
+	return ret;
 }
 
 /* Construct parent dir name and mkdir parent/grandparents if they're not exist */
@@ -1847,6 +1859,7 @@ static int make_parent_dirs_if_need(int mntns_root, char *path)
 		err = mkdirat(mntns_root, path, 0777);
 		if (err && errno != EEXIST) {
 			pr_perror("Can't create dir: %s AT %d", path, mntns_root);
+			/* Failing anyway -> no retcode check */
 			rm_parent_dirs(mntns_root, path, count);
 			count = -1;
 			goto out;
@@ -1933,10 +1946,11 @@ out_root:
 
 	if (linkat_hard(mntns_root, rpath, mntns_root, path, rfi->remap->uid, rfi->remap->gid, 0) < 0) {
 		int errno_saved = errno;
-		rm_parent_dirs(mntns_root, path, *level);
-		errno = errno_saved;
-		if (errno == EEXIST)
+
+		if (!rm_parent_dirs(mntns_root, path, *level) && errno_saved == EEXIST) {
+			errno = errno_saved;
 			return 1;
+		}
 		return -1;
 	}
 
@@ -2124,7 +2138,8 @@ ext:
 				pr_perror("Failed to unlink the remap file");
 				goto err;
 			}
-			rm_parent_dirs(mntns_root, rfi->path, level);
+			if (rm_parent_dirs(mntns_root, rfi->path, level))
+				goto err;
 		}
 
 		mutex_unlock(remap_open_lock);
-- 
2.34.1