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
| // (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?
|
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! |
|
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! |
c486b4c to
fb0b72e
Compare
|
GNU testsuite comparison: |
fb0b72e to
9ee162e
Compare
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
9ee162e to
72d4b7e
Compare
2 participants
Summary
Fixes a race condition where two concurrent
mkdir dirinvocations could both exit 0, breaking the classicmkdir-as-mutex shell pattern.Problem
When
mkdir(2)returnsEEXIST, the previous code checkedpath.is_dir()and returnedOk(())if true — even for plainmkdir 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 withEEXIST,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:is_parent=true(creating parent directories in-pmode), ORconfig.recursive=true(-pwas specified)In the plain
mkdir dircase,EEXISTnow always returns an error, matching GNU behavior.Verification
mkdir -p existing_dirstill succeeds (correct-pbehavior)mkdir existing_dir(without-p) now correctly fails with exit 1Fixes #13970