[systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir
Zbigniew Jędrzejewski-Szmek
zbyszek at in.waw.pl
Tue Mar 12 07:59:27 PDT 2013
On Tue, Mar 12, 2013 at 03:14:49PM +0100, Michal Sekletar wrote:
> All Execs within the service, will get mounted the same /tmp and /var/tmp
> directories, if service is configured with PrivateTmp=yes. Temporary
> directories are cleaned up by service itself, rather than relying on
> systemd-tmpfiles.
What exactly does this mean? Since the patch doesn't touch tmpfiles.d, I
think that you mean "in addition to" instead of "rather than".
> Directory which is mounted as inaccessible is shared,
> created at install time.
> ---
> Makefile.am | 2 +
> man/systemd.exec.xml | 4 +-
> src/core/execute.c | 43 +++++++++-
> src/core/execute.h | 10 ++-
> src/core/manager.c | 6 ++
> src/core/manager.h | 4 +-
> src/core/mount.c | 21 ++++-
> src/core/namespace.c | 226 +++++++++++++++++++++------------------------------
> src/core/namespace.h | 17 ++--
> src/core/service.c | 30 ++++++-
> src/core/socket.c | 20 ++++-
> src/core/swap.c | 20 ++++-
> src/shared/util.c | 45 ++++++++++
> src/shared/util.h | 1 +
> src/test/test-ns.c | 14 +++-
> 15 files changed, 310 insertions(+), 153 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index c8f7b8e..95c36d4 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -183,6 +183,7 @@ define move-to-rootlibdir
> endef
>
> INSTALL_DIRS =
> +INACCESSIBLE_DIR = $(prefix)/lib/systemd/inaccessible
>
> RUNLEVEL1_TARGET_WANTS =
> RUNLEVEL2_TARGET_WANTS =
> @@ -225,6 +226,7 @@ endef
>
> install-directories-hook:
> $(MKDIR_P) $(addprefix $(DESTDIR),$(INSTALL_DIRS))
> + $(MKDIR_P) -m 000 $(addprefix $(DESTDIR),$(INACCESSIBLE_DIR))
There's only one word here, so $(DESTDIR)$(INACCESSIBLE_DIR) is fine.
>
> install-aliases-hook:
> set -- $(SYSTEM_UNIT_ALIASES) && \
> diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
> index 9c31baf..b1cd685 100644
> --- a/man/systemd.exec.xml
> +++ b/man/systemd.exec.xml
> @@ -1107,7 +1107,9 @@
> processes via
> <filename>/tmp</filename> or
> <filename>/var/tmp</filename>
> - impossible. Defaults to
> + impossible. All temporary data created
> + by service will be removed after service
> + is stopped. Defaults to
> false.</para></listitem>
> </varlistentry>
>
> diff --git a/src/core/execute.c b/src/core/execute.c
> index 92cf174..18e25fa 100644
> --- a/src/core/execute.c
> +++ b/src/core/execute.c
> @@ -173,6 +173,18 @@ static bool is_terminal_output(ExecOutput o) {
> o == EXEC_OUTPUT_JOURNAL_AND_CONSOLE;
> }
>
> +void exec_context_serialize(const ExecContext *context, Unit *u, FILE *f) {
> + assert(context);
> + assert(u);
> + assert(f);
> +
> + if (context->tmp_dir)
> + unit_serialize_item(u, f, "tmp-dir", context->tmp_dir);
> +
> + if (context->var_tmp_dir)
> + unit_serialize_item(u, f, "var-tmp-dir", context->var_tmp_dir);
> +}
> +
> static int open_null_as(int flags, int nfd) {
> int fd, r;
>
> @@ -968,7 +980,7 @@ static int apply_seccomp(uint32_t *syscall_filter) {
>
> int exec_spawn(ExecCommand *command,
> char **argv,
> - const ExecContext *context,
> + ExecContext *context,
> int fds[], unsigned n_fds,
> char **environment,
> bool apply_permissions,
> @@ -1036,6 +1048,12 @@ int exec_spawn(ExecCommand *command,
>
> cgroup_attribute_apply_list(cgroup_attributes, cgroup_bondings);
>
> + if (context->private_tmp && !context->tmp_dir && !context->var_tmp_dir) {
> + r = setup_tmpdirs(&context->tmp_dir, &context->var_tmp_dir);
> + if (r < 0)
> + return r;
> + }
> +
> pid = fork();
> if (pid < 0)
> return -errno;
> @@ -1302,6 +1320,8 @@ int exec_spawn(ExecCommand *command,
> err = setup_namespace(context->read_write_dirs,
> context->read_only_dirs,
> context->inaccessible_dirs,
> + context->tmp_dir,
> + context->var_tmp_dir,
> context->private_tmp,
> context->mount_flags);
> if (err < 0) {
> @@ -1530,7 +1550,23 @@ void exec_context_init(ExecContext *c) {
> c->timer_slack_nsec = (nsec_t) -1;
> }
>
> -void exec_context_done(ExecContext *c) {
> +void exec_context_tmp_dirs_done(ExecContext *c) {
> + assert(c);
> +
> + if (c->tmp_dir) {
> + rm_rf_dangerous(c->tmp_dir, false, true, false);
> + free(c->tmp_dir);
> + c->tmp_dir = NULL;
> + }
> +
> + if (c->var_tmp_dir) {
> + rm_rf_dangerous(c->var_tmp_dir, false, true, false);
> + free(c->var_tmp_dir);
> + c->var_tmp_dir = NULL;
> + }
> +}
> +
> +void exec_context_done(ExecContext *c, bool reloading_or_reexecuting) {
> unsigned l;
>
> assert(c);
> @@ -1594,6 +1630,9 @@ void exec_context_done(ExecContext *c) {
>
> free(c->syscall_filter);
> c->syscall_filter = NULL;
> +
> + if (!reloading_or_reexecuting)
> + exec_context_tmp_dirs_done(c);
> }
>
> void exec_command_done(ExecCommand *c) {
> diff --git a/src/core/execute.h b/src/core/execute.h
> index 001eb0e..3ce9221 100644
> --- a/src/core/execute.h
> +++ b/src/core/execute.h
> @@ -36,6 +36,8 @@ typedef struct ExecContext ExecContext;
> struct CGroupBonding;
> struct CGroupAttribute;
>
> +typedef struct Unit Unit;
> +
> #include "list.h"
> #include "util.h"
>
> @@ -141,6 +143,8 @@ struct ExecContext {
> bool non_blocking;
> bool private_tmp;
> bool private_network;
> + char *tmp_dir;
> + char *var_tmp_dir;
>
> bool no_new_privileges;
>
> @@ -164,7 +168,7 @@ struct ExecContext {
>
> int exec_spawn(ExecCommand *command,
> char **argv,
> - const ExecContext *context,
> + ExecContext *context,
> int fds[], unsigned n_fds,
> char **environment,
> bool apply_permissions,
> @@ -192,13 +196,15 @@ void exec_command_append_list(ExecCommand **l, ExecCommand *e);
> int exec_command_set(ExecCommand *c, const char *path, ...);
>
> void exec_context_init(ExecContext *c);
> -void exec_context_done(ExecContext *c);
> +void exec_context_done(ExecContext *c, bool reloading_or_reexecuting);
> +void exec_context_tmp_dirs_done(ExecContext *c);
> void exec_context_dump(ExecContext *c, FILE* f, const char *prefix);
> void exec_context_tty_reset(const ExecContext *context);
>
> int exec_context_load_environment(const ExecContext *c, char ***l);
>
> bool exec_context_may_touch_console(ExecContext *c);
> +void exec_context_serialize(const ExecContext *c, Unit *u, FILE *f);
>
> void exec_status_start(ExecStatus *s, pid_t pid);
> void exec_status_exit(ExecStatus *s, ExecContext *context, pid_t pid, int code, int status);
> diff --git a/src/core/manager.c b/src/core/manager.c
> index c261b25..699df5d 100644
> --- a/src/core/manager.c
> +++ b/src/core/manager.c
> @@ -2340,6 +2340,12 @@ static bool manager_is_booting_or_shutting_down(Manager *m) {
> return false;
> }
>
> +bool manager_is_reloading_or_reexecuting(Manager *m) {
> + assert(m);
> +
> + return m->n_reloading != 0;
> +}
> +
> void manager_reset_failed(Manager *m) {
> Unit *u;
> Iterator i;
> diff --git a/src/core/manager.h b/src/core/manager.h
> index c486a16..a19a093 100644
> --- a/src/core/manager.h
> +++ b/src/core/manager.h
> @@ -85,6 +85,7 @@ struct Watch {
> #include "set.h"
> #include "dbus.h"
> #include "path-lookup.h"
> +#include "execute.h"
>
> struct Manager {
> /* Note that the set of units we know of is allowed to be
> @@ -117,7 +118,6 @@ struct Manager {
>
> /* Units to remove */
> LIST_HEAD(Unit, cleanup_queue);
> -
> /* Units to check when doing GC */
> LIST_HEAD(Unit, gc_queue);
>
> @@ -283,6 +283,8 @@ int manager_distribute_fds(Manager *m, FDSet *fds);
>
> int manager_reload(Manager *m);
>
> +bool manager_is_reloading_or_reexecuting(Manager *m);
> +
> void manager_reset_failed(Manager *m);
>
> void manager_send_unit_audit(Manager *m, Unit *u, int type, bool success);
> diff --git a/src/core/mount.c b/src/core/mount.c
> index 419cf27..f33eb18 100644
> --- a/src/core/mount.c
> +++ b/src/core/mount.c
> @@ -25,6 +25,7 @@
> #include <sys/epoll.h>
> #include <signal.h>
>
> +#include "manager.h"
> #include "unit.h"
> #include "mount.h"
> #include "load-fragment.h"
> @@ -126,7 +127,7 @@ static void mount_done(Unit *u) {
> mount_parameters_done(&m->parameters_proc_self_mountinfo);
> mount_parameters_done(&m->parameters_fragment);
>
> - exec_context_done(&m->exec_context);
> + exec_context_done(&m->exec_context, manager_is_reloading_or_reexecuting(u->manager));
> exec_command_done_array(m->exec_command, _MOUNT_EXEC_COMMAND_MAX);
> m->control_command = NULL;
>
> @@ -869,6 +870,7 @@ static void mount_enter_dead(Mount *m, MountResult f) {
> if (f != MOUNT_SUCCESS)
> m->result = f;
>
> + exec_context_tmp_dirs_done(&m->exec_context);
> mount_set_state(m, m->result != MOUNT_SUCCESS ? MOUNT_FAILED : MOUNT_DEAD);
> }
>
> @@ -1162,6 +1164,8 @@ static int mount_serialize(Unit *u, FILE *f, FDSet *fds) {
> if (m->control_command_id >= 0)
> unit_serialize_item(u, f, "control-command", mount_exec_command_to_string(m->control_command_id));
>
> + exec_context_serialize(&m->exec_context, UNIT(m), f);
> +
> return 0;
> }
>
> @@ -1218,7 +1222,22 @@ static int mount_deserialize_item(Unit *u, const char *key, const char *value, F
> m->control_command_id = id;
> m->control_command = m->exec_command + id;
> }
> + } else if (streq(key, "tmp-dir")) {
> + char *t;
> +
> + t = strdup(value);
> + if (!t)
> + return log_oom();
> +
> + m->exec_context.tmp_dir = t;
> + } else if (streq(key, "var-tmp-dir")) {
> + char *t;
> +
> + t = strdup(value);
> + if (!t)
> + return log_oom();
>
> + m->exec_context.var_tmp_dir = t;
> } else
> log_debug_unit(UNIT(m)->id,
> "Unknown serialization key '%s'", key);
> diff --git a/src/core/namespace.c b/src/core/namespace.c
> index ba18ddc..640c455 100644
> --- a/src/core/namespace.c
> +++ b/src/core/namespace.c
> @@ -36,23 +36,26 @@
> #include "path-util.h"
> #include "namespace.h"
> #include "missing.h"
> +#include "execute.h"
>
> -typedef enum PathMode {
> +typedef enum MountMode {
> /* This is ordered by priority! */
> INACCESSIBLE,
> READONLY,
> PRIVATE_TMP,
> PRIVATE_VAR_TMP,
> READWRITE
> -} PathMode;
> +} MountMode;
>
> -typedef struct Path {
> +typedef struct BindMount {
> const char *path;
> - PathMode mode;
> + MountMode mode;
> bool done;
> -} Path;
> +} BindMount;
>
> -static int append_paths(Path **p, char **strv, PathMode mode) {
> +static const char *inaccessible_dir = "/usr/lib/systemd/inaccessible";
Is there any reason for this not to use @INACCESSIBLE_DIR@ instead
of a static variable?
> +
> +static int append_mounts(BindMount **p, char **strv, MountMode mode) {
> char **i;
>
> STRV_FOREACH(i, strv) {
> @@ -68,8 +71,8 @@ static int append_paths(Path **p, char **strv, PathMode mode) {
> return 0;
> }
>
> -static int path_compare(const void *a, const void *b) {
> - const Path *p = a, *q = b;
> +static int mount_path_compare(const void *a, const void *b) {
> + const BindMount *p = a, *q = b;
>
> if (path_equal(p->path, q->path)) {
>
> @@ -93,14 +96,13 @@ static int path_compare(const void *a, const void *b) {
> return 0;
> }
>
> -static void drop_duplicates(Path *p, unsigned *n, bool *need_inaccessible) {
> - Path *f, *t, *previous;
> +static void drop_duplicates(BindMount *m, unsigned *n) {
> + BindMount *f, *t, *previous;
>
> - assert(p);
> + assert(m);
> assert(n);
> - assert(need_inaccessible);
>
> - for (f = p, t = p, previous = NULL; f < p+*n; f++) {
> + for (f = m, t = m, previous = NULL; f < m+*n; f++) {
>
> /* The first one wins */
> if (previous && path_equal(f->path, previous->path))
> @@ -109,29 +111,25 @@ static void drop_duplicates(Path *p, unsigned *n, bool *need_inaccessible) {
> t->path = f->path;
> t->mode = f->mode;
>
> - if (t->mode == INACCESSIBLE)
> - *need_inaccessible = true;
> -
> previous = t;
>
> t++;
> }
>
> - *n = t - p;
> + *n = t - m;
> }
>
> static int apply_mount(
> - Path *p,
> + BindMount *m,
> const char *tmp_dir,
> - const char *var_tmp_dir,
> - const char *inaccessible_dir) {
> + const char *var_tmp_dir) {
>
> const char *what;
> int r;
>
> - assert(p);
> + assert(m);
>
> - switch (p->mode) {
> + switch (m->mode) {
>
> case INACCESSIBLE:
> what = inaccessible_dir;
> @@ -139,7 +137,7 @@ static int apply_mount(
>
> case READONLY:
> case READWRITE:
> - what = p->path;
> + what = m->path;
> break;
>
> case PRIVATE_TMP:
> @@ -156,133 +154,101 @@ static int apply_mount(
>
> assert(what);
>
> - r = mount(what, p->path, NULL, MS_BIND|MS_REC, NULL);
> + r = mount(what, m->path, NULL, MS_BIND|MS_REC, NULL);
> if (r >= 0)
> - log_debug("Successfully mounted %s to %s", what, p->path);
> + log_debug("Successfully mounted %s to %s", what, m->path);
>
> return r;
> }
>
> -static int make_read_only(Path *p) {
> +static int make_read_only(BindMount *m) {
> int r;
>
> - assert(p);
> + assert(m);
>
> - if (p->mode != INACCESSIBLE && p->mode != READONLY)
> + if (m->mode != INACCESSIBLE && m->mode != READONLY)
> return 0;
>
> - r = mount(NULL, p->path, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY|MS_REC, NULL);
> + r = mount(NULL, m->path, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY|MS_REC, NULL);
> if (r < 0)
> return -errno;
>
> return 0;
> }
>
> -int setup_namespace(
> - char **writable,
> - char **readable,
> - char **inaccessible,
> - bool private_tmp,
> - unsigned long flags) {
> -
> - char
> - tmp_dir[] = "/tmp/systemd-private-XXXXXX",
> - var_tmp_dir[] = "/var/tmp/systemd-private-XXXXXX",
> - inaccessible_dir[] = "/tmp/systemd-inaccessible-XXXXXX";
> -
> - Path *paths, *p;
> - unsigned n;
> - bool need_inaccessible = false;
> - bool remove_tmp = false, remove_var_tmp = false, remove_inaccessible = false;
> - int r;
> +int setup_tmpdirs(char **tmp_dir,
> + char **var_tmp_dir) {
> + int r = 0;
> + bool remove_tmp_dir = false;
> + char tmp_dir_template[] = "/tmp/systemd-private-XXXXXX",
> + var_tmp_dir_template[] = "/var/tmp/systemd-private-XXXXXX";
>
> - if (!flags)
> - flags = MS_SHARED;
> + assert(tmp_dir);
> + assert(var_tmp_dir);
>
> - n =
> - strv_length(writable) +
> - strv_length(readable) +
> - strv_length(inaccessible) +
> - (private_tmp ? 2 : 0);
> -
> - p = paths = alloca(sizeof(Path) * n);
> - if ((r = append_paths(&p, writable, READWRITE)) < 0 ||
> - (r = append_paths(&p, readable, READONLY)) < 0 ||
> - (r = append_paths(&p, inaccessible, INACCESSIBLE)) < 0)
> + r = create_tmp_dir(tmp_dir_template, 0000, true, tmp_dir);
> + if (r < 0)
> goto fail;
> + remove_tmp_dir = true;
Why not just use kernel style cleanup with two labels: fail1, fail2. Seems
easier to read than a bool. It also grows to three cases easily, should we
decide to have a third kind or directory later on.
>
> - if (private_tmp) {
> - p->path = "/tmp";
> - p->mode = PRIVATE_TMP;
> - p++;
> + r = create_tmp_dir(var_tmp_dir_template, 0000, true, var_tmp_dir);
> + if (r < 0)
> + goto fail;
>
> - p->path = "/var/tmp";
> - p->mode = PRIVATE_VAR_TMP;
> - p++;
> + return 0;
> +fail:
> + if (remove_tmp_dir) {
> + rmdir(*tmp_dir);
> + free(*tmp_dir);
> + *tmp_dir = NULL;
> }
>
> - assert(paths + n == p);
> -
> - qsort(paths, n, sizeof(Path), path_compare);
> - drop_duplicates(paths, &n, &need_inaccessible);
> + return r;
> +}
>
> - if (need_inaccessible) {
> - mode_t u;
> - char *d;
> +int setup_namespace(char** read_write_dirs,
> + char** read_only_dirs,
> + char** inaccessible_dirs,
> + char* tmp_dir,
> + char* var_tmp_dir,
> + bool private_tmp,
> + unsigned mount_flags) {
>
> - u = umask(0777);
> - d = mkdtemp(inaccessible_dir);
> - umask(u);
> + unsigned n = strv_length(read_write_dirs) +
> + strv_length(read_only_dirs) +
> + strv_length(inaccessible_dirs) +
> + (private_tmp ? 2 : 0);
> + BindMount *m, *mounts;
> + int r = 0;
>
> - if (!d) {
> - r = -errno;
> - goto fail;
> - }
> + if (!mount_flags)
> + mount_flags = MS_SHARED;
>
> - remove_inaccessible = true;
> + if (unshare(CLONE_NEWNS) < 0) {
> + r = -errno;
> + goto fail;
> }
>
> - if (private_tmp) {
> - mode_t u;
> - char *d;
> -
> - u = umask(0000);
> - d = mkdtemp(tmp_dir);
> - umask(u);
> -
> - if (!d) {
> - r = -errno;
> - goto fail;
> - }
> -
> - remove_tmp = true;
> -
> - u = umask(0000);
> - d = mkdtemp(var_tmp_dir);
> - umask(u);
> -
> - if (!d) {
> - r = -errno;
> - goto fail;
> - }
> -
> - remove_var_tmp = true;
> + m = mounts = (BindMount *) alloca(n * sizeof(BindMount));
> + if ((r = append_mounts(&m, read_write_dirs, READWRITE)) < 0 ||
> + (r = append_mounts(&m, read_only_dirs, READONLY)) < 0 ||
> + (r = append_mounts(&m, inaccessible_dirs, INACCESSIBLE)) < 0)
> + goto fail;
>
> - if (chmod(tmp_dir, 0777 + S_ISVTX) < 0) {
> - r = -errno;
> - goto fail;
> - }
> + if (private_tmp) {
> + m->path = "/tmp";
> + m->mode = PRIVATE_TMP;
> + m++;
>
> - if (chmod(var_tmp_dir, 0777 + S_ISVTX) < 0) {
> - r = -errno;
> - goto fail;
> - }
> + m->path = "/var/tmp";
> + m->mode = PRIVATE_VAR_TMP;
> + m++;
> }
>
> - if (unshare(CLONE_NEWNS) < 0) {
> - r = -errno;
> - goto fail;
> - }
> + assert(mounts + n == m);
> +
> + qsort(mounts, n, sizeof(BindMount), mount_path_compare);
> + drop_duplicates(mounts, &n);
>
> /* Remount / as SLAVE so that nothing now mounted in the namespace
> shows up in the parent */
> @@ -291,20 +257,20 @@ int setup_namespace(
> goto fail;
> }
>
> - for (p = paths; p < paths + n; p++) {
> - r = apply_mount(p, tmp_dir, var_tmp_dir, inaccessible_dir);
> + for (m = mounts; m < mounts + n; ++m) {
> + r = apply_mount(m, tmp_dir, var_tmp_dir);
> if (r < 0)
> goto undo_mounts;
> }
>
> - for (p = paths; p < paths + n; p++) {
> - r = make_read_only(p);
> + for (m = mounts; m < mounts + n; ++m) {
> + r = make_read_only(m);
> if (r < 0)
> goto undo_mounts;
> }
>
> /* Remount / as the desired mode */
> - if (mount(NULL, "/", NULL, flags|MS_REC, NULL) < 0) {
> + if (mount(NULL, "/", NULL, mount_flags | MS_REC, NULL) < 0) {
> r = -errno;
> goto undo_mounts;
> }
> @@ -312,19 +278,11 @@ int setup_namespace(
> return 0;
>
> undo_mounts:
> - for (p = paths; p < paths + n; p++)
> - if (p->done)
> - umount2(p->path, MNT_DETACH);
> + for (m = mounts; m < mounts + n; ++m) {
> + if (m->done)
> + umount2(m->path, MNT_DETACH);
> + }
>
> fail:
> - if (remove_inaccessible)
> - rmdir(inaccessible_dir);
> -
> - if (remove_tmp)
> - rmdir(tmp_dir);
> -
> - if (remove_var_tmp)
> - rmdir(var_tmp_dir);
> -
> return r;
> }
It seems that the "fail:" block consists only of 'return'. So it's not needed,
and you can return directly from the point of error.
> diff --git a/src/core/namespace.h b/src/core/namespace.h
> index 5d72ed9..e6f152a 100644
> --- a/src/core/namespace.h
> +++ b/src/core/namespace.h
> @@ -20,12 +20,15 @@
> You should have received a copy of the GNU Lesser General Public License
> along with systemd; If not, see <http://www.gnu.org/licenses/>.
> ***/
> -
Why?
> #include <stdbool.h>
>
> -int setup_namespace(
> - char **writable,
> - char **readable,
> - char **inaccessible,
> - bool private_tmp,
> - unsigned long flags);
> +typedef struct ExecContext ExecContext;
> +
> +int setup_tmpdirs(char **tmp_dir, char **var_tmp_dir);
> +int setup_namespace(char **read_write_dirs,
> + char **read_only_dirs,
> + char **inaccessible_dirs,
> + char *tmp_dir,
> + char *var_tmp_dir,
> + bool private_tmp,
> + unsigned mount_flags);
> diff --git a/src/core/service.c b/src/core/service.c
> index 3fbb0a1..d93ea9b 100644
> --- a/src/core/service.c
> +++ b/src/core/service.c
> @@ -283,7 +283,7 @@ static void service_done(Unit *u) {
> free(s->status_text);
> s->status_text = NULL;
>
> - exec_context_done(&s->exec_context);
> + exec_context_done(&s->exec_context, manager_is_reloading_or_reexecuting(u->manager));
> exec_command_free_array(s->exec_command, _SERVICE_EXEC_COMMAND_MAX);
> s->control_command = NULL;
> s->main_command = NULL;
> @@ -1932,6 +1932,9 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart)
>
> s->forbid_restart = false;
>
> + /* we want fresh tmpdirs in case service is started again immediately */
> + exec_context_tmp_dirs_done(&s->exec_context);
> +
> return;
>
> fail:
> @@ -2558,6 +2561,7 @@ static int service_stop(Unit *u) {
> s->state == SERVICE_EXITED);
>
> service_enter_stop(s, SERVICE_SUCCESS);
> +
> return 0;
> }
>
> @@ -2638,6 +2642,12 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) {
> dual_timestamp_serialize(f, "watchdog-timestamp",
> &s->watchdog_timestamp);
>
> + if (s->exec_context.tmp_dir)
> + unit_serialize_item(u, f, "tmp-dir", s->exec_context.tmp_dir);
> +
> + if (s->exec_context.var_tmp_dir)
> + unit_serialize_item(u, f, "var-tmp-dir", s->exec_context.var_tmp_dir);
> +
> return 0;
> }
>
> @@ -2756,7 +2766,23 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
> dual_timestamp_deserialize(value, &s->main_exec_status.exit_timestamp);
> else if (streq(key, "watchdog-timestamp"))
> dual_timestamp_deserialize(value, &s->watchdog_timestamp);
> - else
> + else if (streq(key, "tmp-dir")) {
> + char *t;
> +
> + t = strdup(value);
> + if (!t)
> + return log_oom();
> +
> + s->exec_context.tmp_dir = t;
> + } else if (streq(key, "var-tmp-dir")) {
> + char *t;
> +
> + t = strdup(value);
> + if (!t)
> + return log_oom();
> +
> + s->exec_context.var_tmp_dir = t;
> + } else
> log_debug_unit(u->id, "Unknown serialization key '%s'", key);
>
> return 0;
> diff --git a/src/core/socket.c b/src/core/socket.c
> index 2105369..f558026 100644
> --- a/src/core/socket.c
> +++ b/src/core/socket.c
> @@ -127,7 +127,7 @@ static void socket_done(Unit *u) {
>
> socket_free_ports(s);
>
> - exec_context_done(&s->exec_context);
> + exec_context_done(&s->exec_context, manager_is_reloading_or_reexecuting(u->manager));
> exec_command_free_array(s->exec_command, _SOCKET_EXEC_COMMAND_MAX);
> s->control_command = NULL;
>
> @@ -1253,6 +1253,7 @@ static void socket_enter_dead(Socket *s, SocketResult f) {
> if (f != SOCKET_SUCCESS)
> s->result = f;
>
> + exec_context_tmp_dirs_done(&s->exec_context);
> socket_set_state(s, s->result != SOCKET_SUCCESS ? SOCKET_FAILED : SOCKET_DEAD);
> }
>
> @@ -1742,6 +1743,8 @@ static int socket_serialize(Unit *u, FILE *f, FDSet *fds) {
> }
> }
>
> + exec_context_serialize(&s->exec_context, UNIT(s), f);
> +
> return 0;
> }
>
> @@ -1901,7 +1904,22 @@ static int socket_deserialize_item(Unit *u, const char *key, const char *value,
> p->fd = fdset_remove(fds, fd);
> }
> }
> + } else if (streq(key, "tmp-dir")) {
> + char *t;
> +
> + t = strdup(value);
> + if (!t)
> + return log_oom();
> +
> + s->exec_context.tmp_dir = t;
> + } else if (streq(key, "var-tmp-dir")) {
> + char *t;
> +
> + t = strdup(value);
> + if (!t)
> + return log_oom();
>
> + s->exec_context.var_tmp_dir = t;
> } else
> log_debug_unit(UNIT(s)->id,
> "Unknown serialization key '%s'", key);
> diff --git a/src/core/swap.c b/src/core/swap.c
> index 61ce831..58c3f58 100644
> --- a/src/core/swap.c
> +++ b/src/core/swap.c
> @@ -125,7 +125,7 @@ static void swap_done(Unit *u) {
> free(s->parameters_fragment.what);
> s->parameters_fragment.what = NULL;
>
> - exec_context_done(&s->exec_context);
> + exec_context_done(&s->exec_context, manager_is_reloading_or_reexecuting(u->manager));
> exec_command_done_array(s->exec_command, _SWAP_EXEC_COMMAND_MAX);
> s->control_command = NULL;
>
> @@ -632,6 +632,7 @@ static void swap_enter_dead(Swap *s, SwapResult f) {
> if (f != SWAP_SUCCESS)
> s->result = f;
>
> + exec_context_tmp_dirs_done(&s->exec_context);
> swap_set_state(s, s->result != SWAP_SUCCESS ? SWAP_FAILED : SWAP_DEAD);
> }
>
> @@ -831,6 +832,8 @@ static int swap_serialize(Unit *u, FILE *f, FDSet *fds) {
> if (s->control_command_id >= 0)
> unit_serialize_item(u, f, "control-command", swap_exec_command_to_string(s->control_command_id));
>
> + exec_context_serialize(&s->exec_context, UNIT(s), f);
> +
> return 0;
> }
>
> @@ -874,7 +877,22 @@ static int swap_deserialize_item(Unit *u, const char *key, const char *value, FD
> s->control_command_id = id;
> s->control_command = s->exec_command + id;
> }
> + } else if (streq(key, "tmp-dir")) {
> + char *t;
> +
> + t = strdup(value);
> + if (!t)
> + return log_oom();
> +
> + s->exec_context.tmp_dir = t;
> + } else if (streq(key, "var-tmp-dir")) {
> + char *t;
> +
> + t = strdup(value);
> + if (!t)
> + return log_oom();
>
> + s->exec_context.var_tmp_dir = t;
> } else
> log_debug_unit(u->id, "Unknown serialization key '%s'", key);
>
> diff --git a/src/shared/util.c b/src/shared/util.c
> index dc2651f..6f965dd 100644
> --- a/src/shared/util.c
> +++ b/src/shared/util.c
> @@ -5682,3 +5682,48 @@ int search_and_fopen_nulstr(const char *path, const char *mode, const char *sear
>
> return search_and_fopen_internal(path, mode, s, _f);
> }
> +
> +int create_tmp_dir(char template[], mode_t mask, bool need_sticky, char** dir_name) {
> + int r = 0;
> + char *d = NULL;
> + bool remove = false;
> + mode_t u;
> +
> + assert(dir_name);
> +
> + u = umask(mask);
> + d = mkdtemp(template);
> + umask(u);
We have _cleanup_umask_.
> + if (!d) {
> + r = -errno;
> + log_debug("Can't create directory");
> + goto fail;
> + }
> +
> + remove = true;
> +
> + log_debug("Created temporary directory : %s", template);
> +
> + d = strdup(template);
> + if (!d) {
> + r = log_oom();
> + goto fail;
> + }
> +
> + if (need_sticky) {
> + r = chmod(template, 0777 | S_ISVTX);
> + if (r < 0) {
> + r = -errno;
> + goto fail;
> + }
> + log_debug("Setting sticky bit on : %s", template);
> + }
> +
> + *dir_name = d;
> +
> + return 0;
> +fail:
> + if (remove)
> + rmdir(template);
> + return r;
> +}
> diff --git a/src/shared/util.h b/src/shared/util.h
> index 04c9fcd..be758ff 100644
> --- a/src/shared/util.h
> +++ b/src/shared/util.h
> @@ -574,6 +574,7 @@ int on_ac_power(void);
>
> int search_and_fopen(const char *path, const char *mode, const char **search, FILE **_f);
> int search_and_fopen_nulstr(const char *path, const char *mode, const char *search, FILE **_f);
> +int create_tmp_dir(char template[], mode_t mask, bool need_sticky, char** dir_name);
>
> #define FOREACH_LINE(line, f, on_error) \
> for (;;) \
> diff --git a/src/test/test-ns.c b/src/test/test-ns.c
> index b1c759f..a684f20 100644
> --- a/src/test/test-ns.c
> +++ b/src/test/test-ns.c
> @@ -26,6 +26,7 @@
> #include <linux/fs.h>
>
> #include "namespace.h"
> +#include "execute.h"
> #include "log.h"
>
> int main(int argc, char *argv[]) {
> @@ -47,8 +48,19 @@ int main(int argc, char *argv[]) {
> };
>
> int r;
> + char tmp_dir[] = "/tmp/systemd-private-XXXXXX",
> + var_tmp_dir[] = "/var/tmp/systemd-private-XXXXXX";
>
> - r = setup_namespace((char**) writable, (char**) readonly, (char**) inaccessible, true, 0);
> + mkdtemp(tmp_dir);
> + mkdtemp(var_tmp_dir);
This can fail. (assert_se() is enough.)
> +
> + r = setup_namespace((char **) writable,
> + (char **) readonly,
> + (char **) inaccessible,
> + tmp_dir,
> + var_tmp_dir,
> + true,
> + 0);
> if (r < 0) {
> log_error("Failed to setup namespace: %s", strerror(-r));
> return 1;
Zbyszek
More information about the systemd-devel
mailing list