Skip to content

mkdir: do not error out with -EEXIST for racing MkdirAlls#35

Merged
cyphar merged 3 commits intomainfrom
racing-mkdir-eexist
Dec 6, 2024
Merged

mkdir: do not error out with -EEXIST for racing MkdirAlls#35
cyphar merged 3 commits intomainfrom
racing-mkdir-eexist

Conversation

@cyphar
Copy link
Copy Markdown
Owner

@cyphar cyphar commented Dec 6, 2024

If two programs are doing MkdirAll, 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
spurrious errors. All we care about is that we open a directory.

Ref: opencontainers/runc#4543
Ref: moby/buildkit#5566
Signed-off-by: Aleksa Sarai cyphar@cyphar.com

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
@cyphar cyphar force-pushed the racing-mkdir-eexist branch 2 times, most recently from a89da1f to 5d700de Compare December 6, 2024 05:16
@cyphar cyphar changed the title mkdir: do not error out with -EEXIST for rancing MkdirAlls mkdir: do not error out with -EEXIST for racing MkdirAlls Dec 6, 2024
If two programs are doing MkdirAll, 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>
@cyphar cyphar force-pushed the racing-mkdir-eexist branch from 5d700de to ef8c3b4 Compare December 6, 2024 06:40
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post-merge LGTM(nb)

@cyphar
Copy link
Copy Markdown
Owner Author

cyphar commented Dec 7, 2024

Sorry, probably should've waited for a review before merging + releasing. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants