From d14d851c73c1fecbf361db94c48156e02d61c4e8 Mon Sep 17 00:00:00 2001
From: Djalal Harouni <tixxdz@opendz.org>
Date: Sun, 6 Nov 2016 22:51:49 +0100
Subject: [PATCH] core: make RootDirectory= and ProtectKernelModules= work
Instead of having two fields inside BindMount struct where one is stack
based and the other one is heap, use one field to store the full path
and updated it when we chase symlinks. This way we avoid dealing with
both at the same time.
This makes RootDirectory= work with ProtectHome= and ProtectKernelModules=yes
Fixes: https://github.com/systemd/systemd/issues/4567
(cherry picked from commit f0a4feb0a5318e52107b0df63997a9b13f5be668)
---
src/core/namespace.c | 142 +++++++++++++++++++++++++++++++--------------------
1 file changed, 86 insertions(+), 56 deletions(-)
diff --git a/src/core/namespace.c b/src/core/namespace.c
index 1195e9a854..c45c120193 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -58,8 +58,7 @@ typedef enum MountMode {
} MountMode;
typedef struct BindMount {
- const char *path; /* stack memory, doesn't need to be freed explicitly */
- char *chased; /* malloc()ed memory, needs to be freed */
+ char *path;
MountMode mode;
bool ignore; /* Ignore if path does not exist */
} BindMount;
@@ -155,12 +154,27 @@ static const TargetMount protect_system_strict_table[] = {
{ "/root", READWRITE, true }, /* ProtectHome= */
};
-static void set_bind_mount(BindMount **p, const char *path, MountMode mode, bool ignore) {
+static void set_bind_mount(BindMount **p, char *path, MountMode mode, bool ignore) {
(*p)->path = path;
(*p)->mode = mode;
(*p)->ignore = ignore;
}
+static int append_one_mount(BindMount **p, const char *root_directory,
+ const char *path, MountMode mode, bool ignore) {
+ char *lpath;
+ assert(p);
+
+ lpath = prefix_root(root_directory, path);
+ if (!lpath)
+ return -ENOMEM;
+
+ set_bind_mount(p, lpath, mode, ignore);
+ (*p)++;
+
+ return 0;
+}
+
static int append_mounts(BindMount **p, char **strv, MountMode mode) {
char **i;
@@ -168,6 +182,7 @@ static int append_mounts(BindMount **p, char **strv, MountMode mode) {
STRV_FOREACH(i, strv) {
bool ignore = false;
+ char *path;
if (IN_SET(mode, INACCESSIBLE, READONLY, READWRITE) && startswith(*i, "-")) {
(*i)++;
@@ -177,7 +192,11 @@ static int append_mounts(BindMount **p, char **strv, MountMode mode) {
if (!path_is_absolute(*i))
return -EINVAL;
- set_bind_mount(p, *i, mode, ignore);
+ path = strdup(*i);
+ if (!path)
+ return -ENOMEM;
+
+ set_bind_mount(p, path, mode, ignore);
(*p)++;
}
@@ -196,7 +215,11 @@ static int append_target_mounts(BindMount **p, const char *root_directory, const
* declaration we do not support "-" at the beginning.
*/
const TargetMount *m = &mounts[i];
- const char *path = prefix_roota(root_directory, m->path);
+ char *path;
+
+ path = prefix_root(root_directory, m->path);
+ if (!path)
+ return -ENOMEM;
if (!path_is_absolute(path))
return -EINVAL;
@@ -309,6 +332,7 @@ static void drop_duplicates(BindMount *m, unsigned *n) {
* above. */
if (previous && path_equal(f->path, previous->path)) {
log_debug("%s is duplicate.", f->path);
+ f->path = mfree(f->path);
continue;
}
@@ -336,6 +360,7 @@ static void drop_inaccessible(BindMount *m, unsigned *n) {
* it, as inaccessible paths really should drop the entire subtree. */
if (clear && path_startswith(f->path, clear)) {
log_debug("%s is masked by %s.", f->path, clear);
+ f->path = mfree(f->path);
continue;
}
@@ -375,6 +400,7 @@ static void drop_nop(BindMount *m, unsigned *n) {
/* We found it, let's see if it's the same mode, if so, we can drop this entry */
if (found && p->mode == f->mode) {
log_debug("%s is redundant by %s", f->path, p->path);
+ f->path = mfree(f->path);
continue;
}
}
@@ -401,6 +427,7 @@ static void drop_outside_root(const char *root_directory, BindMount *m, unsigned
if (!path_startswith(f->path, root_directory)) {
log_debug("%s is outside of root directory.", f->path);
+ f->path = mfree(f->path);
continue;
}
@@ -652,18 +679,21 @@ static int chase_all_symlinks(const char *root_directory, BindMount *m, unsigned
* can't resolve the path, and which have been marked for such removal. */
for (f = m, t = m; f < m+*n; f++) {
-
- r = chase_symlinks(f->path, root_directory, &f->chased);
- if (r == -ENOENT && f->ignore) /* Doesn't exist? Then remove it! */
+ _cleanup_free_ char *chased = NULL;
+ r = chase_symlinks(f->path, root_directory, &chased);
+ if (r == -ENOENT && f->ignore) {
+ /* Doesn't exist? Then remove it! */
+ f->path = mfree(f->path);
continue;
+ }
if (r < 0)
return log_debug_errno(r, "Failed to chase symlinks for %s: %m", f->path);
- if (path_equal(f->path, f->chased))
- f->chased = mfree(f->chased);
- else {
- log_debug("Chased %s → %s", f->path, f->chased);
- f->path = f->chased;
+ if (!path_equal(f->path, chased)) {
+ log_debug("Chased %s → %s", f->path, chased);
+ r = free_and_strdup(&f->path, chased);
+ if (r < 0)
+ return r;
}
*t = *f;
@@ -724,96 +754,96 @@ int setup_namespace(
BindMount *m, *mounts = NULL;
bool make_slave = false;
- unsigned n;
+ unsigned n_mounts;
int r = 0;
if (mount_flags == 0)
mount_flags = MS_SHARED;
- n = namespace_calculate_mounts(ns_info,
- read_write_paths,
- read_only_paths,
- inaccessible_paths,
- tmp_dir, var_tmp_dir,
- protect_home, protect_system);
+ n_mounts = namespace_calculate_mounts(ns_info,
+ read_write_paths,
+ read_only_paths,
+ inaccessible_paths,
+ tmp_dir, var_tmp_dir,
+ protect_home, protect_system);
/* Set mount slave mode */
- if (root_directory || n > 0)
+ if (root_directory || n_mounts > 0)
make_slave = true;
- if (n > 0) {
- m = mounts = (BindMount *) alloca0(n * sizeof(BindMount));
+ if (n_mounts > 0) {
+ m = mounts = (BindMount *) alloca0(n_mounts * sizeof(BindMount));
r = append_mounts(&m, read_write_paths, READWRITE);
if (r < 0)
- return r;
+ goto finish;
r = append_mounts(&m, read_only_paths, READONLY);
if (r < 0)
- return r;
+ goto finish;
r = append_mounts(&m, inaccessible_paths, INACCESSIBLE);
if (r < 0)
- return r;
+ goto finish;
if (tmp_dir) {
- m->path = prefix_roota(root_directory, "/tmp");
- m->mode = PRIVATE_TMP;
- m++;
+ r = append_one_mount(&m, root_directory, "/tmp", PRIVATE_TMP, false);
+ if (r < 0)
+ goto finish;
}
if (var_tmp_dir) {
- m->path = prefix_roota(root_directory, "/var/tmp");
- m->mode = PRIVATE_VAR_TMP;
- m++;
+ r = append_one_mount(&m, root_directory, "/var/tmp", PRIVATE_VAR_TMP, false);
+ if (r < 0)
+ goto finish;
}
if (ns_info->private_dev) {
- m->path = prefix_roota(root_directory, "/dev");
- m->mode = PRIVATE_DEV;
- m++;
+ r = append_one_mount(&m, root_directory, "/dev", PRIVATE_DEV, false);
+ if (r < 0)
+ goto finish;
}
if (ns_info->protect_kernel_tunables) {
r = append_protect_kernel_tunables(&m, root_directory);
if (r < 0)
- return r;
+ goto finish;
}
if (ns_info->protect_kernel_modules) {
r = append_protect_kernel_modules(&m, root_directory);
if (r < 0)
- return r;
+ goto finish;
}
if (ns_info->protect_control_groups) {
- m->path = prefix_roota(root_directory, "/sys/fs/cgroup");
- m->mode = READONLY;
- m++;
+ r = append_one_mount(&m, root_directory, "/sys/fs/cgroup", READONLY, false);
+ if (r < 0)
+ goto finish;
}
r = append_protect_home(&m, root_directory, protect_home);
if (r < 0)
- return r;
+ goto finish;
r = append_protect_system(&m, root_directory, protect_system);
if (r < 0)
- return r;
+ goto finish;
- assert(mounts + n == m);
+ assert(mounts + n_mounts == m);
/* Resolve symlinks manually first, as mount() will always follow them relative to the host's
* root. Moreover we want to suppress duplicates based on the resolved paths. This of course is a bit
* racy. */
- r = chase_all_symlinks(root_directory, mounts, &n);
+ r = chase_all_symlinks(root_directory, mounts, &n_mounts);
if (r < 0)
goto finish;
- qsort(mounts, n, sizeof(BindMount), mount_path_compare);
+ qsort(mounts, n_mounts, sizeof(BindMount), mount_path_compare);
- drop_duplicates(mounts, &n);
- drop_outside_root(root_directory, mounts, &n);
- drop_inaccessible(mounts, &n);
- drop_nop(mounts, &n);
+ drop_duplicates(mounts, &n_mounts);
+ drop_outside_root(root_directory, mounts, &n_mounts);
+ drop_inaccessible(mounts, &n_mounts);
+ drop_nop(mounts, &n_mounts);
}
if (unshare(CLONE_NEWNS) < 0) {
@@ -843,25 +873,25 @@ int setup_namespace(
}
}
- if (n > 0) {
+ if (n_mounts > 0) {
char **blacklist;
unsigned j;
/* First round, add in all special mounts we need */
- for (m = mounts; m < mounts + n; ++m) {
+ for (m = mounts; m < mounts + n_mounts; ++m) {
r = apply_mount(m, tmp_dir, var_tmp_dir);
if (r < 0)
goto finish;
}
/* Create a blacklist we can pass to bind_mount_recursive() */
- blacklist = newa(char*, n+1);
- for (j = 0; j < n; j++)
+ blacklist = newa(char*, n_mounts+1);
+ for (j = 0; j < n_mounts; j++)
blacklist[j] = (char*) mounts[j].path;
blacklist[j] = NULL;
/* Second round, flip the ro bits if necessary. */
- for (m = mounts; m < mounts + n; ++m) {
+ for (m = mounts; m < mounts + n_mounts; ++m) {
r = make_read_only(m, blacklist);
if (r < 0)
goto finish;
@@ -886,8 +916,8 @@ int setup_namespace(
r = 0;
finish:
- for (m = mounts; m < mounts + n; m++)
- free(m->chased);
+ for (m = mounts; m < mounts + n_mounts; m++)
+ free(m->path);
return r;
}