Re: [systemd-devel] Docker vs PrivateTmp

2015-01-18 Thread Lars Kellogg-Stedman 
On Sun, Jan 18, 2015 at 08:50:35PM -0500, Colin Walters wrote:
 On Sat, Jan 17, 2015, at 11:02 PM, Lars Kellogg-Stedman wrote:
  Hello all,
  
  With systemd 216 on Fedora 21 (kernel 3.17.8), I have run into an odd
  behavior concerning the PrivateTmp directive, and I am looking for
  help identifying this as:
  
  - Everything Is Working As Designed, Citizen
  - A bug in Docker (some mount flag is being set incorrectly?)
 
 This should be fixed by:
 http://pkgs.fedoraproject.org/cgit/docker-io.git/commit/?id=6c9e373ee06cb1aee07d3cae426c46002663010d
 
 i.e. having docker.service use MountFlags=private, so its mounts
 aren't visible to other processes.

Colin,

Thanks for the pointer.

It seems as if using MountFlags=private is going to cause a new set of
problems:

Imagine that I am a system administrator using Docker to containerize
services.  I want to serve set up a webserver container on my Docker
host, so I mount the web content from a remote server:

mount my-fancy-server:/vol/content /content

And then expose that as a Docker volume:

docker run -v /content:/content webserver

This will fail mysteriously, because with MountFlags=private, the
mount of my-fancy-server:/vol/content on /content won't be visible to
Docker containers.  I will spend fruitless hours trying to figure out
why such a seemingly simple operation is failing.

I think we actually want MountFlags=slave, which will permit mounts
from the global namespace to propagate into the service namespace
without permitting propagation in the other direction.  It seems like
this would the Least Surprising behavior.

-- 
Lars Kellogg-Stedman l...@redhat.com | larsks @ {freenode,twitter,github}
Cloud Engineering / OpenStack  | http://blog.oddbit.com/



pgphEQ65s0FS9.pgp
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Docker vs PrivateTmp

2015-01-18 Thread Colin Walters 
On Sat, Jan 17, 2015, at 11:02 PM, Lars Kellogg-Stedman wrote:
 Hello all,
 
 With systemd 216 on Fedora 21 (kernel 3.17.8), I have run into an odd
 behavior concerning the PrivateTmp directive, and I am looking for
 help identifying this as:
 
 - Everything Is Working As Designed, Citizen
 - A bug in Docker (some mount flag is being set incorrectly?)

This should be fixed by:
http://pkgs.fedoraproject.org/cgit/docker-io.git/commit/?id=6c9e373ee06cb1aee07d3cae426c46002663010d

i.e. having docker.service use MountFlags=private, so its mounts
aren't visible to other processes.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Docker vs PrivateTmp

2015-01-18 Thread Lars Kellogg-Stedman 
On Sun, Jan 18, 2015 at 11:38:12PM -0500, Lars Kellogg-Stedman wrote:
 I think we actually want MountFlags=slave, which will permit mounts
 from the global namespace to propagate into the service namespace
 without permitting propagation in the other direction.  It seems like
 this would the Least Surprising behavior.

...which would be the default if docker.service were itself using
PrivateTmp=true, because from systemd.exec:

Note that the file system namespace related options (PrivateTmp=,
PrivateDevices=, ProtectSystem=, ProtectHome=, ReadOnlyDirectories=,
InaccessibleDirectories= and ReadWriteDirectories=) require that mount
and unmount propagation from the unit's file system namespace is
disabled, and hence downgrade shared to slave.

So either explicitly setting MountFlags=slave, or setting
PrivateTmp=true if that doesn't cause any issues of which I am not
aware.

-- 
Lars Kellogg-Stedman l...@redhat.com | larsks @ {freenode,twitter,github}
Cloud Engineering / OpenStack  | http://blog.oddbit.com/



pgpiVLDyZPrQb.pgp
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 04/11] tmpfiles: attach an array of items to each path

2015-01-18 Thread Greg KH 
On Mon, Jan 19, 2015 at 01:20:37AM +0100, Zbigniew Jędrzejewski-Szmek wrote:
 The data structure used by tmpfiles is changed: instead of hashmaps
 mapping {path → Item*} we now have hashmaps containing
 {path - ItemArray}, where ItemArray contains a pointer
 to an array of Items.
 
 For current code it doesn't matter much, but when we add new types it
 is easier to simply add a new Item for a given path, then to coalesce
 multiple lines into one Item.
 
 In the future, this change will also make it possible to remember the
 file and line where each Item originates, and use that in reporting
 errors. Currently this is not possible, since each Item can be created
 from multiple lines.
 ---
  man/tmpfiles.d.xml  |  13 +-
  src/tmpfiles/tmpfiles.c | 308 
 
  2 files changed, 160 insertions(+), 161 deletions(-)

Oh, I like this, nice job.  As someone who has tried to add new
functionalty to this code, this looks to make it much easier to do in
the future, if it is needed.

Bonus being it's the same number of lines :)

good work,

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

Re: [systemd-devel] Docker vs PrivateTmp

2015-01-18 Thread Lokesh Mandvekar 
On Sun, Jan 18, 2015 at 11:38:12PM -0500, Lars Kellogg-Stedman wrote:
 On Sun, Jan 18, 2015 at 08:50:35PM -0500, Colin Walters wrote:
  On Sat, Jan 17, 2015, at 11:02 PM, Lars Kellogg-Stedman wrote:
   Hello all,
   
   With systemd 216 on Fedora 21 (kernel 3.17.8), I have run into an odd
   behavior concerning the PrivateTmp directive, and I am looking for
   help identifying this as:
   
   - Everything Is Working As Designed, Citizen
   - A bug in Docker (some mount flag is being set incorrectly?)
  
  This should be fixed by:
  http://pkgs.fedoraproject.org/cgit/docker-io.git/commit/?id=6c9e373ee06cb1aee07d3cae426c46002663010d
  
  i.e. having docker.service use MountFlags=private, so its mounts
  aren't visible to other processes.
 
 Colin,
 
 Thanks for the pointer.
 
 It seems as if using MountFlags=private is going to cause a new set of
 problems:
 
 Imagine that I am a system administrator using Docker to containerize
 services.  I want to serve set up a webserver container on my Docker
 host, so I mount the web content from a remote server:
 
 mount my-fancy-server:/vol/content /content
 
 And then expose that as a Docker volume:
 
 docker run -v /content:/content webserver
 
 This will fail mysteriously, because with MountFlags=private, the
 mount of my-fancy-server:/vol/content on /content won't be visible to
 Docker containers.  I will spend fruitless hours trying to figure out
 why such a seemingly simple operation is failing.
 
 I think we actually want MountFlags=slave, which will permit mounts
 from the global namespace to propagate into the service namespace
 without permitting propagation in the other direction.  It seems like
 this would the Least Surprising behavior.

Copying dwalsh


-- 
Lokesh
Freenode, OFTC: lsm5
GPG: 0xC7C3A0DD


pgpTr9Yj9xv1t.pgp
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v2] backlight: let the administrator override clamping

2015-01-18 Thread Topi Miettinen 
On my computer, the minimum brightness enforced by clamping in
backlight is too bright.

Add a new option to override clamping in unit file. While at it, describe
the clamping in documentation.
---
 man/systemd-backli...@.service.xml | 27 +--
 src/backlight/backlight.c  | 11 ++-
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/man/systemd-backli...@.service.xml 
b/man/systemd-backli...@.service.xml
index 453afbf..0d30957 100644
--- a/man/systemd-backli...@.service.xml
+++ b/man/systemd-backli...@.service.xml
@@ -48,7 +48,17 @@
 
 refsynopsisdiv
 parafilenamesystemd-backlight@.service/filename/para
-
parafilename/usr/lib/systemd/systemd-backlight/filename/para
+cmdsynopsis
+commandsystemd-backlight/command
+arg choice=plainsave/arg
+arg choice=optreplaceablePATH/replaceable/arg
+/cmdsynopsis
+cmdsynopsis
+commandsystemd-backlight/command
+arg choice=plainload/arg
+arg choice=optreplaceablePATH/replaceable/arg
+arg 
choice=optreplaceable--allow-any-brightness/replaceable/arg
+/cmdsynopsis
 /refsynopsisdiv
 
 refsect1
@@ -58,7 +68,20 @@
 is a service that restores the display backlight
 brightness at early boot and saves it at shutdown. On
 disk, the backlight brightness is stored in
-filename/var/lib/systemd/backlight//filename./para
+filename/var/lib/systemd/backlight//filename. During
+loading, unless option
+replaceable--allow-any-brightness/replaceable is
+specified, the brightness is clamped to at least value
+literal1/literal or 5% of maximum brightness. This
+is to avoid an unpowered display due to poor API
+design in kernel (unfixed as of 2015-01-18) that does
+not allow user space to know the difference between
+lowest brightness and powering off the
+backlight./para
+
+parareplaceablePATH/replaceable identifies the
+display brightness control device. It is resolved by
+udev./para
 /refsect1
 
 refsect1
diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c
index 1271a66..96a25d8 100644
--- a/src/backlight/backlight.c
+++ b/src/backlight/backlight.c
@@ -255,7 +255,7 @@ static void clamp_brightness(struct udev_device *device, 
char **value, unsigned
 return;
 }
 
-log_info(Saved brightness %s %s to %s., old_value,
+log_info(Saved brightness %s %s to %s (use 
--allow-any-brightness to override)., old_value,
  new_brightness  brightness ?
  too low; increasing : too high; decreasing,
  *value);
@@ -272,8 +272,8 @@ int main(int argc, char *argv[]) {
 unsigned max_brightness;
 int r;
 
-if (argc != 3) {
-log_error(This program requires two arguments.);
+if (argc  3 || argc  4) {
+log_error(This program requires two or three arguments.);
 return EXIT_FAILURE;
 }
 
@@ -389,8 +389,9 @@ int main(int argc, char *argv[]) {
 log_error_errno(r, Failed to read %s: %m, saved);
 return EXIT_FAILURE;
 }
-
-clamp_brightness(device, value, max_brightness);
+/* Don't clamp brightness if asked */
+if (!(argc == 4  streq(argv[3], --allow-any-brightness)))
+clamp_brightness(device, value, max_brightness);
 
 r = udev_device_set_sysattr_value(device, brightness, value);
 if (r  0) {
-- 
2.1.4

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

[systemd-devel] [PATCH] timedated: support split usr v3

2015-01-18 Thread Shawn Landden 
From: Shawn Paul Landden sh...@churchofgit.com

The current Debian solution to this is really ugly, and I would rather
have them use the correct patch even if split usr is dumb.

Read: http://rusty.ozlabs.org/?p=236
 (Why Everyone Must Oppose The Merging of /usr and /)

(I managed to skip the pulseaudio implamentation mess because I
had a fancy emu10k1 SoundBlaster Live! 5.1 which does its own
hardware mixing.)

v3: revert 99f861310d3f05f4
---
 src/timedate/timedated.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index 753c3d1..3fbc24e 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -29,6 +29,7 @@
 #include sd-bus.h
 
 #include util.h
+#include copy.h
 #include strv.h
 #include def.h
 #include clock-util.h
@@ -95,6 +96,14 @@ static int context_read_data(Context *c) {
 }
 }
 
+#ifdef HAVE_SPLIT_USR
+r = read_one_line_file(/etc/timezone, c-zone);
+if (r  0) {
+if (r != -ENOENT)
+log_warning(Failed to read /etc/timezone: %s, 
strerror(-r));
+}
+#endif
+
 have_timezone:
 if (isempty(c-zone)) {
 free(c-zone);
@@ -123,9 +132,21 @@ static int context_write_data_timezone(Context *c) {
 if (!p)
 return log_oom();
 
+#ifdef HAVE_SPLIT_USR
+r = write_string_file_atomic(/etc/timezone, c-zone);
+if (r  0)
+return r;
+
+   /* /usr/sha... */
+r = copy_file((p + 2), /etc/localtime, O_TRUNC,
+S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH); /*644*/
+if (r  0)
+return r;
+#else
 r = symlink_atomic(p, /etc/localtime);
 if (r  0)
 return r;
+#endif
 
 return 0;
 }
-- 
2.1.4

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

[systemd-devel] [PATCH 03/11] tmpfiles: make sure not to concatenate non-absolute path

2015-01-18 Thread Zbigniew Jędrzejewski-Szmek 
If the path is absolute was only checked later.
Also do not check if path if absolute if we just
specified it starting with a slash.
---
 src/tmpfiles/tmpfiles.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 8811f27482..84d778a08f 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -1321,7 +1321,7 @@ static int parse_line(const char *fname, unsigned line, 
const char *buffer) {
 
 case CREATE_SYMLINK:
 if (!i-argument) {
-i-argument = strappend(/usr/share/factory, i-path);
+i-argument = strappend(/usr/share/factory/, 
i-path);
 if (!i-argument)
 return log_oom();
 }
@@ -1336,12 +1336,10 @@ static int parse_line(const char *fname, unsigned line, 
const char *buffer) {
 
 case COPY_FILES:
 if (!i-argument) {
-i-argument = strappend(/usr/share/factory, i-path);
+i-argument = strappend(/usr/share/factory/, 
i-path);
 if (!i-argument)
 return log_oom();
-}
-
-if (!path_is_absolute(i-argument)) {
+} else if (!path_is_absolute(i-argument)) {
 log_error([%s:%u] Source path is not absolute., 
fname, line);
 return -EBADMSG;
 }
-- 
1.8.4.652.g0d6e0ce

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

[systemd-devel] [PATCH 09/11] tmpfiles: use ACL magic on journal directories

2015-01-18 Thread Zbigniew Jędrzejewski-Szmek 
---
 README | 11 +++
 configure.ac   |  1 +
 tmpfiles.d/systemd.conf.m4 |  8 
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/README b/README
index fa95433ecb..c72209262e 100644
--- a/README
+++ b/README
@@ -178,14 +178,9 @@ USERS AND GROUPS:
 During runtime, the journal daemon requires the
 systemd-journal system group to exist. New journal files will
 be readable by this group (but not writable), which may be used
-to grant specific users read access.
-
-It is also recommended to grant read access to all journal
-files to the system groups wheel and adm with a command
-like the following in the post installation script of the
-package:
-
-# setfacl -nm g:wheel:rx,d:g:wheel:rx,g:adm:rx,d:g:adm:rx 
/var/log/journal/
+to grant specific users read access. In addition, system
+groups wheel and adm will be given read-only access to
+journal files using systemd-tmpfiles.service.
 
 The journal gateway daemon requires the
 systemd-journal-gateway system user and group to
diff --git a/configure.ac b/configure.ac
index 6d510df332..8205a677a8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -655,6 +655,7 @@ if test x${have_acl} != xno ; then
 if test x$have_acl = xyes ; then
 ACL_LIBS=-lacl
 AC_DEFINE(HAVE_ACL, 1, [ACL available])
+M4_DEFINES=$M4_DEFINES -DHAVE_ACL
 else
 have_acl=no
 fi
diff --git a/tmpfiles.d/systemd.conf.m4 b/tmpfiles.d/systemd.conf.m4
index ad05f43334..b447b01f58 100644
--- a/tmpfiles.d/systemd.conf.m4
+++ b/tmpfiles.d/systemd.conf.m4
@@ -26,9 +26,17 @@ d /run/log 0755 root root -
 
 z /run/log/journal 2755 root systemd-journal - -
 Z /run/log/journal/%m ~2750 root systemd-journal - -
+m4_ifdef(`HAVE_ACL',``
+a+ /run/log/journal/%m - - - - d:group:adm:r-x,d:group:wheel:r-x
+A+ /run/log/journal/%m - - - - group:adm:r-x,group:wheel:r-x
+'')m4_dnl
 
 z /var/log/journal 2755 root systemd-journal - -
 z /var/log/journal/%m 2755 root systemd-journal - -
+m4_ifdef(`HAVE_ACL',``
+a+ /var/log/journal/%m - - - - d:group:adm:r-x,d:group:wheel:r-x
+A+ /var/log/journal/%m - - - - group:adm:r-x,group:wheel:r-x
+'')m4_dnl
 
 d /var/lib/systemd 0755 root root -
 d /var/lib/systemd/coredump 0755 root root 3d
-- 
1.8.4.652.g0d6e0ce

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

[systemd-devel] [PATCH 01/11] tmpfiles: simplification

2015-01-18 Thread Zbigniew Jędrzejewski-Szmek 
Certain conditions were checked more than once. Warning message
is improved.
---
This is a sprawling set of changes to add ACL support to tmpfiles.
The patches are not entirely clean, you can see how my understanding
of the problem evolved :)

Comments, suggestions for improved syntax, etc., very much welcome.

Zbyszek

 src/tmpfiles/tmpfiles.c | 52 -
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 332ddcea76..91ae62da45 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -23,7 +23,6 @@
 #include fcntl.h
 #include errno.h
 #include string.h
-#include sys/stat.h
 #include limits.h
 #include dirent.h
 #include grp.h
@@ -34,10 +33,11 @@
 #include getopt.h
 #include stdbool.h
 #include time.h
-#include sys/types.h
-#include sys/param.h
 #include glob.h
 #include fnmatch.h
+#include sys/stat.h
+#include sys/types.h
+#include sys/param.h
 #include sys/xattr.h
 
 #include log.h
@@ -480,11 +480,9 @@ static int item_set_perms(Item *i, const char *path) {
 (i-uid_set || i-gid_set))
 if (chown(path,
   i-uid_set ? i-uid : UID_INVALID,
-  i-gid_set ? i-gid : GID_INVALID)  0) {
+  i-gid_set ? i-gid : GID_INVALID)  0)
 
-log_error_errno(errno, chown(%s) failed: %m, path);
-return -errno;
-}
+return log_error_errno(errno, chown(%s) failed: %m, 
path);
 
 return label_fix(path, false, false);
 }
@@ -495,36 +493,36 @@ static int get_xattrs_from_arg(Item *i) {
 int r;
 
 assert(i);
+assert(i-argument);
 
-if (!i-argument) {
-log_error(%s: Argument can't be empty!, i-path);
-return -EBADMSG;
-}
 p = i-argument;
 
 while ((r = unquote_first_word(p, xattr, false))  0) {
-_cleanup_free_ char *tmp = NULL, *name = NULL, *value = NULL;
+_cleanup_free_ char *tmp = NULL, *name = NULL,
+*value = NULL, *value2 = NULL, *_xattr = xattr;
+
 r = split_pair(xattr, =, name, value);
 if (r  0) {
 log_warning(Illegal xattr found: \%s\ - ignoring., 
xattr);
-free(xattr);
 continue;
 }
-free(xattr);
-if (streq(name, ) || streq(value, )) {
-log_warning(Malformed xattr found: \%s=%s\ - 
ignoring., name, value);
+
+if (strempty(name) || strempty(value)) {
+log_warning(Malformed xattr found: \%s\ - 
ignoring., xattr);
 continue;
 }
+
 tmp = unquote(value, \);
 if (!tmp)
 return log_oom();
-free(value);
-value = cunescape(tmp);
-if (!value)
+
+value2 = cunescape(tmp);
+if (!value2)
 return log_oom();
-if (strv_consume_pair(i-xattrs, name, value)  0)
+
+if (strv_push_pair(i-xattrs, name, value2)  0)
 return log_oom();
-name = value = NULL;
+name = value2 = NULL;
 }
 
 return r;
@@ -536,14 +534,13 @@ static int item_set_xattrs(Item *i, const char *path) {
 assert(i);
 assert(path);
 
-if (strv_isempty(i-xattrs))
-return 0;
-
 STRV_FOREACH_PAIR(name, value, i-xattrs) {
 int n;
+
 n = strlen(*value);
 if (lsetxattr(path, *name, *value, n, 0)  0) {
-log_error(Setting extended attribute %s=%s on %s 
failed: %m, *name, *value, path);
+log_error(Setting extended attribute %s=%s on %s 
failed: %m,
+  *name, *value, path);
 return -errno;
 }
 }
@@ -771,7 +768,7 @@ static int create_item(Item *i) {
 } else
 r = 0;
 
-if (i-type == CREATE_DIRECTORY || i-type == 
TRUNCATE_DIRECTORY || r == -ENOTTY) {
+if (IN_SET(i-type, CREATE_DIRECTORY, TRUNCATE_DIRECTORY) || r 
== -ENOTTY) {
 RUN_WITH_UMASK()
 r = mkdir_label(i-path, i-mode);
 }
@@ -1486,7 +1483,8 @@ static int parse_line(const char *fname, unsigned line, 
const char *buffer) {
 } else {
 /* Two identical items are fine */
 if (!item_equal(existing, i))
-log_warning(Two or more conflicting lines for 
%s configured, ignoring., i-path);
+

[systemd-devel] [PATCH 02/11] tmpfiles: detect all combinations of + and !

2015-01-18 Thread Zbigniew Jędrzejewski-Szmek 
The same algorithm as with - and @ in ExecStart= is used.
---
 src/tmpfiles/tmpfiles.c | 30 +-
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 91ae62da45..8811f27482 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -1238,9 +1238,9 @@ static int parse_line(const char *fname, unsigned line, 
const char *buffer) {
 _cleanup_free_ char *action = NULL, *mode = NULL, *user = NULL, *group 
= NULL, *age = NULL, *path = NULL;
 _cleanup_(item_freep) Item *i = NULL;
 Item *existing;
-char type;
 Hashmap *h;
-int r, n = -1;
+int r, n = -1, pos;
+bool force = false, boot = false;
 
 assert(fname);
 assert(line = 1);
@@ -1265,21 +1265,27 @@ static int parse_line(const char *fname, unsigned line, 
const char *buffer) {
 return -EINVAL;
 }
 
-if (strlen(action)  1  !in_charset(action+1, !+)) {
-log_error([%s:%u] Unknown modifiers in command '%s', fname, 
line, action);
-return -EINVAL;
+for (pos = 1; action[pos]; pos++) {
+if (action[pos] == '!'  !boot)
+boot = true;
+else if (action[pos] == '+'  !force)
+force = true;
+else {
+log_error([%s:%u] Unknown modifiers in command '%s',
+  fname, line, action);
+return -EINVAL;
+}
 }
 
-if (strchr(action+1, '!')  !arg_boot)
+if (boot  !arg_boot)
 return 0;
 
-type = action[0];
-
 i = new0(Item, 1);
 if (!i)
 return log_oom();
 
-i-force = !!strchr(action+1, '+');
+i-type = action[0];
+i-force = force;
 
 r = specifier_printf(path, specifier_table, NULL, i-path);
 if (r  0) {
@@ -1296,7 +1302,7 @@ static int parse_line(const char *fname, unsigned line, 
const char *buffer) {
 }
 }
 
-switch (type) {
+switch (i-type) {
 
 case CREATE_FILE:
 case TRUNCATE_FILE:
@@ -1372,12 +1378,10 @@ static int parse_line(const char *fname, unsigned line, 
const char *buffer) {
 break;
 
 default:
-log_error([%s:%u] Unknown command type '%c'., fname, line, 
type);
+log_error([%s:%u] Unknown command type '%c'., fname, line, 
i-type);
 return -EBADMSG;
 }
 
-i-type = type;
-
 if (!path_is_absolute(i-path)) {
 log_error([%s:%u] Path '%s' not absolute., fname, line, 
i-path);
 return -EBADMSG;
-- 
1.8.4.652.g0d6e0ce

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

[systemd-devel] [PATCH 06/11] tmpfiles: make recursive operation generic

2015-01-18 Thread Zbigniew Jędrzejewski-Szmek 
---
 src/tmpfiles/tmpfiles.c | 52 ++---
 1 file changed, 19 insertions(+), 33 deletions(-)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 7081b4dc57..d563989790 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -700,7 +700,9 @@ static int write_one_file(Item *i, const char *path) {
 return 0;
 }
 
-static int item_set_perms_children(Item *i, const char *path) {
+typedef int (*action_t)(Item *, const char *);
+
+static int item_do_children(Item *i, const char *path, action_t action) {
 _cleanup_closedir_ DIR *d;
 int r = 0;
 
@@ -735,12 +737,12 @@ static int item_set_perms_children(Item *i, const char 
*path) {
 if (!p)
 return -ENOMEM;
 
-q = item_set_perms(i, p);
+q = action(i, p);
 if (q  0  q != -ENOENT  r == 0)
 r = q;
 
 if (IN_SET(de-d_type, DT_UNKNOWN, DT_DIR)) {
-q = item_set_perms_children(i, p);
+q = item_do_children(i, p, action);
 if (q  0  r == 0)
 r = q;
 }
@@ -749,42 +751,26 @@ static int item_set_perms_children(Item *i, const char 
*path) {
 return r;
 }
 
-static int item_set_perms_recursive(Item *i, const char *path) {
-int r, q;
-
-assert(i);
-assert(path);
-
-r = item_set_perms(i, path);
-if (r  0)
-return r;
-
-q = item_set_perms_children(i, path);
-if (q  0  r == 0)
-r = q;
-
-return r;
-}
-
-static int glob_item(Item *i, int (*action)(Item *, const char *)) {
+static int glob_item(Item *i, action_t action, bool recursive) {
 _cleanup_globfree_ glob_t g = {};
 int r = 0, k;
 char **fn;
 
 errno = 0;
 k = glob(i-path, GLOB_NOSORT|GLOB_BRACE, NULL, g);
-if (k != 0  k != GLOB_NOMATCH) {
-if (errno == 0)
-errno = EIO;
-
-log_error_errno(errno, glob(%s) failed: %m, i-path);
-return -errno;
-}
+if (k != 0  k != GLOB_NOMATCH)
+return log_error_errno(errno ?: EIO, glob(%s) failed: %m, 
i-path);
 
 STRV_FOREACH(fn, g.gl_pathv) {
 k = action(i, *fn);
 if (k  0  r == 0)
 r = k;
+
+if (recursive) {
+k = item_do_children(i, *fn, action);
+if (k  0  r == 0)
+r = k;
+}
 }
 
 return r;
@@ -838,7 +824,7 @@ static int create_item(Item *i) {
 break;
 
 case WRITE_FILE:
-r = glob_item(i, write_one_file);
+r = glob_item(i, write_one_file, false);
 if (r  0)
 return r;
 
@@ -1016,14 +1002,14 @@ static int create_item(Item *i) {
 case ADJUST_MODE:
 case RELABEL_PATH:
 
-r = glob_item(i, item_set_perms);
+r = glob_item(i, item_set_perms, false);
 if (r  0)
 return r;
 break;
 
 case RECURSIVE_RELABEL_PATH:
 
-r = glob_item(i, item_set_perms_recursive);
+r = glob_item(i, item_set_perms, true);
 if (r  0)
 return r;
 break;
@@ -1120,7 +1106,7 @@ static int remove_item(Item *i) {
 case REMOVE_PATH:
 case TRUNCATE_DIRECTORY:
 case RECURSIVE_REMOVE_PATH:
-r = glob_item(i, remove_item_instance);
+r = glob_item(i, remove_item_instance, false);
 break;
 }
 
@@ -1187,7 +1173,7 @@ static int clean_item(Item *i) {
 clean_item_instance(i, i-path);
 break;
 case IGNORE_DIRECTORY_PATH:
-r = glob_item(i, clean_item_instance);
+r = glob_item(i, clean_item_instance, false);
 break;
 default:
 break;
-- 
1.8.4.652.g0d6e0ce

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

[systemd-devel] [PATCH 05/11] tmpfiles: add 'a' type to set ACLs

2015-01-18 Thread Zbigniew Jędrzejewski-Szmek 
---
 Makefile.am   |  5 +++
 man/tmpfiles.d.xml| 32 -
 src/journal/coredump.c|  6 +---
 src/journal/journalctl.c  |  6 +---
 src/journal/journald-server.c |  5 ---
 src/login/logind-acl.c|  2 --
 src/shared/acl-util.c | 66 +--
 src/shared/acl-util.h | 19 ++
 src/tmpfiles/tmpfiles.c   | 81 +++
 9 files changed, 188 insertions(+), 34 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index ce5ebf7c48..d8b443e9dc 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2189,6 +2189,11 @@ systemd_tmpfiles_LDADD = \
libsystemd-internal.la \
libsystemd-shared.la
 
+if HAVE_ACL
+systemd_tmpfiles_LDADD += \
+   libsystemd-acl.la
+endif
+
 rootbin_PROGRAMS += \
systemd-tmpfiles
 
diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 8d806a41ea..7c1ef42c20 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -292,6 +292,13 @@
   path. This can be useful for setting SMACK labels.
   /para/listitem
 /varlistentry
+
+varlistentry
+  termvarnamea/varname/term
+  listitemparaSet POSIX ACLs (access control lists) on the
+  specified path. This can be useful for allowing aditional
+  access to certain files./para/listitem
+/varlistentry
   /variablelist
 
   paraIf the exclamation mark is used, this line is only safe of
@@ -374,8 +381,8 @@
   if omitted or when set to literal-/literal, the file access
   mode will not be modified. This parameter is ignored for
   varnamex/varname, varnamer/varname,
-  varnameR/varname, varnameL/varname, varnamet/varname
-  lines./para
+  varnameR/varname, varnameL/varname, varnamet/varname,
+  and varnamea/varname lines./para
 
   paraOptionally, if prefixed with literal~/literal, the
   access mode is masked based on the already set access bits for
@@ -397,11 +404,12 @@
   may either be a numeric user/group ID or a user or group
   name. If omitted or when set to literal-/literal, the
   default 0 (root) is used. For varnamez/varname,
-  varnameZ/varname lines, when omitted or when set to -, the
-  file ownership will not be modified. These parameters are
-  ignored for varnamex/varname, varnamer/varname,
-  varnameR/varname, varnameL/varname, varnamet/varname
-  lines./para
+  varnameZ/varname lines, when omitted or when set to
+  literal-/literal, the file ownership will not be
+  modified. These parameters are ignored for varnamex/varname,
+  varnamer/varname, varnameR/varname,
+  varnameL/varname, varnamet/varname, and
+  varnamea/varname lines./para
 /refsect2
 
 refsect2
@@ -458,7 +466,8 @@
   is written to the file, suffixed by a newline. For
   varnameC/varname, specifies the source file or
   directory. For varnamet/varname determines extended
-  attributes to be set. Ignored for all other lines./para
+  attributes to be set. For varnamea/varname determines
+  ACL attributes to be set. Ignored for all other lines./para
 /refsect2
 
   /refsect1
@@ -489,7 +498,12 @@
   
citerefentryrefentrytitlesystemd/refentrytitlemanvolnum1/manvolnum/citerefentry,
   
citerefentryrefentrytitlesystemd-tmpfiles/refentrytitlemanvolnum8/manvolnum/citerefentry,
   
citerefentryrefentrytitlesystemd-delta/refentrytitlemanvolnum1/manvolnum/citerefentry,
-  
citerefentryrefentrytitlesystemd.exec/refentrytitlemanvolnum5/manvolnum/citerefentry
+  
citerefentryrefentrytitlesystemd.exec/refentrytitlemanvolnum5/manvolnum/citerefentry,
+  citerefentry 
project='man-pages'refentrytitleattr/refentrytitlemanvolnum5/manvolnum/citerefentry,
+  citerefentry 
project='man-pages'refentrytitlegetfattr/refentrytitlemanvolnum1/manvolnum/citerefentry,
+  citerefentry 
project='man-pages'refentrytitlesetfattr/refentrytitlemanvolnum1/manvolnum/citerefentry,
+  citerefentry 
project='man-pages'refentrytitlesetfacl/refentrytitlemanvolnum1/manvolnum/citerefentry,
+  citerefentry 
project='man-pages'refentrytitlegetfacl/refentrytitlemanvolnum1/manvolnum/citerefentry
 /para
   /refsect1
 
diff --git a/src/journal/coredump.c b/src/journal/coredump.c
index a37e5eb8a7..d322e7984c 100644
--- a/src/journal/coredump.c
+++ b/src/journal/coredump.c
@@ -49,11 +49,7 @@
 #include path-util.h
 #include compress.h
 #include coredump-vacuum.h
-
-#ifdef HAVE_ACL
-#  include sys/acl.h
-#  include acl-util.h
-#endif
+#include acl-util.h
 
 /* The maximum size up to which we process coredumps */
 #define PROCESS_SIZE_MAX ((off_t) (2LLU*1024LLU*1024LLU*1024LLU))
diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index db9576c493..3d890ffb61 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -37,17 +37,13 @@
 #include sys/inotify.h
 #include linux/fs.h
 
-#ifdef HAVE_ACL

[systemd-devel] [PATCH 11/11] TODO: tmpfiles

2015-01-18 Thread Zbigniew Jędrzejewski-Szmek 
---
 TODO | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/TODO b/TODO
index d443209414..4d0fb06dbf 100644
--- a/TODO
+++ b/TODO
@@ -682,6 +682,8 @@ Features:
 
 * tmpfiles:
   - apply x on D too (see patch from William Douglas)
+  - replace F with f+.
+  - instead of ignoring unknown fields, reject them.
 
 * for services: do not set $HOME in services unless requested
 
-- 
1.8.4.652.g0d6e0ce

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

[systemd-devel] [PATCH 10/11] shared/acl-util: add mask only when needed, always add base ACLs

2015-01-18 Thread Zbigniew Jędrzejewski-Szmek 
For ACLs to be valid, a set of entries for user, group, and other
must be always present. Always add those entries.

While at it, only add the mask ACL if it is actually required, i.e.
when at least on ACL for non-owner group or user exists.
---
 man/tmpfiles.d.xml  |  20 +-
 src/shared/acl-util.c   | 102 ++--
 src/shared/acl-util.h   |   1 +
 src/tmpfiles/tmpfiles.c |  25 +---
 4 files changed, 112 insertions(+), 36 deletions(-)

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 957910dd6d..8815bf9970 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -306,16 +306,16 @@
   termvarnamea/varname/term
   termvarnamea+/varname/term
   listitemparaSet POSIX ACLs (access control lists). If
-  suffixed with varname+/varname, specified mask will be
-  added to existing
-  entries. commandsystemd-tmpfiles/command does not
-  automatically add the required base entries for user and
-  group to the specified mask, so they must be specified
-  explicitly if varname+/varname is not used. The
-  mask will be added if not specified explicitly.
-  Lines of this type accept shell-style globs in place
-  of normal path names. This can be useful for allowing
-  additional access to certain files.  /para/listitem
+  suffixed with varname+/varname, specified entries will
+  be added to the existing set.
+  commandsystemd-tmpfiles/command will automatically add
+  the required base entries for user and group based on the
+  access mode of the file, unless base entries already exist
+  or are explictly specified. The mask will be added if not
+  specified explicitly or already present. Lines of this type
+  accept shell-style globs in place of normal path names. This
+  can be useful for allowing additional access to certain
+  files./para/listitem
 /varlistentry
 
 varlistentry
diff --git a/src/shared/acl-util.c b/src/shared/acl-util.c
index 950f472ddd..a4ff1ab878 100644
--- a/src/shared/acl-util.c
+++ b/src/shared/acl-util.c
@@ -29,14 +29,14 @@
 
 int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *entry) {
 acl_entry_t i;
-int found;
+int r;
 
 assert(acl);
 assert(entry);
 
-for (found = acl_get_entry(acl, ACL_FIRST_ENTRY, i);
- found  0;
- found = acl_get_entry(acl, ACL_NEXT_ENTRY, i)) {
+for (r = acl_get_entry(acl, ACL_FIRST_ENTRY, i);
+ r  0;
+ r = acl_get_entry(acl, ACL_NEXT_ENTRY, i)) {
 
 acl_tag_t tag;
 uid_t *u;
@@ -60,8 +60,7 @@ int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *entry) {
 return 1;
 }
 }
-
-if (found  0)
+if (r  0)
 return -errno;
 
 return 0;
@@ -69,14 +68,13 @@ int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *entry) {
 
 int calc_acl_mask_if_needed(acl_t *acl_p) {
 acl_entry_t i;
-int found;
+int r;
 
 assert(acl_p);
 
-for (found = acl_get_entry(*acl_p, ACL_FIRST_ENTRY, i);
- found  0;
- found = acl_get_entry(*acl_p, ACL_NEXT_ENTRY, i)) {
-
+for (r = acl_get_entry(*acl_p, ACL_FIRST_ENTRY, i);
+ r  0;
+ r = acl_get_entry(*acl_p, ACL_NEXT_ENTRY, i)) {
 acl_tag_t tag;
 
 if (acl_get_tag_type(i, tag)  0)
@@ -84,14 +82,80 @@ int calc_acl_mask_if_needed(acl_t *acl_p) {
 
 if (tag == ACL_MASK)
 return 0;
+if (IN_SET(tag, ACL_USER, ACL_GROUP))
+goto calc;
 }
-
-if (found  0)
+if (r  0)
 return -errno;
+return 0;
 
+calc:
 if (acl_calc_mask(acl_p)  0)
 return -errno;
+return 1;
+}
+
+int add_base_acls_if_needed(acl_t *acl_p, const char *path) {
+acl_entry_t i;
+int r;
+bool have_user_obj = false, have_group_obj = false, have_other = false;
+struct stat st;
+_cleanup_(acl_freep) acl_t basic = NULL;
+
+assert(acl_p);
+
+for (r = acl_get_entry(*acl_p, ACL_FIRST_ENTRY, i);
+ r  0;
+ r = acl_get_entry(*acl_p, ACL_NEXT_ENTRY, i)) {
+acl_tag_t tag;
+
+if (acl_get_tag_type(i, tag)  0)
+return -errno;
+
+if (tag == ACL_USER_OBJ)
+have_user_obj = true;
+else if (tag == ACL_GROUP_OBJ)
+have_group_obj = true;
+else if (tag == ACL_OTHER)
+have_other = true;
+if (have_user_obj  have_group_obj  have_other)
+return 0;
+}
+if (r

[systemd-devel] [PATCH 07/11] tmpfiles: make t and a globby, add their recursive versions T and A

2015-01-18 Thread Zbigniew Jędrzejewski-Szmek 
For types which adapt existing files it is generally more useful to accept
globs.

In analogy to z and Z, add recursive versions using uppercase letters.

Technically, making a accept globs is backwards incompatible, but in
practice it probably isn't yet widely used and we can assume that most
people don't create files with wildcards in names.

Functions which are used as callbacks, but not directly on items, are
renamed not to have item_ prefix.
---
 man/tmpfiles.d.xml  | 31 +--
 src/tmpfiles/tmpfiles.c | 57 ++---
 2 files changed, 65 insertions(+), 23 deletions(-)

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 7c1ef42c20..ee33afcf6b 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -273,7 +273,7 @@
   listitemparaAdjust the access mode, group and user, and
   restore the SELinux security context of a file or directory,
   if it exists. Lines of this type accept shell-style globs in
-  place of normal path names.  /para/listitem
+  place of normal path names./para/listitem
 /varlistentry
 
 varlistentry
@@ -288,16 +288,35 @@
 
 varlistentry
   termvarnamet/varname/term
-  listitemparaSet extended attributes on the specified
-  path. This can be useful for setting SMACK labels.
+  listitemparaSet extended attributes. Lines of this type
+  accept shell-style globs in place of normal path names.
+  This can be useful for setting SMACK labels.
+  /para/listitem
+/varlistentry
+
+varlistentry
+  termvarnameT/varname/term
+  listitemparaRecursively set extended attributes. Lines
+  of this type accept shell-style globs in place of normal
+  path names.  This can be useful for setting SMACK labels.
   /para/listitem
 /varlistentry
 
 varlistentry
   termvarnamea/varname/term
-  listitemparaSet POSIX ACLs (access control lists) on the
-  specified path. This can be useful for allowing aditional
-  access to certain files./para/listitem
+  listitemparaSet POSIX ACLs (access control lists).
+  Lines of this type accept shell-style globs in
+  place of normal path names. This can be useful for
+  allowing additional access to certain files.
+  /para/listitem
+/varlistentry
+
+varlistentry
+  termvarnameA/varname/term
+  listitemparaRecursively set POSIX ACLs. Lines of this
+  type accept shell-style globs in place of normal path
+  names. This can be useful for allowing additional access to
+  certain files./para/listitem
 /varlistentry
   /variablelist
 
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index d563989790..44a087807e 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -76,10 +76,12 @@ typedef enum ItemType {
 CREATE_CHAR_DEVICE = 'c',
 CREATE_BLOCK_DEVICE = 'b',
 COPY_FILES = 'C',
-SET_XATTR = 't',
-SET_ACL = 'a',
 
 /* These ones take globs */
+SET_XATTR = 't',
+RECURSIVE_SET_XATTR = 'T',
+SET_ACL = 'a',
+RECURSIVE_SET_ACL = 'A',
 WRITE_FILE = 'w',
 IGNORE_PATH = 'x',
 IGNORE_DIRECTORY_PATH = 'X',
@@ -151,7 +153,11 @@ static bool needs_glob(ItemType t) {
   RECURSIVE_REMOVE_PATH,
   ADJUST_MODE,
   RELABEL_PATH,
-  RECURSIVE_RELABEL_PATH);
+  RECURSIVE_RELABEL_PATH,
+  SET_XATTR,
+  RECURSIVE_SET_XATTR,
+  SET_ACL,
+  RECURSIVE_SET_ACL);
 }
 
 static bool takes_ownership(ItemType t) {
@@ -486,7 +492,7 @@ finish:
 return r;
 }
 
-static int item_set_perms(Item *i, const char *path) {
+static int path_set_perms(Item *i, const char *path) {
 struct stat st;
 bool st_valid;
 
@@ -568,7 +574,7 @@ static int get_xattrs_from_arg(Item *i) {
 return r;
 }
 
-static int item_set_xattrs(Item *i, const char *path) {
+static int path_set_xattrs(Item *i, const char *path) {
 char **name, **value;
 
 assert(i);
@@ -605,7 +611,7 @@ static int get_acls_from_arg(Item *item) {
 return 0;
 }
 
-static int item_set_acl(Item *item, const char *path) {
+static int path_set_acls(Item *item, const char *path) {
 #ifdef HAVE_ACL
 int r;
 
@@ -693,7 +699,7 @@ static int write_one_file(Item *i, const char *path) {
 return -EEXIST;
 }
 
-r = item_set_perms(i, path);
+r = path_set_perms(i, path);
 if (r  0)
 return r;
 
@@ -817,7 +823,7 @@ static int create_item(Item *i) {
 }
 }
 
-r = item_set_perms(i, i-path);
+

[systemd-devel] [PATCH 04/11] tmpfiles: attach an array of items to each path

2015-01-18 Thread Zbigniew Jędrzejewski-Szmek 
The data structure used by tmpfiles is changed: instead of hashmaps
mapping {path → Item*} we now have hashmaps containing
{path - ItemArray}, where ItemArray contains a pointer
to an array of Items.

For current code it doesn't matter much, but when we add new types it
is easier to simply add a new Item for a given path, then to coalesce
multiple lines into one Item.

In the future, this change will also make it possible to remember the
file and line where each Item originates, and use that in reporting
errors. Currently this is not possible, since each Item can be created
from multiple lines.
---
 man/tmpfiles.d.xml  |  13 +-
 src/tmpfiles/tmpfiles.c | 308 
 2 files changed, 160 insertions(+), 161 deletions(-)

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 9fd5913d83..8d806a41ea 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -288,16 +288,9 @@
 
 varlistentry
   termvarnamet/varname/term
-  listitemparaSet extended attributes on item. It may be
-  used in conjunction with other types (only
-  varnamed/varname, varnameD/varname,
-  varnamef/varname, varnameF/varname,
-  varnameL/varname, varnamep/varname,
-  varnamec/varname, varnameb/varname, makes sense).
-  If used as a standalone line, then
-  commandsystemd-tmpfiles/command will try to set extended
-  attributes on specified path.  This can be especially used
-  to set SMACK labels.  /para/listitem
+  listitemparaSet extended attributes on the specified
+  path. This can be useful for setting SMACK labels.
+  /para/listitem
 /varlistentry
   /variablelist
 
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 84d778a08f..c44dfaf1d2 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -4,6 +4,7 @@
   This file is part of systemd.
 
   Copyright 2010 Lennart Poettering, Kay Sievers
+  Copyright 2015 Zbigniew Jędrzejewski-Szmek
 
   systemd is free software; you can redistribute it and/or modify it
   under the terms of the GNU Lesser General Public License as published by
@@ -113,6 +114,12 @@ typedef struct Item {
 bool done:1;
 } Item;
 
+typedef struct ItemArray {
+Item *items;
+size_t count;
+size_t size;
+} ItemArray;
+
 static bool arg_create = false;
 static bool arg_clean = false;
 static bool arg_remove = false;
@@ -141,13 +148,40 @@ static bool needs_glob(ItemType t) {
   RECURSIVE_RELABEL_PATH);
 }
 
+static bool takes_ownership(ItemType t) {
+return IN_SET(t,
+  CREATE_FILE,
+  TRUNCATE_FILE,
+  CREATE_DIRECTORY,
+  TRUNCATE_DIRECTORY,
+  CREATE_SUBVOLUME,
+  CREATE_FIFO,
+  CREATE_SYMLINK,
+  CREATE_CHAR_DEVICE,
+  CREATE_BLOCK_DEVICE,
+  COPY_FILES,
+
+  WRITE_FILE,
+  IGNORE_PATH,
+  IGNORE_DIRECTORY_PATH,
+  REMOVE_PATH,
+  RECURSIVE_REMOVE_PATH);
+}
+
 static struct Item* find_glob(Hashmap *h, const char *match) {
-Item *j;
+ItemArray *j;
 Iterator i;
 
-HASHMAP_FOREACH(j, h, i)
-if (fnmatch(j-path, match, FNM_PATHNAME|FNM_PERIOD) == 0)
-return j;
+HASHMAP_FOREACH(j, h, i) {
+unsigned n;
+
+for (n = 0; n  j-count; n++) {
+Item *item = j-items + n;
+
+if (fnmatch(item-path, match, 
FNM_PATHNAME|FNM_PERIOD) == 0)
+return item;
+}
+}
 
 return NULL;
 }
@@ -604,10 +638,6 @@ static int write_one_file(Item *i, const char *path) {
 if (r  0)
 return r;
 
-r = item_set_xattrs(i, i-path);
-if (r  0)
-return r;
-
 return 0;
 }
 
@@ -790,10 +820,6 @@ static int create_item(Item *i) {
 if (r  0)
 return r;
 
-r = item_set_xattrs(i, i-path);
-if (r  0)
-return r;
-
 break;
 
 case CREATE_FIFO:
@@ -834,10 +860,6 @@ static int create_item(Item *i) {
 if (r  0)
 return r;
 
-r = item_set_xattrs(i, i-path);
-if (r  0)
-return r;
-
 break;
 
 case CREATE_SYMLINK:
@@ -869,10 +891,6 @@ static int create_item(Item *i) {
 }
 }
 
-r = item_set_xattrs(i, i-path);
-if (r  0)
-   return r;
-
 break;
 
 case CREATE_BLOCK_DEVICE:
@@ -933,10

[systemd-devel] [PATCH 08/11] tmpfiles: implement augmenting of existing ACLs

2015-01-18 Thread Zbigniew Jędrzejewski-Szmek 
This is much more useful in practice (equivalent to setfacl -m).
---
 man/tmpfiles.d.xml  | 28 +++--
 src/shared/acl-util.c   | 49 +---
 src/shared/acl-util.h   |  3 ++-
 src/tmpfiles/tmpfiles.c | 54 -
 4 files changed, 96 insertions(+), 38 deletions(-)

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index ee33afcf6b..957910dd6d 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -234,7 +234,7 @@
   to exclude paths from clean-up as controlled with the Age
   parameter. Note that lines of this type do not influence the
   effect of varnamer/varname or varnameR/varname
-  lines.  Lines of this type accept shell-style globs in place
+  lines. Lines of this type accept shell-style globs in place
   of normal path names.  /para/listitem
 /varlistentry
 
@@ -246,7 +246,7 @@
   not exclude the content if path is a directory, but only
   directory itself. Note that lines of this type do not
   influence the effect of varnamer/varname or
-  varnameR/varname lines.  Lines of this type accept
+  varnameR/varname lines. Lines of this type accept
   shell-style globs in place of normal path names.
   /para/listitem
 /varlistentry
@@ -304,19 +304,25 @@
 
 varlistentry
   termvarnamea/varname/term
-  listitemparaSet POSIX ACLs (access control lists).
-  Lines of this type accept shell-style globs in
-  place of normal path names. This can be useful for
-  allowing additional access to certain files.
-  /para/listitem
+  termvarnamea+/varname/term
+  listitemparaSet POSIX ACLs (access control lists). If
+  suffixed with varname+/varname, specified mask will be
+  added to existing
+  entries. commandsystemd-tmpfiles/command does not
+  automatically add the required base entries for user and
+  group to the specified mask, so they must be specified
+  explicitly if varname+/varname is not used. The
+  mask will be added if not specified explicitly.
+  Lines of this type accept shell-style globs in place
+  of normal path names. This can be useful for allowing
+  additional access to certain files.  /para/listitem
 /varlistentry
 
 varlistentry
   termvarnameA/varname/term
-  listitemparaRecursively set POSIX ACLs. Lines of this
-  type accept shell-style globs in place of normal path
-  names. This can be useful for allowing additional access to
-  certain files./para/listitem
+  termvarnameA+/varname/term
+  listitemparaSame as varnamea/varname and
+  varnamea+/varname, but recursive./para/listitem
 /varlistentry
   /variablelist
 
diff --git a/src/shared/acl-util.c b/src/shared/acl-util.c
index 22bb8444e5..950f472ddd 100644
--- a/src/shared/acl-util.c
+++ b/src/shared/acl-util.c
@@ -150,7 +150,7 @@ int search_acl_groups(char*** dst, const char* path, bool* 
belong) {
 return 0;
 }
 
-int parse_acl(char *text, acl_t *acl_access, acl_t *acl_default) {
+int parse_acl(char *text, acl_t *acl_access, acl_t *acl_default, bool 
want_mask) {
 _cleanup_free_ char **a = NULL, **d = NULL; /* strings are not be 
freed */
 _cleanup_strv_free_ char **split;
 char **entry;
@@ -187,9 +187,11 @@ int parse_acl(char *text, acl_t *acl_access, acl_t 
*acl_default) {
 if (!a_acl)
 return -EINVAL;
 
-r = calc_acl_mask_if_needed(a_acl);
-if (r  0)
-return r;
+if (want_mask) {
+r = calc_acl_mask_if_needed(a_acl);
+if (r  0)
+return r;
+}
 }
 
 if (!strv_isempty(d)) {
@@ -203,9 +205,11 @@ int parse_acl(char *text, acl_t *acl_access, acl_t 
*acl_default) {
 if (!d_acl)
 return -EINVAL;
 
-r = calc_acl_mask_if_needed(d_acl);
-if (r  0)
-return r;
+if (want_mask) {
+r = calc_acl_mask_if_needed(d_acl);
+if (r  0)
+return r;
+}
 }
 
 *acl_access = a_acl;
@@ -213,3 +217,34 @@ int parse_acl(char *text, acl_t *acl_access, acl_t 
*acl_default) {
 a_acl = d_acl = NULL;
 return 0;
 }
+
+int acls_for_file(const char *path, acl_type_t type, acl_t new, acl_t *acl) {
+_cleanup_(acl_freep) acl_t old;
+acl_entry_t i;
+int found, r;
+
+old = acl_get_file(path, type);
+if (!old)
+return -errno;
+
+for (found = acl_get_entry(new,

[systemd-devel] [PATCH] libudev: fix check for too long packet

2015-01-18 Thread Topi Miettinen 
Don't use recvmsg(2) return value to check for too long packets
(it doesn't work) but MSG_TRUNC flag.
---
 src/libudev/libudev-monitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libudev/libudev-monitor.c b/src/libudev/libudev-monitor.c
index 484fefe..d8e551b 100644
--- a/src/libudev/libudev-monitor.c
+++ b/src/libudev/libudev-monitor.c
@@ -609,7 +609,7 @@ retry:
 return NULL;
 }
 
-if (buflen  32 || (size_t)buflen = sizeof(buf)) {
+if (buflen  32 || smsg.msg_flags  MSG_TRUNC) {
 log_debug(invalid message length);
 return NULL;
 }
-- 
2.1.4

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

Re: [systemd-devel] [PATCH] timedated: support split usr v3

2015-01-18 Thread Kay Sievers 
On Sun, Jan 18, 2015 at 7:53 PM, Shawn Landden sh...@churchofgit.com wrote:
 From: Shawn Paul Landden sh...@churchofgit.com

 The current Debian solution to this is really ugly, and I would rather
 have them use the correct patch even if split usr is dumb.

Please keep this local to the distro, this is no upstream material.
/etc/timezon is redundant information in a separate file. We do not
want to support this thing in systemd.

The split /usr support is pretty much limited to locations of files,
but should not change fundamental logic or introduce new concepts or
config files.

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

Re: [systemd-devel] Suspicious assertions in resolved

2015-01-18 Thread Topi Miettinen 
On 01/18/15 20:45, David Herrmann wrote:
 Hi
 
 On Sun, Jan 18, 2015 at 8:12 PM, Topi Miettinen toiwo...@gmail.com wrote:
 Hello,

 I think resolved_manager.c function manager_recv() has an assertion that
 could be triggerable by the server sending an oversized packet:

 assert(!(mh.msg_flags  MSG_TRUNC));

 The other assertions look suspicious too but I don't know if they can
 really be triggered by the other side.
 
 We use FIONREAD to read the size of the next pending datagram.
 Therefore, MSG_TRUNC cannot be set. Similarly, we provide suitable
 control-data space so MSG_CTRUNC cannot be set, either.

OK. What about the assertions later, is it possible to receive a reply
via IPv6 for IPv4 request or the other way around?

 
 Thanks
 David
 
 I'd propose something like this:

 diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
 index 0594479..b1defa3 100644
 --- a/src/resolve/resolved-manager.c
 +++ b/src/resolve/resolved-manager.c
 @@ -894,7 +894,8 @@ int manager_recv(Manager *m, int fd, DnsProtocol
 protocol, DnsPacket **ret) {
  return -EIO;

  assert(!(mh.msg_flags  MSG_CTRUNC));
 -assert(!(mh.msg_flags  MSG_TRUNC));
 +if (mh.msg_flags  MSG_TRUNC)
 +return -EIO;

  p-size = (size_t) l;


 -Topi
 ___
 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] Suspicious assertions in resolved

2015-01-18 Thread David Herrmann 
Hi

On Sun, Jan 18, 2015 at 10:15 PM, Topi Miettinen toiwo...@gmail.com wrote:
 On 01/18/15 20:45, David Herrmann wrote:
 Hi

 On Sun, Jan 18, 2015 at 8:12 PM, Topi Miettinen toiwo...@gmail.com wrote:
 Hello,

 I think resolved_manager.c function manager_recv() has an assertion that
 could be triggerable by the server sending an oversized packet:

 assert(!(mh.msg_flags  MSG_TRUNC));

 The other assertions look suspicious too but I don't know if they can
 really be triggered by the other side.

 We use FIONREAD to read the size of the next pending datagram.
 Therefore, MSG_TRUNC cannot be set. Similarly, we provide suitable
 control-data space so MSG_CTRUNC cannot be set, either.

 OK. What about the assertions later, is it possible to receive a reply
 via IPv6 for IPv4 request or the other way around?

Those asserts verify that the CMSG socket-type is the same as the
PACKET/PEER socket-type. I don't see how this looks at the
request-type? Those assert()s look good to me.

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

Re: [systemd-devel] [PATCH 3/3] remove RUN_WITH_LOCALE et all, use extended locale functions instead

2015-01-18 Thread David Herrmann 
Hi

On Thu, Jan 15, 2015 at 6:27 AM, Cristian Rodríguez
crrodrig...@opensuse.org wrote:
 There were two callers, one can use strtod_l() and the other strptime_l()
 ---
  src/import/curl-util.c | 38 +++---
  src/shared/util.c  | 18 +-
  src/shared/util.h  | 26 --
  3 files changed, 36 insertions(+), 46 deletions(-)

I fixed up the whitespace (8chars instead of 4) and other coding-style
issues and pushed the patch.

Thanks
David

 diff --git a/src/import/curl-util.c b/src/import/curl-util.c
 index 0c6c867..bbb68f7 100644
 --- a/src/import/curl-util.c
 +++ b/src/import/curl-util.c
 @@ -418,27 +418,35 @@ int curl_header_strdup(const void *contents, size_t sz, 
 const char *field, char
  int curl_parse_http_time(const char *t, usec_t *ret) {
  struct tm tm;
  time_t v;
 +const char *e;
 +locale_t loc;

  assert(t);
  assert(ret);

 -RUN_WITH_LOCALE(LC_TIME, C) {
 -const char *e;
 -
 -/* RFC822 */
 -e = strptime(t, %a, %d %b %Y %H:%M:%S %Z, tm);
 -if (!e || *e != 0)
 -/* RFC 850 */
 -e = strptime(t, %A, %d-%b-%y %H:%M:%S %Z, tm);
 -if (!e || *e != 0)
 -/* ANSI C */
 -e = strptime(t, %a %b %d %H:%M:%S %Y, tm);
 -if (!e || *e != 0)
 -return -EINVAL;
 -
 -v = timegm(tm);
 +loc = newlocale (LC_TIME_MASK, C, (locale_t)0);
 +
 +if (loc == (locale_t) 0)
 +return -errno;
 +
 +/* RFC822 */
 +e = strptime_l(t, %a, %d %b %Y %H:%M:%S %Z, tm, loc);
 +
 +if (!e || *e != 0)
 +/* RFC 850 */
 +e = strptime_l(t, %A, %d-%b-%y %H:%M:%S %Z, tm, loc);
 +if (!e || *e != 0)
 +/* ANSI C */
 +e = strptime_l(t, %a %b %d %H:%M:%S %Y, tm, loc);
 +if (!e || *e != 0) {
 +freelocale(loc);
 +return -EINVAL;
  }

 +freelocale(loc);
 +
 +v = timegm(tm);
 +
  if (v == (time_t) -1)
  return -EINVAL;

 diff --git a/src/shared/util.c b/src/shared/util.c
 index 884e782..599f3ca 100644
 --- a/src/shared/util.c
 +++ b/src/shared/util.c
 @@ -507,19 +507,27 @@ int safe_atolli(const char *s, long long int *ret_lli) {
  int safe_atod(const char *s, double *ret_d) {
  char *x = NULL;
  double d = 0;
 +locale_t loc;

  assert(s);
  assert(ret_d);

 -RUN_WITH_LOCALE(LC_NUMERIC_MASK, C) {
 -errno = 0;
 -d = strtod(s, x);
 -}
 +loc = newlocale (LC_NUMERIC_MASK, C, (locale_t)0);

 -if (!x || x == s || *x || errno)
 +if(loc == (locale_t)0)
 +return -errno;
 +
 +errno = 0;
 +d = strtod_l(s, x, loc);
 +
 +if (!x || x == s || *x || errno) {
 +freelocale(loc);
  return errno ? -errno : -EINVAL;
 +}

  *ret_d = (double) d;
 +freelocale(loc);
 +
  return 0;
  }

 diff --git a/src/shared/util.h b/src/shared/util.h
 index fdb9fb6..8445371 100644
 --- a/src/shared/util.h
 +++ b/src/shared/util.h
 @@ -942,32 +942,6 @@ int unlink_noerrno(const char *path);
  _r_;\
  })

 -struct _locale_struct_ {
 -locale_t saved_locale;
 -locale_t new_locale;
 -bool quit;
 -};
 -
 -static inline void _reset_locale_(struct _locale_struct_ *s) {
 -PROTECT_ERRNO;
 -if (s-saved_locale != (locale_t) 0)
 -uselocale(s-saved_locale);
 -if (s-new_locale != (locale_t) 0)
 -freelocale(s-new_locale);
 -}
 -
 -#define RUN_WITH_LOCALE(mask, loc) \
 -for (_cleanup_(_reset_locale_) struct _locale_struct_ _saved_locale_ 
 = { (locale_t) 0, (locale_t) 0, false }; \
 - ({ \
 - if (!_saved_locale_.quit) {\
 - PROTECT_ERRNO; \
 - _saved_locale_.new_locale = newlocale((mask), 
 (loc), (locale_t) 0); \
 - if (_saved_locale_.new_locale != (locale_t) 0)  
\
 - _saved_locale_.saved_locale = 
 uselocale(_saved_locale_.new_locale); \
 - }  \
 - !_saved_locale_.quit; }) ; \
 - _saved_locale_.quit = true)
 -
  bool id128_is_valid(const char *s) _pure_;

  int split_pair(const char *s, const char *sep, char **l, char **r);
 --
 2.2.1

 ___
 systemd-devel mailing list

Re: [systemd-devel] Suspicious assertions in resolved

2015-01-18 Thread Topi Miettinen 
On 01/18/15 21:23, David Herrmann wrote:
 Hi
 
 On Sun, Jan 18, 2015 at 10:15 PM, Topi Miettinen toiwo...@gmail.com wrote:
 On 01/18/15 20:45, David Herrmann wrote:
 Hi

 On Sun, Jan 18, 2015 at 8:12 PM, Topi Miettinen toiwo...@gmail.com wrote:
 Hello,

 I think resolved_manager.c function manager_recv() has an assertion that
 could be triggerable by the server sending an oversized packet:

 assert(!(mh.msg_flags  MSG_TRUNC));

 The other assertions look suspicious too but I don't know if they can
 really be triggered by the other side.

 We use FIONREAD to read the size of the next pending datagram.
 Therefore, MSG_TRUNC cannot be set. Similarly, we provide suitable
 control-data space so MSG_CTRUNC cannot be set, either.

 OK. What about the assertions later, is it possible to receive a reply
 via IPv6 for IPv4 request or the other way around?
 
 Those asserts verify that the CMSG socket-type is the same as the
 PACKET/PEER socket-type. I don't see how this looks at the
 request-type? Those assert()s look good to me.

OK, sorry for the noise.

-Topi

 Thanks
 David
 

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