Re: [systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-03-16 Thread Michal Sekletár 
On Sat, Mar 16, 2013 at 4:11 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Thu, Mar 14, 2013 at 06:12:27PM +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 in addition to systemd-tmpfiles. Directory which is mounted
 as inaccessible is created at runtime in /run/systemd.
 Finally applied. Thanks!

Hi Zbyszek

thank you for the reviews and very helpful suggestions.

Michal

 Zbyszek
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-03-15 Thread Zbigniew Jędrzejewski-Szmek 
On Thu, Mar 14, 2013 at 06:12:27PM +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 in addition to systemd-tmpfiles. Directory which is mounted
 as inaccessible is created at runtime in /run/systemd.
Finally applied. Thanks!

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-03-14 Thread Michal Sekletár 
On Thu, Mar 14, 2013 at 3:33 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 Hi,

 Colin Walters wrote
  install-directories-hook:
   $(MKDIR_P) $(addprefix $(DESTDIR),$(INSTALL_DIRS))
 + $(MKDIR_P) -m 000 $(addprefix $(DESTDIR),$(INACCESSIBLE_DIR))
 Ugh.  Can you make this /run/systemd/inaccessible or something, and
 have systemd do this at runtime?

 Having systemd install a directory with mode 0 is going to screw with
 a lot of build systems.

 Indeed. Even our own 'make distcheck' fails badly with this patch.
 What do you think about the idea of creating /run/systemd/inaccessible
 on first use?

 (Note: there's an error there, and the $(addprefix) call is not needed.
  But if this line will be removed this doesn't matter.)

 Zbyszek

Hi Zbyszek,

It was my intention to do it at runtime, however I couldn't figure out
when it is the right time to do it. I wanted to do it once, and not to
check for this directory everytime we are spawning a new process which
needs it. Do you have any idea, when is the right time to do it, such
that it will work for all scenarios (with dracut, in containers,
etc...) ?

Do you prefer to have it /run or in /tmp?

Michal

 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-03-14 Thread Kay Sievers 
On Thu, Mar 14, 2013 at 2:07 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Thu, Mar 14, 2013 at 11:43:32AM +0100, Michal Sekletár wrote:

 Do you prefer to have it /run or in /tmp?
 It seems to be without much difference, but I'd go for /run, since this
 way there'll be less chance that somebody deletes it by mistake.

Things private to services, and for services which carry the
privileges to access /run should always live in /run, because /run is
private, cannot be used by ordinary users, and does not have the weird
it's world-accessible rules.

Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-03-14 Thread Michal Sekletar 
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 in addition to systemd-tmpfiles. Directory which is mounted
as inaccessible is created at runtime in /run/systemd.
---
 man/systemd.exec.xml   |   4 +-
 src/core/execute.c |  43 +-
 src/core/execute.h |  10 ++-
 src/core/manager.c |   6 ++
 src/core/manager.h |   3 +
 src/core/mount-setup.c |   1 +
 src/core/mount.c   |  21 -
 src/core/namespace.c   | 226 -
 src/core/namespace.h   |  16 ++--
 src/core/service.c |  29 ++-
 src/core/socket.c  |  20 -
 src/core/swap.c|  20 -
 src/shared/util.c  |  44 ++
 src/shared/util.h  |   1 +
 src/test/test-ns.c |  14 ++-
 15 files changed, 305 insertions(+), 153 deletions(-)

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

Re: [systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-03-14 Thread Michal Sekletar 
Hi Zbyszek,

- Original Message -
 Maybe just stick it in mount_setup()?

Thank you for this suggestion!

 
 It seems to be without much difference, but I'd go for /run, since
 this way there'll be less chance that somebody deletes it by mistake.

I first tried /tmp and I had issue that original directory I created got lost 
because /tmp was remounted as tmpfs. Putting directory to /run resolved this
issue and I think it is better than previous approach.

 
 Zbyszek
 

Michal
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-03-13 Thread Zbigniew Jędrzejewski-Szmek 
Hi,

Colin Walters wrote
  install-directories-hook:
   $(MKDIR_P) $(addprefix $(DESTDIR),$(INSTALL_DIRS))
 + $(MKDIR_P) -m 000 $(addprefix $(DESTDIR),$(INACCESSIBLE_DIR))
 Ugh.  Can you make this /run/systemd/inaccessible or something, and
 have systemd do this at runtime?

 Having systemd install a directory with mode 0 is going to screw with
 a lot of build systems.

Indeed. Even our own 'make distcheck' fails badly with this patch.
What do you think about the idea of creating /run/systemd/inaccessible
on first use?

(Note: there's an error there, and the $(addprefix) call is not needed.
 But if this line will be removed this doesn't matter.)

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-03-12 Thread Michal Sekletar 
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. 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))
 
 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) {

Re: [systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-03-12 Thread Michal Sekletar 
Thank you for the review! It is very appreciated.

Michal
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-03-12 Thread Colin Walters 
On Tue, 2013-03-12 at 15:14 +0100, Michal Sekletar wrote:

  install-directories-hook:
   $(MKDIR_P) $(addprefix $(DESTDIR),$(INSTALL_DIRS))
 + $(MKDIR_P) -m 000 $(addprefix $(DESTDIR),$(INACCESSIBLE_DIR))

Ugh.  Can you make this /run/systemd/inaccessible or something, and
have systemd do this at runtime?

Having systemd install a directory with mode 0 is going to screw with a
lot of build systems.


___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-03-04 Thread Lennart Poettering 
On Sat, 02.03.13 15:00, Michal Sekletar (sekleta...@gmail.com) wrote:

 
 On Mar 1, 2013, at 3:16 PM, Lennart Poettering lenn...@poettering.net wrote:
 
  On Wed, 20.02.13 14:50, Michal Sekletar (msekl...@redhat.com) 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. Same logic applies also to inaccessible directories.
  
  Hmm, looks good in principle, but I am don't grok why we need
  ExecContext.bind_mounts? Can you elaborate?
 
 Hi Lennart,
 
 Originally we determined what bind mounts should be done in a child
 process each time we forked of a new process, and it was done after
 fork() in the child before executing target binary.
 
 Now, I've moved this computation to systemd itself and results are
 stored in ExecContext.bind_mounts set. Another reason was that, using
 former approach it was impossible to determine in pid 1, if we need to
 create tmpdir to be mounted as inaccessible for a child.

Not following really? Which bind mounts are these? For the inaccessible
dir stuff? But those are only visible in the per-service namespace, and
go away automatically of the service dies (because if all processes of a
service dies the namespace dies too). So I don't really understand why
we would have to keep track of this?

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-03-02 Thread Michal Sekletar 
On Mar 1, 2013, at 3:16 PM, Lennart Poettering lenn...@poettering.net wrote:

 On Wed, 20.02.13 14:50, Michal Sekletar (msekl...@redhat.com) 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. Same logic applies also to inaccessible directories.
 
 Hmm, looks good in principle, but I am don't grok why we need
 ExecContext.bind_mounts? Can you elaborate?

Hi Lennart,

Originally we determined what bind mounts should be done in a child process 
each time we forked of a new process, and it was done after fork() in the child 
before executing target binary. 

Now, I've moved this computation to systemd itself and results are stored in 
ExecContext.bind_mounts set. Another reason was that, using former approach it 
was impossible to determine in pid 1, if we need to create tmpdir to be mounted 
as inaccessible for a child.

Michal

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-03-01 Thread Lennart Poettering 
On Wed, 20.02.13 14:50, Michal Sekletar (msekl...@redhat.com) 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. Same logic applies also to inaccessible directories.

Hmm, looks good in principle, but I am don't grok why we need
ExecContext.bind_mounts? Can you elaborate?

Thanks,

Lennart

 ---
  man/systemd.exec.xml |   4 +-
  src/core/execute.c   |  78 --
  src/core/execute.h   |  18 +++-
  src/core/manager.c   |   6 ++
  src/core/manager.h   |   2 +
  src/core/mount.c |  29 -
  src/core/namespace.c | 291 
 ++-
  src/core/namespace.h |  26 +++--
  src/core/service.c   |  41 +++-
  src/core/socket.c|  31 +-
  src/core/swap.c  |  28 -
  src/test/test-ns.c   |  18 +++-
  12 files changed, 423 insertions(+), 149 deletions(-)
 
 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 b28962a..fabc0cd 100644
 --- a/src/core/execute.c
 +++ b/src/core/execute.c
 @@ -40,6 +40,7 @@
  #include sys/poll.h
  #include linux/seccomp-bpf.h
  #include glob.h
 +#include execinfo.h
  
  #ifdef HAVE_PAM
  #include security/pam_appl.h
 @@ -165,6 +166,21 @@ void exec_context_tty_reset(const ExecContext *context) {
  vt_disallocate(context-tty_path);
  }
  
 +void exec_context_tmp_dirs_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);
 +
 +if (context-inaccessible_dir)
 +unit_serialize_item(u, f, inaccessible-dir, 
 context-inaccessible_dir);
 +}
 +
  static int open_null_as(int flags, int nfd) {
  int fd, r;
  
 @@ -960,7 +976,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,
 @@ -1028,6 +1044,10 @@ int exec_spawn(ExecCommand *command,
  
  cgroup_attribute_apply_list(cgroup_attributes, cgroup_bondings);
  
 +r = setup_tmpdirs(context);
 +if (r  0)
 +return r;
 +
  pid = fork();
  if (pid  0)
  return -errno;
 @@ -1289,13 +1309,8 @@ int exec_spawn(ExecCommand *command,
  if (strv_length(context-read_write_dirs)  0 ||
  strv_length(context-read_only_dirs)  0 ||
  strv_length(context-inaccessible_dirs)  0 ||
 -context-mount_flags != 0 ||
 -context-private_tmp) {
 -err = setup_namespace(context-read_write_dirs,
 -  context-read_only_dirs,
 -  context-inaccessible_dirs,
 -  context-private_tmp,
 -  context-mount_flags);
 +context-mount_flags != 0 ) {
 +err = setup_namespace(context);
  if (err  0) {
  r = EXIT_NAMESPACE;
  goto fail_child;
 @@ -1522,7 +1537,46 @@ void exec_context_init(ExecContext *c) {
  c-timer_slack_nsec = (nsec_t) -1;
  }
  
 -void exec_context_done(ExecContext *c) {
 +static 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;
 +}
 +
 +

[systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-02-20 Thread Michal Sekletar 
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. Same logic applies also to inaccessible directories.
---
 man/systemd.exec.xml |   4 +-
 src/core/execute.c   |  78 --
 src/core/execute.h   |  18 +++-
 src/core/manager.c   |   6 ++
 src/core/manager.h   |   2 +
 src/core/mount.c |  29 -
 src/core/namespace.c | 291 ++-
 src/core/namespace.h |  26 +++--
 src/core/service.c   |  41 +++-
 src/core/socket.c|  31 +-
 src/core/swap.c  |  28 -
 src/test/test-ns.c   |  18 +++-
 12 files changed, 423 insertions(+), 149 deletions(-)

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 b28962a..fabc0cd 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -40,6 +40,7 @@
 #include sys/poll.h
 #include linux/seccomp-bpf.h
 #include glob.h
+#include execinfo.h
 
 #ifdef HAVE_PAM
 #include security/pam_appl.h
@@ -165,6 +166,21 @@ void exec_context_tty_reset(const ExecContext *context) {
 vt_disallocate(context-tty_path);
 }
 
+void exec_context_tmp_dirs_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);
+
+if (context-inaccessible_dir)
+unit_serialize_item(u, f, inaccessible-dir, 
context-inaccessible_dir);
+}
+
 static int open_null_as(int flags, int nfd) {
 int fd, r;
 
@@ -960,7 +976,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,
@@ -1028,6 +1044,10 @@ int exec_spawn(ExecCommand *command,
 
 cgroup_attribute_apply_list(cgroup_attributes, cgroup_bondings);
 
+r = setup_tmpdirs(context);
+if (r  0)
+return r;
+
 pid = fork();
 if (pid  0)
 return -errno;
@@ -1289,13 +1309,8 @@ int exec_spawn(ExecCommand *command,
 if (strv_length(context-read_write_dirs)  0 ||
 strv_length(context-read_only_dirs)  0 ||
 strv_length(context-inaccessible_dirs)  0 ||
-context-mount_flags != 0 ||
-context-private_tmp) {
-err = setup_namespace(context-read_write_dirs,
-  context-read_only_dirs,
-  context-inaccessible_dirs,
-  context-private_tmp,
-  context-mount_flags);
+context-mount_flags != 0 ) {
+err = setup_namespace(context);
 if (err  0) {
 r = EXIT_NAMESPACE;
 goto fail_child;
@@ -1522,7 +1537,46 @@ void exec_context_init(ExecContext *c) {
 c-timer_slack_nsec = (nsec_t) -1;
 }
 
-void exec_context_done(ExecContext *c) {
+static 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;
+}
+
+if (c-inaccessible_dir) {
+rm_rf_dangerous(c-inaccessible_dir, false, true, false);
+free(c-inaccessible_dir);
+c-inaccessible_dir = NULL;
+c-need_inaccessible = false;
+}
+}
+
+static void exec_context_bind_mounts_done(ExecContext *c) {
+

[systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-02-13 Thread Michal Sekletar 
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. Same logic applies also to inaccessible directories.
---
 man/systemd.exec.xml |   4 +-
 src/core/execute.c   |  43 ++--
 src/core/execute.h   |  13 ++-
 src/core/namespace.c | 291 +--
 src/core/namespace.h |  26 +++--
 src/core/service.c   |  10 ++
 src/test/test-ns.c   |  18 +++-
 7 files changed, 261 insertions(+), 144 deletions(-)

diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index 53094e5..a33517e 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 aa58bc4..770f1f1 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -40,6 +40,7 @@
 #include sys/poll.h
 #include linux/seccomp-bpf.h
 #include glob.h
+#include execinfo.h
 
 #ifdef HAVE_PAM
 #include security/pam_appl.h
@@ -984,7 +985,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,
@@ -1052,6 +1053,10 @@ int exec_spawn(ExecCommand *command,
 
 cgroup_attribute_apply_list(cgroup_attributes, cgroup_bondings);
 
+r = setup_tmpdirs(context);
+if (r  0)
+return r;
+
 pid = fork();
 if (pid  0)
 return -errno;
@@ -1313,13 +1318,8 @@ int exec_spawn(ExecCommand *command,
 if (strv_length(context-read_write_dirs)  0 ||
 strv_length(context-read_only_dirs)  0 ||
 strv_length(context-inaccessible_dirs)  0 ||
-context-mount_flags != 0 ||
-context-private_tmp) {
-err = setup_namespace(context-read_write_dirs,
-  context-read_only_dirs,
-  context-inaccessible_dirs,
-  context-private_tmp,
-  context-mount_flags);
+context-mount_flags != 0 ) {
+err = setup_namespace(context);
 if (err  0) {
 r = EXIT_NAMESPACE;
 goto fail_child;
@@ -1546,6 +1546,26 @@ void exec_context_init(ExecContext *c) {
 c-timer_slack_nsec = (nsec_t) -1;
 }
 
+void exec_context_tmpdirs_done(ExecContext *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;
+}
+
+if (c-inaccessible_dir) {
+rm_rf_dangerous(c-inaccessible_dir, false, true, false);
+free(c-inaccessible_dir);
+c-inaccessible_dir = NULL;
+}
+}
+
 void exec_context_done(ExecContext *c) {
 unsigned l;
 
@@ -1610,6 +1630,11 @@ void exec_context_done(ExecContext *c) {
 
 free(c-syscall_filter);
 c-syscall_filter = NULL;
+
+exec_context_tmpdirs_done(c);
+
+free(c-bind_mounts);
+c-bind_mounts = NULL;
 }
 
 void exec_command_done(ExecCommand *c) {
@@ -2158,4 +2183,4 @@ static const char* const 
exec_output_table[_EXEC_OUTPUT_MAX] = {
 [EXEC_OUTPUT_SOCKET] = socket
 };
 
-DEFINE_STRING_TABLE_LOOKUP(exec_output, ExecOutput);
+DEFINE_STRING_TABLE_LOOKUP(exec_output, ExecOutput);
\ No newline at end of file
diff --git a/src/core/execute.h b/src/core/execute.h
index 2bcd2e1..1728d89 100644
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -38,6 +38,8 @@ struct CGroupAttribute;
 
 #include list.h
 #include util.h
+#include namespace.h
+
 
 typedef enum ExecInput {
 EXEC_INPUT_NULL,
@@ -141,6 +143,14 @@ struct ExecContext {
 bool non_blocking;
 bool private_tmp;
 bool private_network;
+char *tmp_dir;
+char

Re: [systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-02-13 Thread Tom Gundersen 
On Wed, Feb 13, 2013 at 2:45 PM, Michal Sekletar msekl...@redhat.com 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. Same logic applies also to inaccessible directories.

Something I still can't quite understand: you clean /tmp in
service_stop so that it will be clean for when we call service_start.
Is there any reason you don't do this in service_enter_dead instead?
To me that seems much clearer, but I might be missing something (I
tried to chase the state diagram to see if they are equivalent, but I
gave up in the end...).

Also, shouldn't we be cleaning up private tmp for all the other unit
types as well?

A couple of very minor nitpicks inline.

 diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
 index 53094e5..a33517e 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

Assuming you agree with the above, this should probably be 'unit'
rather than 'service'? Also, the rest of the paragraph should probably
be rephrased as it talks about the impossibility of sharing /tmp
between processes, but now we can do that, just not with processes not
belonging to the same unit.

 @@ -312,19 +362,10 @@ int setup_namespace(
  return 0;

  undo_mounts:
 -for (p = paths; p  paths + n; p++)
 -if (p-done)
 -umount2(p-path, MNT_DETACH);
 +for (m = context-bind_mounts; m  context-bind_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;

You could drop the 'fail' label, and just do 'return -errno' instead
of the 'goto'.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

2013-02-13 Thread Lennart Poettering 
On Wed, 13.02.13 14:45, Michal Sekletar (msekl...@redhat.com) wrote:

Heya!

Looks good in principle.

 +if (context-private_tmp) {
 +if (!context-tmp_dir) {
 +d = mktemp(tmp_dir);

mktemp() is never OK. Also see BUGS section in the man page about
that. Use mkdtemp() or so instead.

 +if (!d) {
 +r = -errno;
 +goto fail;
 +}
 +
 +context-tmp_dir = strdup(d);
 +if (!context-tmp_dir) {
 +r = log_oom();
 +goto fail;
 +}
 +
 +u = umask();
 +r = mkdir(tmp_dir, 0777);
 +umask(u);
 +if (r  0) {
 +free(context-tmp_dir);
 +context-tmp_dir = NULL;
 +r = -errno;
 +goto fail;
 +}
 +remove_tmp = true;
 +
 +if (chmod(tmp_dir, 0777 | S_ISVTX)  0) {
 +r = -errno;
 +goto fail;
 +}
 +}
  
 -p-path = /var/tmp;
 -p-mode = PRIVATE_VAR_TMP;
 -p++;
 +if (!context-var_tmp_dir) {
 +d = mktemp(var_tmp_dir);
 +if (!d) {
 +r = -errno;
 +goto fail;
 +}
 +
 +context-var_tmp_dir = strdup(d);
 +if (!context-var_tmp_dir) {
 +r = log_oom();
 +goto fail;
 +}
 +
 +u = umask();
 +r = mkdir(var_tmp_dir, 0777);
 +umask(u);
 +if (r  0) {
 +free(context-var_tmp_dir);
 +context-var_tmp_dir = NULL;
 +r = -errno;
 +goto fail;
 +}
 +remove_var_tmp = true;
 +
 +if (chmod(var_tmp_dir, 0777 | S_ISVTX)  0) {
 +r = -errno;
 +goto fail;
 +}


These two blocks really look like they want to be replaced by a function
that we just can call twice...

  static void service_enter_dead(Service *s, ServiceResult f, bool 
 allow_restart) {
  int r;
  assert(s);
 @@ -2520,6 +2526,10 @@ static int service_stop(Unit *u) {
 s-state == SERVICE_EXITED);
  
  service_enter_stop(s, SERVICE_SUCCESS);
 +
 +/* we want empty tmp dirs when service is started again */
 +service_cleanup_tmpdirs(s);
 +

This should probably move to service_enter_dead() or so.

Also, we should have the same in socket.c, mount.c, swap.c probably, as
they also use exec context...

Lennart

-- 
Lennart Poettering - Red Hat, Inc.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel