Skip to content

Return EISDIR when trying to create a path ending in / with open #23135

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

Merged
merged 10 commits into from
Dec 18, 2024

Conversation

hoodmane
Copy link
Collaborator

If we open a path that in a / and we're about to create a file, instead return EISDIR.

If we open a path that in a / and we're about to create a file, instead return EISDIR.
Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

lgtm % comments

@hoodmane hoodmane merged commit d9d60a1 into emscripten-core:main Dec 18, 2024
29 checks passed
@hoodmane hoodmane deleted the eisdir branch December 18, 2024 21:57
@dschuff
Copy link
Member

dschuff commented Dec 19, 2024

It looks like this test is failing with ASan:
https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8728149317242328273/+/u/Emscripten_testsuite__ASan_/stdout
(and SAFE_HEAP, I imagine it's the same issue: https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8728149317242328273/+/u/Emscripten_testsuite__SAFE_HEAP_/stdout)

It looks like there's a null deref in open(). It may very well be pre-existing, but just exposed by this test.

@hoodmane
Copy link
Collaborator Author

I'll look into it (unless someone else fixes it first).

@hoodmane
Copy link
Collaborator Author

hoodmane commented Dec 19, 2024

This is enough to reproduce the asan failure:

#include <fcntl.h>

int main() {
  open("./does-not-exist/", O_CREAT);
}

@sbc100
Copy link
Collaborator

sbc100 commented Dec 19, 2024

This is enough to reproduce the asan failure:

#include <fcntl.h>

int main() {
  open("./does-not-exist/", O_CREAT);
}

Can you share the command line flags needed?

@sbc100
Copy link
Collaborator

sbc100 commented Dec 19, 2024

Oh thats because if you specify O_CREAT you also need to specify that mode argument. If you don't the open syscall will read the mode from uninitialized memory.

open is actually variadic. So specifying O_CREAT with a mode is like using print without enough argments.

@sbc100
Copy link
Collaborator

sbc100 commented Dec 19, 2024

From https://man7.org/linux/man-pages/man2/open.2.html:

The mode argument must be supplied if O_CREAT or O_TMPFILE is
specified in flags; if it is not supplied, some arbitrary
bytes from the stack will be applied as the file mode.

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.

3 participants