[CRIU] [PATCH 2/2] files: try to change fsuid only if linkat() failed

Andrey Vagin avagin at openvz.org
Tue Apr 19 00:42:05 PDT 2016


From: Andrew Vagin <avagin at virtuozzo.com>

We found that linkat for "unsafe" files doesn't work in userns
if a file uid isn't equal to the currect fsuid. This issue was
fixed by changing fsuid before calling linkat. But in this
case we are not able to createa link if a target directory doesn't
have write premissions.

Starting with the 4.3 kernel, it's possible to create links of
"unsafe files":

f2ca379642d7 ("namei: permit linking with CAP_FOWNER in userns")

So we can try to call linkat() without changing fsuid and make one
more attempt with changing fsuid if the first one failed with EPERM.

https://jira.sw.ru/browse/PSBM-46201
Signed-off-by: Andrew Vagin <avagin at virtuozzo.com>
---
 criu/files-reg.c | 70 ++++++++++++++++++++++++++++++--------------------------
 1 file changed, 37 insertions(+), 33 deletions(-)

diff --git a/criu/files-reg.c b/criu/files-reg.c
index c6c7235..0fc2598 100644
--- a/criu/files-reg.c
+++ b/criu/files-reg.c
@@ -1206,49 +1206,53 @@ static int linkat_hard(int odir, char *opath, int ndir, char *npath, uid_t owner
 	int ret, old_fsuid = -1;
 	int errno_save;
 
-	if (root_ns_mask & CLONE_NEWUSER)
-		/*
-		 * Kernel has strange secutiry restrictions about
-		 * linkat. If the fsuid of the caller doesn't equals
-		 * the uid of the file and the file is not "safe"
-		 * one, then only global CAP_CHOWN will be allowed
-		 * to link().
-		 *
-		 * Next, when we're in user namespace we're ns root,
-		 * but not global CAP_CHOWN. Thus, even though we
-		 * ARE ns root, we will not be allowed to link() at
-		 * files that belong to regular users %)
-		 *
-		 * Fortunately, the setfsuid() requires ns-level
-		 * CAP_SETUID which we have.
-		 */
+	ret = linkat(odir, opath, ndir, npath, 0);
+	if (ret < 0)
+		pr_perror("Can't link %s -> %s", opath, npath);
+	if (ret == 0 || errno != EPERM || !(root_ns_mask & CLONE_NEWUSER))
+		return ret;
+
+	/*
+	 * Kernel before 4.3 has strange secutiry restrictions about
+	 * linkat. If the fsuid of the caller doesn't equals
+	 * the uid of the file and the file is not "safe"
+	 * one, then only global CAP_CHOWN will be allowed
+	 * to link().
+	 *
+	 * Next, when we're in user namespace we're ns root,
+	 * but not global CAP_CHOWN. Thus, even though we
+	 * ARE ns root, we will not be allowed to link() at
+	 * files that belong to regular users %)
+	 *
+	 * Fortunately, the setfsuid() requires ns-level
+	 * CAP_SETUID which we have.
+	 */
 
-		old_fsuid = setfsuid(owner);
+	old_fsuid = setfsuid(owner);
 
 	ret = linkat(odir, opath, ndir, npath, 0);
 	errno_save = errno;
 	if (ret < 0)
 		pr_perror("Can't link %s -> %s", opath, npath);
 
-	if (root_ns_mask & CLONE_NEWUSER) {
-		setfsuid(old_fsuid);
-		if (setfsuid(-1) != old_fsuid) {
-			pr_warn("Failed to restore old fsuid!\n");
-			/*
-			 * Don't fail here. We still have chances to run till
-			 * the pie/restorer, and if _this_ guy fails to set
-			 * the proper fsuid, then we'll abort the restore.
-			 */
-		}
-
+	setfsuid(old_fsuid);
+	if (setfsuid(-1) != old_fsuid) {
+		pr_warn("Failed to restore old fsuid!\n");
 		/*
-		 * Restoring PR_SET_DUMPABLE flag is required after setfsuid,
-		 * as if it not set, proc inode will be created with root cred
-		 * (see proc_pid_make_inode), which will result in permission
-		 * check fail when trying to access files in /proc/self/
+		 * Don't fail here. We still have chances to run till
+		 * the pie/restorer, and if _this_ guy fails to set
+		 * the proper fsuid, then we'll abort the restore.
 		 */
-		prctl(PR_SET_DUMPABLE, 1, 0);
 	}
+
+	/*
+	 * Restoring PR_SET_DUMPABLE flag is required after setfsuid,
+	 * as if it not set, proc inode will be created with root cred
+	 * (see proc_pid_make_inode), which will result in permission
+	 * check fail when trying to access files in /proc/self/
+	 */
+	prctl(PR_SET_DUMPABLE, 1, 0);
+
 	errno = errno_save;
 
 	return ret;
-- 
2.5.0



More information about the CRIU mailing list