diff --git a/CHANGELOG.md b/CHANGELOG.md index 04b5685..fa6a7de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] ## +### Fixed ### +- `MkdirAll` will now no longer return an `EEXIST` error if two racing + processes are creating the same directory. We will still verify that the path + is a directory, but this will avoid spurious errors when multiple threads or + programs are trying to `MkdirAll` the same path. opencontainers/runc#4543 ## [0.3.4] - 2024-10-09 ## diff --git a/mkdir_linux.go b/mkdir_linux.go index b5f6745..6dfe8c4 100644 --- a/mkdir_linux.go +++ b/mkdir_linux.go @@ -119,7 +119,12 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err // NOTE: mkdir(2) will not follow trailing symlinks, so we can safely // create the final component without worrying about symlink-exchange // attacks. - if err := unix.Mkdirat(int(currentDir.Fd()), part, uint32(mode)); err != nil { + // + // If we get -EEXIST, it's possible that another program created the + // directory at the same time as us. In that case, just continue on as + // if we created it (if the created inode is not a directory, the + // following open call will fail). + if err := unix.Mkdirat(int(currentDir.Fd()), part, uint32(mode)); err != nil && !errors.Is(err, unix.EEXIST) { err = &os.PathError{Op: "mkdirat", Path: currentDir.Name() + "/" + part, Err: err} // Make the error a bit nicer if the directory is dead. if err2 := isDeadInode(currentDir); err2 != nil { diff --git a/mkdir_linux_test.go b/mkdir_linux_test.go index 9499057..140562c 100644 --- a/mkdir_linux_test.go +++ b/mkdir_linux_test.go @@ -149,14 +149,14 @@ func testMkdirAll_Basic(t *testing.T, mkdirAll mkdirAllFunc) { "nondir-symlink-dotdot": {unsafePath: "b-file/../d", expectedErr: unix.ENOTDIR}, "nondir-symlink-subdir": {unsafePath: "b-file/subdir", expectedErr: unix.ENOTDIR}, // Dangling symlinks are not followed. - "dangling1-trailing": {unsafePath: "a-fake1", expectedErr: unix.EEXIST}, - "dangling1-basic": {unsafePath: "a-fake1/foo", expectedErr: unix.EEXIST}, + "dangling1-trailing": {unsafePath: "a-fake1", expectedErr: unix.ENOTDIR}, + "dangling1-basic": {unsafePath: "a-fake1/foo", expectedErr: unix.ENOTDIR}, "dangling1-dotdot": {unsafePath: "a-fake1/../bar/baz", expectedErr: unix.ENOENT}, - "dangling2-trailing": {unsafePath: "a-fake2", expectedErr: unix.EEXIST}, - "dangling2-basic": {unsafePath: "a-fake2/foo", expectedErr: unix.EEXIST}, + "dangling2-trailing": {unsafePath: "a-fake2", expectedErr: unix.ENOTDIR}, + "dangling2-basic": {unsafePath: "a-fake2/foo", expectedErr: unix.ENOTDIR}, "dangling2-dotdot": {unsafePath: "a-fake2/../bar/baz", expectedErr: unix.ENOENT}, - "dangling3-trailing": {unsafePath: "a-fake3", expectedErr: unix.EEXIST}, - "dangling3-basic": {unsafePath: "a-fake3/foo", expectedErr: unix.EEXIST}, + "dangling3-trailing": {unsafePath: "a-fake3", expectedErr: unix.ENOTDIR}, + "dangling3-basic": {unsafePath: "a-fake3/foo", expectedErr: unix.ENOTDIR}, "dangling3-dotdot": {unsafePath: "a-fake3/../bar/baz", expectedErr: unix.ENOENT}, // Non-lexical symlinks should work. "nonlexical-basic": {unsafePath: "target/foo"}, @@ -171,11 +171,11 @@ func testMkdirAll_Basic(t *testing.T, mkdirAll mkdirAllFunc) { "nonlexical-level3-abs": {unsafePath: "link3/target_abs/foo"}, "nonlexical-level3-rel": {unsafePath: "link3/target_rel/foo"}, // But really tricky dangling symlinks should fail. - "dangling-tricky1-trailing": {unsafePath: "link3/deep_dangling1", expectedErr: unix.EEXIST}, - "dangling-tricky1-basic": {unsafePath: "link3/deep_dangling1/foo", expectedErr: unix.EEXIST}, + "dangling-tricky1-trailing": {unsafePath: "link3/deep_dangling1", expectedErr: unix.ENOTDIR}, + "dangling-tricky1-basic": {unsafePath: "link3/deep_dangling1/foo", expectedErr: unix.ENOTDIR}, "dangling-tricky1-dotdot": {unsafePath: "link3/deep_dangling1/../bar", expectedErr: unix.ENOENT}, - "dangling-tricky2-trailing": {unsafePath: "link3/deep_dangling2", expectedErr: unix.EEXIST}, - "dangling-tricky2-basic": {unsafePath: "link3/deep_dangling2/foo", expectedErr: unix.EEXIST}, + "dangling-tricky2-trailing": {unsafePath: "link3/deep_dangling2", expectedErr: unix.ENOTDIR}, + "dangling-tricky2-basic": {unsafePath: "link3/deep_dangling2/foo", expectedErr: unix.ENOTDIR}, "dangling-tricky2-dotdot": {unsafePath: "link3/deep_dangling2/../bar", expectedErr: unix.ENOENT}, // And trying to mkdir inside a loop should fail. "loop-trailing": {unsafePath: "loop/link", expectedErr: unix.ELOOP}, @@ -348,7 +348,7 @@ func TestMkdirAllHandle_RacingRename(t *testing.T) { {"trailing", "target/a/b/c/d/e", "swapdir-nonempty1", "target/a/b/c/d/e", nil}, {"partial", "target/a/b/c/d/e", "swapdir-nonempty1", "target/a/b/c/d/e/f/g/h/i/j/k", nil}, {"trailing", "target/a/b/c/d/e", "swapdir-nonempty2", "target/a/b/c/d/e", nil}, - {"partial", "target/a/b/c/d/e", "swapdir-nonempty2", "target/a/b/c/d/e/f/g/h/i/j/k", []error{unix.EEXIST}}, + {"partial", "target/a/b/c/d/e", "swapdir-nonempty2", "target/a/b/c/d/e/f/g/h/i/j/k", []error{unix.ENOTDIR}}, {"trailing", "target/a/b/c/d/e", "swapfile", "target/a/b/c/d/e", []error{unix.ENOTDIR}}, {"partial", "target/a/b/c/d/e", "swapfile", "target/a/b/c/d/e/f/g/h/i/j/k", []error{unix.ENOTDIR}}, }