Root: do not error out with EEXIST for racing Root::mkdir_all()s

If two programs are doing Root::mkdir_all, the previous logic would
return an error if a directory already existed once we got into the
"mkdir" portion of the creation.

Since we already have to accept that an attacker can swap the inode with
a different directory, returning -EEXIST from mkdirat(2) just causes
spurious errors. All we care about is that we open a directory.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

CHANGELOG.md

@@ -56,6 +56,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   it would return `-ELOOP` in most cases and in other cases it would return
   unexpected results because the `O_NOFOLLOW` would have an effect on the
   magic-link used internally by `Handle::reopen`.
+- `Root::mkdir_all` will no longer return `-EEXIST` if another process tried to
+  do `Root::mkdir_all` at the same time, instead the race winner's directory
+  will be used by both processes. See [opencontainers/runc#4543][] for more
+  details.
 
 ### Changed ###
 - syscalls: switch to rustix for most of our syscall wrappers to simplify how
@@ -68,6 +72,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 [rustix#1186]: https://github.com/bytecodealliance/rustix/issues/1186
 [rustix#1187]: https://github.com/bytecodealliance/rustix/issues/1187
+[opencontainers/runc#4543]: https://github.com/opencontainers/runc/issues/4543
 
 ## [0.1.3] - 2024-10-10 ##
 

src/root.rs

@@ -39,7 +39,10 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use rustix::fs::{self as rustix_fs, AtFlags};
+use rustix::{
+    fs::{self as rustix_fs, AtFlags},
+    io::Errno,
+};
 
 /// An inode type to be created with [`Root::create`].
 #[derive(Clone, Debug)]
@@ -1005,12 +1008,17 @@ impl RootRef<'_> {
             // a dangling symlink with only a trailing component missing), so we
             // can safely create the final component without worrying about
             // symlink-exchange attacks.
-            syscalls::mkdirat(&current, &part, perm.mode()).map_err(|err| {
-                ErrorImpl::RawOsError {
-                    operation: "create next directory component".into(),
-                    source: err,
+            if let Err(err) = syscalls::mkdirat(&current, &part, perm.mode()) {
+                // If we got EEXIST then it's possible a racing Root::mkdir_all
+                // created the directory before us. We can safely continue
+                // because the following openat() will
+                if err.errno() != Errno::EXIST {
+                    Err(ErrorImpl::RawOsError {
+                        operation: "create next directory component".into(),
+                        source: err,
+                    })?;
                 }
-            })?;
+            }
 
             // Get a handle to the directory we just created. Unfortunately we
             // can't do an atomic create+open (a-la O_CREAT) with mkdirat(), so

src/syscalls.rs

@@ -254,9 +254,9 @@ pub(crate) enum Error {
 }
 
 impl Error {
-    fn errno(&self) -> &Errno {
+    pub(crate) fn errno(&self) -> Errno {
         // XXX: This should probably be a macro...
-        match self {
+        *match self {
             Error::InvalidFd { source, .. } => source,
             Error::Openat { source, .. } => source,
             Error::Openat2 { source, .. } => source,

src/tests/test_root_ops.rs

@@ -401,14 +401,14 @@ root_op_tests! {
     nondir_symlink_dotdot: mkdir_all("b-file/../d", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOTDIR)));
     nondir_symlink_subdir: mkdir_all("b-file/subdir", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOTDIR)));
     // Dangling symlinks are not followed.
-    dangling1_trailing: mkdir_all("a-fake1", 0o711) => Err(ErrorKind::OsError(Some(libc::EEXIST)));
-    dangling1_basic: mkdir_all("a-fake1/foo", 0o711) => Err(ErrorKind::OsError(Some(libc::EEXIST)));
+    dangling1_trailing: mkdir_all("a-fake1", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOTDIR)));
+    dangling1_basic: mkdir_all("a-fake1/foo", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOTDIR)));
     dangling1_dotdot: mkdir_all("a-fake1/../bar/baz", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOENT)));
-    dangling2_trailing: mkdir_all("a-fake2", 0o711) => Err(ErrorKind::OsError(Some(libc::EEXIST)));
-    dangling2_basic: mkdir_all("a-fake2/foo", 0o711) => Err(ErrorKind::OsError(Some(libc::EEXIST)));
+    dangling2_trailing: mkdir_all("a-fake2", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOTDIR)));
+    dangling2_basic: mkdir_all("a-fake2/foo", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOTDIR)));
     dangling2_dotdot: mkdir_all("a-fake2/../bar/baz", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOENT)));
-    dangling3_trailing: mkdir_all("a-fake3", 0o711) => Err(ErrorKind::OsError(Some(libc::EEXIST)));
-    dangling3_basic: mkdir_all("a-fake3/foo", 0o711) => Err(ErrorKind::OsError(Some(libc::EEXIST)));
+    dangling3_trailing: mkdir_all("a-fake3", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOTDIR)));
+    dangling3_basic: mkdir_all("a-fake3/foo", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOTDIR)));
     dangling3_dotdot: mkdir_all("a-fake3/../bar/baz", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOENT)));
     // Non-lexical symlinks should work.
     nonlexical_basic: mkdir_all("target/foo", 0o711) => Ok(());
@@ -423,11 +423,11 @@ root_op_tests! {
     nonlexical_level3_abs: mkdir_all("link3/target_abs/foo", 0o711) => Ok(());
     nonlexical_level3_rel: mkdir_all("link3/target_rel/foo", 0o711) => Ok(());
     // But really tricky dangling symlinks should fail.
-    dangling_tricky1_trailing: mkdir_all("link3/deep_dangling1", 0o711) => Err(ErrorKind::OsError(Some(libc::EEXIST)));
-    dangling_tricky1_basic: mkdir_all("link3/deep_dangling1/foo", 0o711) => Err(ErrorKind::OsError(Some(libc::EEXIST)));
+    dangling_tricky1_trailing: mkdir_all("link3/deep_dangling1", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOTDIR)));
+    dangling_tricky1_basic: mkdir_all("link3/deep_dangling1/foo", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOTDIR)));
     dangling_tricky1_dotdot: mkdir_all("link3/deep_dangling1/../bar", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOENT)));
-    dangling_tricky2_trailing: mkdir_all("link3/deep_dangling2", 0o711) => Err(ErrorKind::OsError(Some(libc::EEXIST)));
-    dangling_tricky2_basic: mkdir_all("link3/deep_dangling2/foo", 0o711) => Err(ErrorKind::OsError(Some(libc::EEXIST)));
+    dangling_tricky2_trailing: mkdir_all("link3/deep_dangling2", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOTDIR)));
+    dangling_tricky2_basic: mkdir_all("link3/deep_dangling2/foo", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOTDIR)));
     dangling_tricky2_dotdot: mkdir_all("link3/deep_dangling2/../bar", 0o711) => Err(ErrorKind::OsError(Some(libc::ENOENT)));
     // And trying to mkdir inside a loop should fail.
     loop_trailing: mkdir_all("loop/link", 0o711) => Err(ErrorKind::OsError(Some(libc::ELOOP)));