You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gi...@apache.org on 2017/07/28 19:28:18 UTC
[3/4] mesos git commit: Fixed the sandbox volume relative host path
ownership.
Fixed the sandbox volume relative host path ownership.
This bugfix addresses the issue from MESOS-5178. Basically, the
sandbox volume ownership was not set correctly. This issue can be
exposed if a framework user is non-root while the agent
process runs as root. Then, the non-root user does not have
permissions to write to this volume.
The correct solution should be giving permissions to corresponding
users by leveraging supplementary groups. But we can still
introduce a workaround in this patch by changing the ownership
of this sandbox volume to its sandbox's ownership.
Review: https://reviews.apache.org/r/61122/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b5efb912
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b5efb912
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b5efb912
Branch: refs/heads/master
Commit: b5efb9121e523edf344534ba6c5332d6f7190643
Parents: f99a717
Author: Gilbert Song <so...@gmail.com>
Authored: Fri Jul 28 12:27:55 2017 -0700
Committer: Gilbert Song <so...@gmail.com>
Committed: Fri Jul 28 12:27:55 2017 -0700
----------------------------------------------------------------------
.../mesos/isolators/filesystem/linux.cpp | 37 ++++++++++++++++----
1 file changed, 30 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/b5efb912/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp b/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
index bf35b7f..d7fe9a8 100644
--- a/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
+++ b/src/slave/containerizer/mesos/isolators/filesystem/linux.cpp
@@ -432,6 +432,15 @@ Try<vector<CommandInfo>> LinuxFilesystemIsolatorProcess::getPreExecCommands(
commands.push_back(command);
}
+ // Get the parent sandbox user and group info for the source path.
+ struct stat s;
+ if (::stat(containerConfig.directory().c_str(), &s) < 0) {
+ return ErrnoError("Failed to stat '" + containerConfig.directory() + "'");
+ }
+
+ const uid_t uid = s.st_uid;
+ const gid_t gid = s.st_gid;
+
foreach (const Volume& volume, containerConfig.container_info().volumes()) {
// NOTE: Volumes with source will be handled by the corresponding
// isolators (e.g., docker/volume).
@@ -477,14 +486,28 @@ Try<vector<CommandInfo>> LinuxFilesystemIsolatorProcess::getPreExecCommands(
// work directory because a user can potentially use a container
// path like '../../abc'.
- Try<Nothing> mkdir = os::mkdir(source);
- if (mkdir.isError()) {
- return Error(
- "Failed to create the source of the mount at '" +
- source + "': " + mkdir.error());
- }
+ // NOTE: Chown should be avoided if the source directory already
+ // exists because it may be owned by some other user and should
+ // not be mutated.
+ if (!os::exists(source)) {
+ Try<Nothing> mkdir = os::mkdir(source);
+ if (mkdir.isError()) {
+ return Error(
+ "Failed to create the source of the mount at '" +
+ source + "': " + mkdir.error());
+ }
- // TODO(idownes): Consider setting ownership and mode.
+ LOG(INFO) << "Changing the ownership of the sandbox volume at '"
+ << source << "' with UID " << uid << " and GID " << gid;
+
+ Try<Nothing> chown = os::chown(uid, gid, source, false);
+ if (chown.isError()) {
+ return Error(
+ "Failed to change the ownership of the sandbox volume at '" +
+ source + "' with UID " + stringify(uid) + " and GID " +
+ stringify(gid) + ": " + chown.error());
+ }
+ }
}
// Determine the target of the mount. The mount target