mkdir: fix EEXIST race condition in non-recursive mode - #14181
mkdir: fix EEXIST race condition in non-recursive mode#14181MadeNavaneeth wants to merge 1 commit into
Conversation
Sorry, something went wrong.
| // (i.e., not just a parent reference like "test_dir/..") | ||
| // Directory already exists. Only treat this as success when we | ||
| // are creating parent directories (is_parent) or when -p was | ||
| // given (recursive). In the plain `mkdir dir` case, EEXIST must |
There was a problem hiding this comment.
Why do not you catch EEXIST itself directly at match arm?
Sorry, something went wrong.
|
Good question! I considered matching on the error kind directly, but the current approach catches a broader set of "directory already exists" errors. The call goes through , which may map to different values depending on the platform and Rust version (e.g. on some, on others via the raw OS error). Checking after any creation failure is the same pattern GNU uses — it's resilient to these mapping differences. That said, I'm happy to refactor if there's a preferred approach! |
Sorry, something went wrong.
|
Good question! I considered matching on the error kind directly, but the current approach catches a broader set of "directory already exists" errors. The create_dir_with_mode call goes through std::fs::DirBuilder::create(), which may map EEXIST to different io::ErrorKind values depending on the platform and Rust version. Checking path.is_dir() after any creation failure is the same pattern GNU uses -- it is resilient to these mapping differences. That said, I am happy to refactor if there is a preferred approach! |
Sorry, something went wrong.
|
GNU testsuite comparison: GNU test failed: tests/dd/misc. tests/dd/misc is passing on 'main'. Maybe you have to rebase? GNU test failed: tests/df/over-mount-device. tests/df/over-mount-device is passing on 'main'. Maybe you have to rebase? Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch) Skip an intermittent issue tests/tail/tail-n0f (fails in this run but passes in the 'main' branch) Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch) Congrats! The gnu test tests/cat/splice is no longer failing! Congrats! The gnu test tests/cut/cut is no longer failing! Congrats! The gnu test tests/cut/mb-non-utf8 is no longer failing! Congrats! The gnu test tests/dd/partial-write is no longer failing! Congrats! The gnu test tests/expand/mb is no longer failing! Congrats! The gnu test tests/ls/stat-free-symlinks is no longer failing! Congrats! The gnu test tests/mktemp/write-error is no longer failing! Congrats! The gnu test tests/mv/dir2dir is no longer failing! Congrats! The gnu test tests/mv/mv-exchange is no longer failing! Congrats! The gnu test tests/nl/multibyte is no longer failing! Congrats! The gnu test tests/od/od-float is no longer failing! Congrats! The gnu test tests/od/od-j is no longer failing! Congrats! The gnu test tests/ptx/ptx-overrun is no longer failing! Congrats! The gnu test tests/sort/sort-merge-fdlimit is no longer failing! Congrats! The gnu test tests/unexpand/mb is no longer failing! Note: The gnu test tests/dd/fail-ftruncate-fstat was skipped on 'main' but is now failing. |
Sorry, something went wrong.
When mkdir(2) returns EEXIST, the previous code checked path.is_dir() and returned Ok(()) if true — even for plain `mkdir dir` (non-recursive). This meant two concurrent processes racing to create the same directory could both exit 0, breaking the classic mkdir-as-mutex pattern. Fix by only treating EEXIST + is_dir() as success when creating parent directories (is_parent) or when -p was given (recursive). In the plain `mkdir dir` case, EEXIST now always returns an error, matching GNU behavior. Fixes uutils#13970
Summary
Fixes a race condition where two concurrent mkdir dir invocations could both exit 0, breaking the classic mkdir-as-mutex shell pattern.
Problem
When mkdir(2) returns EEXIST, the previous code checked path.is_dir() and returned Ok(()) if true — even for plain mkdir dir (non-recursive). This meant two concurrent processes racing to create the same directory could both exit 0:
The root cause is a TOCTOU race: after mkdir(2) fails with EEXIST, path.is_dir() returns true because the winning process already created it. The code treated this as success.
Fix
Only treat EEXIST + is_dir() as success when:
In the plain mkdir dir case, EEXIST now always returns an error, matching GNU behavior.
Verification
Fixes #13970