Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Dec 6, 2024

Conversation

cyphar
Copy link
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 [email protected]

@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 <[email protected]>
@cyphar cyphar force-pushed the racing-mkdir-eexist branch from 5d700de to ef8c3b4 Compare December 6, 2024 06:40
Copy link
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
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