Skip to content

Commit 96d7d96

Browse files
committed
9p: MkdirAll - partially fix Mkdir race condition
If 2 threads call `MkdirAll` at the same time, they can both fail the `Walk` check. 1 thread will succeed when calling `Mkdir`, and the other will fail with `EEXIST`. This is a classic mistake. I think this is a result of the initial implementation not having access to the error code constants. When this was resolved (by exporting them so they may be compared against), the logic was not updated to account for this erroneous sequence. This change inverts the order of operations, and simply ignores `EEXIST` in the first call to `Mkdir. There's still a chance the directory can be manipulated between the call to `Mkdir` and `Walk`, and this change does not account for nor resolve that. The POSIX spec does not currently define a standard way to atomically create and open a directory in the same way `creat` works (which extends into the 9P2000.L standard we're conforming to). In the future we may decide to account for this problem using some extension to the file system protocol, and/or by utilizing file system locks. Again POSIX does not define locking semantics in regard to directories as it does for regular files.
1 parent 569d161 commit 96d7d96

File tree

1 file changed

+5
-11
lines changed

1 file changed

+5
-11
lines changed

internal/filesystem/9p/operations.go

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ func MkdirAll(root p9.File, names []string,
164164
}
165165
for _, name := range names {
166166
var (
167-
next, err = getOrMkdir(current, name, permissions, uid, gid)
167+
next, err = mkdirAndWalk(current, name, permissions, uid, gid)
168168
cErr = current.Close()
169169
)
170170
if err != nil {
@@ -178,19 +178,13 @@ func MkdirAll(root p9.File, names []string,
178178
return current, nil
179179
}
180180

181-
func getOrMkdir(fsys p9.File, name string, permissions p9.FileMode, uid p9.UID, gid p9.GID) (p9.File, error) {
181+
func mkdirAndWalk(fsys p9.File, name string, permissions p9.FileMode, uid p9.UID, gid p9.GID) (p9.File, error) {
182182
wnames := []string{name}
183-
_, dir, err := fsys.Walk(wnames)
184-
if err == nil {
185-
return dir, nil
186-
}
187-
if !errors.Is(err, perrors.ENOENT) {
183+
if _, err := fsys.Mkdir(name, permissions, uid, gid); err != nil &&
184+
!errors.Is(err, perrors.EEXIST) {
188185
return nil, err
189186
}
190-
if _, err = fsys.Mkdir(name, permissions, uid, gid); err != nil {
191-
return nil, err
192-
}
193-
_, dir, err = fsys.Walk(wnames)
187+
_, dir, err := fsys.Walk(wnames)
194188
return dir, err
195189
}
196190

0 commit comments

Comments
 (0)