libct/nsenter: fix extra runc re-exec on tmpfs

kolyshkin · kolyshkin · commit 18e286261ec6 · 2022-01-27T08:42:11.000-08:00
After adding some debug info to cloned_binary.c I found out that
is_self_cloned() is not working right when runc binary is on tmpfs,
resulting in one extra re-exec of runc.

With some added debug:

	$ mkdir bin
	$ sudo mount -t tmpfs tmp bin
	$ sudo cp runc bin
	$ sudo ./bin/runc --debug exec xxx true
	DEBU[0000] nsexec[763590]: =&gt; is_self_cloned
	DEBU[0000] nsexec[763590]: got seals 1 (want 15)
	DEBU[0000] nsexec[763590]: &lt;= is_self_cloned, is_cloned = 0
	DEBU[0000] nsexec[763590]: try_bindfd: 5
	DEBU[0000] nsexec[763590]: re-exec itself...
	DEBU[0000] nsexec[763590]: =&gt; is_self_cloned
	DEBU[0000] nsexec[763590]: got seals 1 (want 15)
	DEBU[0000] nsexec[763590]: &lt;= is_self_cloned, is_cloned = 0
	DEBU[0000] nsexec[763590]: try_bindfd: -1
	DEBU[0000] nsexec[763590]: fallback to make_execfd: 5
	DEBU[0000] nsexec[763590]: re-exec itself...
	DEBU[0000] nsexec[763590]: =&gt; is_self_cloned
	DEBU[0000] nsexec[763590]: got seals 15 (want 15)
	DEBU[0000] nsexec[763590]: &lt;= is_self_cloned, is_cloned = 1

From the above, it is seen that
 - `is_self_cloned` returns 0,
 - `try_bindfd` is called and succeeds,
 - runc re-execs itself,
 - the second call to `is_self_cloned` returns 0 again (because GET_SEALS returns 1),
 - runc falls back to `make_execfd`, and re-execs again,
 - finally, the third `is_self_cloned` returns 1.

I guess that the code relied on the following (quoting fcntl(2)):

&gt; Currently, file seals can be applied only to a file descriptor
&gt; returned by memfd_create(2) (if the MFD_ALLOW_SEALING was employed).
&gt; On other filesystems, all fcntl() operations that operate on seals
&gt; will return EINVAL.

It looks like in case of a file on tmpfs it returns 1 (F_SEAL_SEAL).

With the fix:

	DEBU[0000] nsexec[768367]: =&gt; is_self_cloned
	DEBU[0000] nsexec[768367]: got seals 1 (want 15)
	DEBU[0000] nsexec[768367]: no CLONED_BINARY_ENV
	DEBU[0000] nsexec[768367]: &lt;= is_self_cloned, is_cloned = 0
	DEBU[0000] nsexec[768367]: try_bindfd: 5
	DEBU[0000] nsexec[768367]: re-exec itself...
	DEBU[0000] nsexec[768367]: =&gt; is_self_cloned
	DEBU[0000] nsexec[768367]: got seals 1 (want 15)
	DEBU[0000] nsexec[768367]: fstatfs says ro = 1
	DEBU[0000] nsexec[768367]: fstat says nlink = 1
	DEBU[0000] nsexec[768367]: &lt;= is_self_cloned, is_cloned = 1

Signed-off-by: Kir Kolyshkin &lt;kolyshkin@gmail.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  * When Intel RDT feature is not available, its initialization is skipped,
    resulting in slightly faster `runc exec` and `runc run`. (#3306)
 
+### Fixed
+
+ * In case the runc binary resides on tmpfs, `runc init` no longer re-execs
+   itself twice. (#3342)
+
 ## [1.1.0] - 2022-01-14
 
 > A plan depends as much upon execution as it does upon concept.
diff --git a/libcontainer/nsenter/cloned_binary.c b/libcontainer/nsenter/cloned_binary.c
@@ -137,7 +137,7 @@ static void *must_realloc(void *ptr, size_t size)
  */
 static int is_self_cloned(void)
 {
-	int fd, ret, is_cloned = 0;
+	int fd, is_cloned = 0;
 	struct stat statbuf = { };
 	struct statfs fsbuf = { };
 
@@ -153,11 +153,9 @@ static int is_self_cloned(void)
 	 * sharing it isn't a bad thing -- and an admin could bind-mount a sealed
 	 * memfd to /usr/bin/runc to allow re-use).
 	 */
-	ret = fcntl(fd, F_GET_SEALS);
-	if (ret >= 0) {
-		is_cloned = (ret == RUNC_MEMFD_SEALS);
+	is_cloned = (fcntl(fd, F_GET_SEALS) == RUNC_MEMFD_SEALS);
+	if (is_cloned)
 		goto out;
-	}
 
 	/*
 	 * All other forms require CLONED_BINARY_ENV, since they are potentially
