pid1 namespacing fixes #8708
Merged
pid1 namespacing fixes #8708
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
keszybz
reviewed
Apr 18, 2018
src/core/namespace.c
Outdated
| int r; | ||
|
|
||
| assert(m); | ||
| /* Let's chase symlinks, but only one step at a time. That's because depending where the symlink points we | ||
| * might to change the order in which we mount stuff. Hence: let's normalize piecemeal, and do one step at a |
src/core/namespace.c
Outdated
| if (r == -ENOENT && m->ignore) { | ||
| log_debug_errno(r, "Path %s does not exist, ignoring.", path); | ||
| return 0; | ||
| if (m->n_followed > CHASE_SYMLINKS_MAX) { /* But a boundary on things */ |
There was a problem hiding this comment.
But → Put ?
I think this should be >=, because we'll do at least another symlink chase.
If the flag is set only a single step of the normalization is executed, and the resulting path is returned. This allows callers to normalize piecemeal, taking into account every single intermediary path of the normalization.
Before this patch we'd resolve all symlinks of bind mounts and other mount points to establish for a service in advance, and only then start mounting them. This is problematic, if symlink chains jump around between directories in a namespace tree, so that to resolve a specific symlink chain we need to establish another mount already. A typical case where this happens is if /etc/resolv.conf is a symlink to some file in /run: in that case we'd normally resolve and mount /etc/resolv.conf early on, but that's broken, as to do this properly we'd need to resolve /etc/resolv.conf first, then figure out that /run needs to be mounted before we can proceed, and thus reorder the order in which we apply mounts dynamically. With this change, whenever we are about to apply a mount, we'll do a single step of the symlink normalization process, patch the mount entry accordingly, and then sort the list of mounts to establish again, taking the new path into account. This means that we can correctly deal with the example above: we might start with wanting to mount /etc/resolv.conf early, but after resolving it to the path in /run/ we'd push it to the end of the list, ensuring that /run is mounted first. (Note that this also fixes another bug: we were following symlinks on the bind mount source relative to the root directory of the service, rather than of the host. That's wrong though as we explicitly document tha the source of bind mounts is always on the host.)
poettering
force-pushed
the
namespace-repeat
branch
from
90ddca9
to
088696f
Compare
|
OK, force pushed a new version. Only changes: the three suggested fixes Thanks for the review! |
keszybz
added
the
good-to-merge/waiting-for-ci 👍
|
poettering
added
ci-failure-appears-unrelated
and removed
good-to-merge/waiting-for-ci 👍
|
Mergin as CI failure appears unrelated |
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.
This primarily contains fixes for the per-service namespacing code, regarding following for symlinks on the files involved.
(This came our of my portable services work, but fixes bugs independently of that, and hence can be reviewed and merged earlier, in more digestable steps)