SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #12005 from poettering/exec-dir-fixup

Browse Source 
Some fixes to exec directory management
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2019-03-20 09:48:45 +01:00
committed by GitHub
parent 17b70256f2 08f6769675
commit bd504f6a22
2 changed files with 98 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/basic/fs-util.c
+62 -17
View File
@@ -215,64 +215,109 @@ int readlink_and_make_absolute(const char *p, char **r) {
int chmod_and_chown(const char *path, mode_t mode, uid_t uid, gid_t gid) {
char fd_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int) + 1];
_cleanup_close_ int fd = -1;
bool st_valid = false;
struct stat st;
int r;
assert(path);
/* Under the assumption that we are running privileged we first change the access mode and only then hand out
* ownership to avoid a window where access is too open. */
/* Under the assumption that we are running privileged we first change the access mode and only then
* hand out ownership to avoid a window where access is too open. */
fd = open(path, O_PATH|O_CLOEXEC|O_NOFOLLOW); /* Let's acquire an O_PATH fd, as precaution to change mode/owner
* on the same file */
fd = open(path, O_PATH|O_CLOEXEC|O_NOFOLLOW); /* Let's acquire an O_PATH fd, as precaution to change
* mode/owner on the same file */
if (fd < 0)
return -errno;
xsprintf(fd_path, "/proc/self/fd/%i", fd);
if (mode != MODE_INVALID) {
if ((mode & S_IFMT) != 0) {
struct stat st;
if (stat(fd_path, &st) < 0)
return -errno;
if ((mode & S_IFMT) != (st.st_mode & S_IFMT))
return -EINVAL;
st_valid = true;
}
if (chmod(fd_path, mode & 07777) < 0)
return -errno;
if (chmod(fd_path, mode & 07777) < 0) {
r = -errno;
if (!st_valid && stat(fd_path, &st) < 0)
return -errno;
if ((mode & 07777) != (st.st_mode & 07777))
return r;
st_valid = true;
}
}
if (uid != UID_INVALID || gid != GID_INVALID)
if (chown(fd_path, uid, gid) < 0)
return -errno;
if (uid != UID_INVALID || gid != GID_INVALID) {
if (chown(fd_path, uid, gid) < 0) {
r = -errno;
if (!st_valid && stat(fd_path, &st) < 0)
return -errno;
if (uid != UID_INVALID && st.st_uid != uid)
return r;
if (gid != GID_INVALID && st.st_gid != gid)
return r;
}
}
return 0;
}
int fchmod_and_chown(int fd, mode_t mode, uid_t uid, gid_t gid) {
bool st_valid = false;
struct stat st;
int r;
/* Under the assumption that we are running privileged we first change the access mode and only then hand out
* ownership to avoid a window where access is too open. */
if (mode != MODE_INVALID) {
if ((mode & S_IFMT) != 0) {
struct stat st;
if (fstat(fd, &st) < 0)
return -errno;
if ((mode & S_IFMT) != (st.st_mode & S_IFMT))
return -EINVAL;
st_valid = true;
}
if (fchmod(fd, mode & 0777) < 0)
return -errno;
if (fchmod(fd, mode & 07777) < 0) {
r = -errno;
if (!st_valid && fstat(fd, &st) < 0)
return -errno;
if ((mode & 07777) != (st.st_mode & 07777))
return r;
st_valid = true;
}
}
if (uid != UID_INVALID || gid != GID_INVALID)
if (fchown(fd, uid, gid) < 0)
return -errno;
if (fchown(fd, uid, gid) < 0) {
r = -errno;
if (!st_valid && fstat(fd, &st) < 0)
return -errno;
if (uid != UID_INVALID && st.st_uid != uid)
return r;
if (gid != GID_INVALID && st.st_gid != gid)
return r;
}
return 0;
}
src/core/execute.c
+36 -29
View File
@@ -2074,7 +2074,7 @@ static int setup_exec_directory(
STRV_FOREACH(rt, context->directories[type].paths) {
_cleanup_free_ char *p = NULL, *pp = NULL;
p = strjoin(params->prefix[type], "/", *rt);
p = path_join(params->prefix[type], *rt);
if (!p) {
r = -ENOMEM;
goto fail;
@@ -2085,7 +2085,8 @@ static int setup_exec_directory(
goto fail;
if (context->dynamic_user &&
!IN_SET(type, EXEC_DIRECTORY_RUNTIME, EXEC_DIRECTORY_CONFIGURATION)) {
(!IN_SET(type, EXEC_DIRECTORY_RUNTIME, EXEC_DIRECTORY_CONFIGURATION) ||
(type == EXEC_DIRECTORY_RUNTIME && context->runtime_directory_preserve_mode != EXEC_PRESERVE_NO))) {
_cleanup_free_ char *private_root = NULL;
/* So, here's one extra complication when dealing with DynamicUser=1 units. In that case we
@@ -2110,7 +2111,7 @@ static int setup_exec_directory(
* Also, note that we don't do this for EXEC_DIRECTORY_RUNTIME as that's often used for sharing
* files or sockets with other services. */
private_root = strjoin(params->prefix[type], "/private");
private_root = path_join(params->prefix[type], "private");
if (!private_root) {
r = -ENOMEM;
goto fail;
@@ -2121,7 +2122,7 @@ static int setup_exec_directory(
if (r < 0)
goto fail;
pp = strjoin(private_root, "/", *rt);
pp = path_join(private_root, *rt);
if (!pp) {
r = -ENOMEM;
goto fail;
@@ -2156,36 +2157,42 @@ static int setup_exec_directory(
if (r < 0)
goto fail;
/* Lock down the access mode */
if (chmod(pp, context->directories[type].mode) < 0) {
r = -errno;
goto fail;
}
} else {
r = mkdir_label(p, context->directories[type].mode);
if (r < 0 && r != -EEXIST)
goto fail;
if (r == -EEXIST) {
struct stat st;
if (stat(p, &st) < 0) {
r = -errno;
if (r < 0) {
if (r != -EEXIST)
goto fail;
}
if (((st.st_mode ^ context->directories[type].mode) & 07777) != 0)
log_warning("%s \'%s\' already exists but the mode is different. "
"(filesystem: %o %sMode: %o)",
exec_directory_type_to_string(type), *rt,
st.st_mode & 07777, exec_directory_type_to_string(type), context->directories[type].mode & 07777);
if (!context->dynamic_user)
if (type == EXEC_DIRECTORY_CONFIGURATION) {
struct stat st;
/* Don't change the owner/access mode of the configuration directory,
* as in the common case it is not written to by a service, and shall
* not be writable. */
if (stat(p, &st) < 0) {
r = -errno;
goto fail;
}
/* Still complain if the access mode doesn't match */
if (((st.st_mode ^ context->directories[type].mode) & 07777) != 0)
log_warning("%s \'%s\' already exists but the mode is different. "
"(File system: %o %sMode: %o)",
exec_directory_type_to_string(type), *rt,
st.st_mode & 07777, exec_directory_type_to_string(type), context->directories[type].mode & 07777);
continue;
}
}
}
/* Don't change the owner of the configuration directory, as in the common case it is not written to by
* a service, and shall not be writable. */
if (type == EXEC_DIRECTORY_CONFIGURATION)
continue;
/* Lock down the access mode (we use chmod_and_chown() to make this idempotent. We don't
* specifiy UID/GID here, so that path_chown_recursive() can optimize things depending on the
* current UID/GID ownership.) */
r = chmod_and_chown(pp ?: p, context->directories[type].mode, UID_INVALID, GID_INVALID);
if (r < 0)
goto fail;
/* Then, change the ownership of the whole tree, if necessary */
r = path_chown_recursive(pp ?: p, uid, gid);
@@ -3154,7 +3161,7 @@ static int exec_child(
USER_PROCESS,
username);
if (context->user) {
if (uid_is_valid(uid)) {
r = chown_terminal(STDIN_FILENO, uid);
if (r < 0) {
*exit_status = EXIT_STDIN;
@@ -3459,7 +3466,7 @@ static int exec_child(
}
if (needs_setuid) {
if (context->user) {
if (uid_is_valid(uid)) {
r = enforce_user(context, uid);
if (r < 0) {
*exit_status = EXIT_USER;