open: make OpenInRoot errors match a simple openat2 #17
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cyphar
force-pushed
the
fix-openinroot-enotdir
branch
3 times, most recently
from
0b5d3f0
to
98ebf77
Compare
It makes more sense to make the open("/proc") unsafe fallback more like
a hasNewMountApi() failure.
Signed-off-by: Aleksa Sarai <[email protected]>
renameat2(fd, ".", ...) is not allowed, and so our rename-swap tests where we swap the root itself would silently do nothing (this explains why the racing tests would always succeed). The tests still pass, so our logic was correct, we just didn't exercise that particular check properly. Fixes: ac32743 ("tests: add racing tests for partialLokupInRoot and MkdirAllHandle") Signed-off-by: Aleksa Sarai <[email protected]>
When switching away from O_PATH, we forgot to close the O_PATH handle when replacing it with the non-O_DIRECTORY handle. Fixes: ebb9f1f ("mkdirall: switch away from O_PATH for mkdir loop") Signed-off-by: Aleksa Sarai <[email protected]>
Because we use partialOpenInRoot as the backend implementation of OpenInRoot (which didn't return error information when a partial lookup succeeded), we would map all non-complete errors as ENOENT. This meant that for non-directories you didn't get ENOTDIR, which is what you'd get from a basic openat2(RESOLVE_IN_ROOT) using the path. While we could map the error in OpenInRoot to -ENOTDIR in simple cases, in dangling symlink cases OpenInRoot doesn't know what source error stopped the iteration. So we have to change the partialOpenInRoot API to return an error when a partial open is done, and all of the callers need to be updated to handle that. Since partialOpenInRoot is an internal API, this slightly unconventional interface (where a non-nil error is paired with actual value information) is not that bad. This also lets us remove a bit of duplication from partialOpenInRoot when handling a non-directory component, which I think makes things much nicer. Signed-off-by: Aleksa Sarai <[email protected]>
For openat2 this means we can just one-shot the lookup (making our lookups faster) and for partialLookupInRoot we can not bother with the symlink stack (and simplify the error handling case when doing a complete lookup). Signed-off-by: Aleksa Sarai <[email protected]>
cyphar
force-pushed
the
fix-openinroot-enotdir
branch
from
98ebf77
to
1f4688a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Because we use partialOpenInRoot as the backend implementation of
OpenInRoot (which didn't return error information when a partial lookup
succeeded), we would map all non-complete errors as ENOENT. This meant
that for non-directories you didn't get ENOTDIR, which is what you'd
get from a basic openat2(RESOLVE_IN_ROOT) using the path.
While we could map the error in OpenInRoot to -ENOTDIR in simple cases,
in dangling symlink cases OpenInRoot doesn't know what source error
stopped the iteration. So we have to change the partialOpenInRoot API to
return an error when a partial open is done, and all of the callers need
to be updated to handle that. Since partialOpenInRoot is an internal
API, this slightly unconventional interface (where a non-nil error is
paired with actual value information) is not that bad.
This also lets us remove a bit of duplication from partialOpenInRoot
when handling a non-directory component, which I think makes things much
nicer.
Signed-off-by: Aleksa Sarai [email protected]