[systemd-commits] 6 commits - TODO man/tmpfiles.d.xml src/tmpfiles

Lennart Poettering lennart at kemper.freedesktop.org
Mon Apr 13 06:30:36 PDT 2015


 TODO                    |    4 +
 man/tmpfiles.d.xml      |   75 +++++++++++++++---------------
 src/tmpfiles/tmpfiles.c |  120 ++++++++++++++++++++++++++++++++----------------
 3 files changed, 123 insertions(+), 76 deletions(-)

New commits:
commit 48e6d6a6e911af0cf4e3ef12b0a3eeb2c8031d8a
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Apr 13 15:23:42 2015 +0200

    update TODO

diff --git a/TODO b/TODO
index 3de2bb8..2e70362 100644
--- a/TODO
+++ b/TODO
@@ -48,6 +48,10 @@ Before 220:
 
 Features:
 
+* tmpfiles: creating new directories/subvolumes/fifos/device nodes
+  should not follow symlinks. None of the other adjustment or creation
+  calls follow symlinks.
+
 * bus-proxy: fix return code when releasing name that we don't have:
   http://lists.freedesktop.org/archives/systemd-devel/2015-April/030494.html
 

commit 1ae705fb3d635eb0c19d3712b2b78c5f45e7efad
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Apr 13 15:23:37 2015 +0200

    man: slightly fewer paragraphs can help readability

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 107d422..971b142 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -310,25 +310,23 @@
           <listitem><para>Set file/directory attributes. Lines of this type
           accept shell-style globs in place of normal path names.</para>
 
-          <para>The format of the argument field is <varname>[+-=][aAcCdDeijsStTu]
-          </varname></para>
-
-          <para>The prefix <varname>+</varname> (the default one) causes the
+          <para>The format of the argument field is
+          <varname>[+-=][aAcCdDeijsStTu] </varname>. The prefix
+          <varname>+</varname> (the default one) causes the
           attribute(s) to be added; <varname>-</varname> causes the
-          attribute(s) to be removed; <varname>=</varname>
-          causes the attributes to set exactly as the following letters.</para>
-          <para>The letters <literal>aAcCdDeijsStTu</literal> select the new
+          attribute(s) to be removed; <varname>=</varname> causes the
+          attributes to set exactly as the following letters. The
+          letters <literal>aAcCdDeijsStTu</literal> select the new
           attributes for the files, see
           <citerefentry><refentrytitle>chattr</refentrytitle>
           <manvolnum>1</manvolnum></citerefentry> for further information.
           </para>
-          <para>Passing only <varname>=</varname> as argument,
-          resets all the file attributes listed above. It has to be pointed
-          out that the <varname>=</varname> prefix, limits itself to the
-          attributes corresponding to the letters listed here. All other
-          attributes will be left untouched.</para>
-
-          <para>Does not follow symlinks.</para>
+          <para>Passing only <varname>=</varname> as argument resets
+          all the file attributes listed above. It has to be pointed
+          out that the <varname>=</varname> prefix, limits itself to
+          the attributes corresponding to the letters listed here. All
+          other attributes will be left untouched. Does not follow
+          symlinks.</para>
           </listitem>
         </varlistentry>
 

commit bd1100898d63e9e2d8f6327b6895454f9abd5bd0
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Apr 13 15:23:07 2015 +0200

    man: fix examples indentation in tmpfiles.d(5)

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 2882a05..107d422 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -546,15 +546,15 @@
       boot with specific modes and ownership.</para>
 
       <programlisting>d /run/screens  1777 root root 10d
-      d /run/uscreens 0755 root root 10d12h
-      t /run/screen - - - - user.name="John Smith" security.SMACK64=screen</programlisting>
+d /run/uscreens 0755 root root 10d12h
+t /run/screen - - - - user.name="John Smith" security.SMACK64=screen</programlisting>
     </example>
     <example>
       <title>/etc/tmpfiles.d/abrt.conf example</title>
       <para><command>abrt</command> needs a directory created at boot with specific mode and ownership and its content should be preserved.</para>
 
       <programlisting>d /var/tmp/abrt 0755 abrt abrt
-      x /var/tmp/abrt/*</programlisting>
+x /var/tmp/abrt/*</programlisting>
     </example>
   </refsect1>
 

commit 0ac0b1e720d009c021730a8627f2ad3d93047a85
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Apr 13 15:22:50 2015 +0200

    man: add information about more lines to explanation of argument field

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index a9ef3df..2882a05 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -528,9 +528,12 @@
       <varname>w</varname> may be used to specify a short string that
       is written to the file, suffixed by a newline. For
       <varname>C</varname>, specifies the source file or
-      directory. For <varname>t</varname> determines extended
-      attributes to be set. For <varname>a</varname> determines
-      ACL attributes to be set. Ignored for all other lines.</para>
+      directory. For <varname>t</varname>, <varname>T</varname>
+      determines extended attributes to be set. For
+      <varname>a</varname>, <varname>A</varname> determines ACL
+      attributes to be set. For <varname>h</varname>,
+      <varname>H</varname> determines the file attributes to
+      set. Ignored for all other lines.</para>
     </refsect2>
 
   </refsect1>

commit 6a9171d2ec4f2cc1d7850fcb7b076cb75fbd3dd3
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Apr 13 15:18:09 2015 +0200

    man: document which tmpfiles line types follow symlinks
    
    Generally, we will not follow symlinks, except for "w".
    
    Avoid documentation for now for fifo, device node, directory lines,
    which currently follow symlinks but better shouldn't.

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 839bb76..a9ef3df 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -133,13 +133,13 @@
           <term><varname>f</varname></term>
           <listitem><para>Create a file if it does not exist yet. If
           the argument parameter is given, it will be written to the
-          file.</para></listitem>
+          file. Does not follow symlinks.</para></listitem>
         </varlistentry>
 
         <varlistentry>
           <term><varname>F</varname></term>
           <listitem><para>Create or truncate a file. If the argument
-          parameter is given, it will be written to the file.</para>
+          parameter is given, it will be written to the file. Does not follow symlinks.</para>
           </listitem>
         </varlistentry>
 
@@ -149,7 +149,8 @@
           the file exists.  Lines of this type accept shell-style
           globs in place of normal path names. The argument parameter
           will be written without a trailing newline. C-style
-          backslash escapes are interpreted.</para></listitem>
+          backslash escapes are interpreted. Follows
+          symlinks.</para></listitem>
         </varlistentry>
 
         <varlistentry>
@@ -227,7 +228,7 @@
           copy operation is skipped. If the argument is omitted, files
           from the source directory
           <filename>/usr/share/factory/</filename> with the same name
-          are copied.</para></listitem>
+          are copied. Does not follow symlinks.</para></listitem>
         </varlistentry>
 
         <varlistentry>
@@ -259,7 +260,7 @@
           This may not be used to remove non-empty directories, use
           <varname>R</varname> for that.  Lines of this type accept
           shell-style globs in place of normal path
-          names.</para></listitem>
+          names. Does not follow symlinks.</para></listitem>
         </varlistentry>
 
         <varlistentry>
@@ -267,7 +268,7 @@
           <listitem><para>Recursively remove a path and all its
           subdirectories (if it is a directory). Lines of this type
           accept shell-style globs in place of normal path
-          names.</para></listitem>
+          names. Does not follow symlinks.</para></listitem>
         </varlistentry>
 
         <varlistentry>
@@ -275,7 +276,7 @@
           <listitem><para>Adjust 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. Does not follow symlinks.</para></listitem>
         </varlistentry>
 
         <varlistentry>
@@ -284,24 +285,24 @@
           user, and restore the SELinux security context of a file or
           directory if it exists, as well as of its subdirectories and
           the files contained therein (if applicable). Lines of this
-          type accept shell-style globs in place of normal path names.
-          </para></listitem>
+          type accept shell-style globs in place of normal path
+          names. Does not follow symlinks.  </para></listitem>
         </varlistentry>
 
         <varlistentry>
           <term><varname>t</varname></term>
           <listitem><para>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>
+          This can be useful for setting SMACK labels. Does not follow
+          symlinks.</para></listitem>
         </varlistentry>
 
         <varlistentry>
           <term><varname>T</varname></term>
           <listitem><para>Recursively 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>
+          path names.  This can be useful for setting SMACK
+          labels. Does not follow symlinks.  </para></listitem>
         </varlistentry>
 
         <varlistentry>
@@ -325,9 +326,9 @@
           resets all the file attributes listed above. It has to be pointed
           out that the <varname>=</varname> prefix, limits itself to the
           attributes corresponding to the letters listed here. All other
-          attributes will be left untouched.
-          </para>
+          attributes will be left untouched.</para>
 
+          <para>Does not follow symlinks.</para>
           </listitem>
         </varlistentry>
 
@@ -335,7 +336,7 @@
           <term><varname>H</varname></term>
           <listitem><para>Recursively set file/directory attributes. Lines
           of this type accept shell-style globs in place of normal
-          path names.
+          path names. Does not follow symlinks.
           </para></listitem>
         </varlistentry>
 
@@ -352,14 +353,15 @@
           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>
+          files. Does not follow symlinks.</para></listitem>
         </varlistentry>
 
         <varlistentry>
           <term><varname>A</varname></term>
           <term><varname>A+</varname></term>
           <listitem><para>Same as <varname>a</varname> and
-          <varname>a+</varname>, but recursive.</para></listitem>
+          <varname>a+</varname>, but recursive. Does not follow
+          symlinks.</para></listitem>
         </varlistentry>
       </variablelist>
 

commit 48b8aaa82724bc2d8440470f414fb0d2416f29c7
Author: Lennart Poettering <lennart at poettering.net>
Date:   Mon Apr 13 15:16:54 2015 +0200

    tmpfiles: don't follow symlinks when adjusting ACLs, fille attributes, access modes or ownership

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 16114b2..b7dd37c 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -583,51 +583,69 @@ finish:
 }
 
 static int path_set_perms(Item *i, const char *path) {
+        _cleanup_close_ int fd = -1;
         struct stat st;
-        bool st_valid;
 
         assert(i);
         assert(path);
 
-        st_valid = stat(path, &st) == 0;
+        /* We open the file with O_PATH here, to make the operation
+         * somewhat atomic. Also there's unfortunately no fchmodat()
+         * with AT_SYMLINK_NOFOLLOW, hence we emulate it here via
+         * O_PATH. */
+
+        fd = open(path, O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_PATH|O_NOATIME);
+        if (fd < 0)
+                return log_error_errno(errno, "Adjusting owner and mode for %s failed: %m", path);
+
+        if (fstatat(fd, "", &st, AT_EMPTY_PATH) < 0)
+                return log_error_errno(errno, "Failed to fstat() file %s: %m", path);
 
-        /* not using i->path directly because it may be a glob */
-        if (i->mode_set) {
-                mode_t m = i->mode;
+        if (S_ISLNK(st.st_mode))
+                log_debug("Skipping mode an owner fix for symlink %s.", path);
+        else {
+                char fn[strlen("/proc/self/fd/") + DECIMAL_STR_MAX(int)];
+                xsprintf(fn, "/proc/self/fd/%i", fd);
 
-                if (i->mask_perms && st_valid) {
-                        if (!(st.st_mode & 0111))
-                                m &= ~0111;
-                        if (!(st.st_mode & 0222))
+                /* not using i->path directly because it may be a glob */
+                if (i->mode_set) {
+                        mode_t m = i->mode;
+
+                        if (i->mask_perms) {
+                                if (!(st.st_mode & 0111))
+                                        m &= ~0111;
+                                if (!(st.st_mode & 0222))
                                 m &= ~0222;
-                        if (!(st.st_mode & 0444))
-                                m &= ~0444;
-                        if (!S_ISDIR(st.st_mode))
-                                m &= ~07000; /* remove sticky/sgid/suid bit, unless directory */
-                }
+                                if (!(st.st_mode & 0444))
+                                        m &= ~0444;
+                                if (!S_ISDIR(st.st_mode))
+                                        m &= ~07000; /* remove sticky/sgid/suid bit, unless directory */
+                        }
 
-                if (st_valid && m == (st.st_mode & 07777))
-                        log_debug("\"%s\" has right mode %o", path, st.st_mode);
-                else {
-                        log_debug("chmod \"%s\" to mode %o", path, m);
-                        if (chmod(path, m) < 0)
-                                return log_error_errno(errno, "chmod(%s) failed: %m", path);
+                        if (m == (st.st_mode & 07777))
+                                log_debug("\"%s\" has right mode %o", path, st.st_mode);
+                        else {
+                                log_debug("chmod \"%s\" to mode %o", path, m);
+                                if (chmod(fn, m) < 0)
+                                        return log_error_errno(errno, "chmod(%s) failed: %m", path);
+                        }
                 }
-        }
-
-        if ((!st_valid || i->uid != st.st_uid || i->gid != st.st_gid) &&
-            (i->uid_set || i->gid_set)) {
-                log_debug("chown \"%s\" to "UID_FMT"."GID_FMT,
-                          path,
-                          i->uid_set ? i->uid : UID_INVALID,
-                          i->gid_set ? i->gid : GID_INVALID);
-                if (chown(path,
-                          i->uid_set ? i->uid : UID_INVALID,
-                          i->gid_set ? i->gid : GID_INVALID) < 0)
 
+                if ((i->uid != st.st_uid || i->gid != st.st_gid) &&
+                    (i->uid_set || i->gid_set)) {
+                        log_debug("chown \"%s\" to "UID_FMT"."GID_FMT,
+                                  path,
+                                  i->uid_set ? i->uid : UID_INVALID,
+                                  i->gid_set ? i->gid : GID_INVALID);
+                        if (chown(fn,
+                                  i->uid_set ? i->uid : UID_INVALID,
+                                  i->gid_set ? i->gid : GID_INVALID) < 0)
                         return log_error_errno(errno, "chown(%s) failed: %m", path);
+                }
         }
 
+        fd = safe_close(fd);
+
         return label_fix(path, false, false);
 }
 
@@ -712,10 +730,10 @@ static int parse_acls_from_arg(Item *item) {
 }
 
 #ifdef HAVE_ACL
-static int path_set_acl(const char *path, acl_type_t type, acl_t acl, bool modify) {
+static int path_set_acl(const char *path, const char *pretty, acl_type_t type, acl_t acl, bool modify) {
+        _cleanup_(acl_free_charpp) char *t = NULL;
         _cleanup_(acl_freep) acl_t dup = NULL;
         int r;
-        _cleanup_(acl_free_charpp) char *t = NULL;
 
         /* Returns 0 for success, positive error if already warned,
          * negative error otherwise. */
@@ -741,9 +759,9 @@ static int path_set_acl(const char *path, acl_type_t type, acl_t acl, bool modif
                 return r;
 
         t = acl_to_any_text(dup, NULL, ',', TEXT_ABBREVIATE);
-        log_debug("\"%s\": setting %s ACL \"%s\"", path,
+        log_debug("Setting %s ACL %s on %s.",
                   type == ACL_TYPE_ACCESS ? "access" : "default",
-                  strna(t));
+                  strna(t), pretty);
 
         r = acl_set_file(path, type, dup);
         if (r < 0)
@@ -751,7 +769,7 @@ static int path_set_acl(const char *path, acl_type_t type, acl_t acl, bool modif
                 return -log_error_errno(errno,
                                         "Setting %s ACL \"%s\" on %s failed: %m",
                                         type == ACL_TYPE_ACCESS ? "access" : "default",
-                                        strna(t), path);
+                                        strna(t), pretty);
 
         return 0;
 }
@@ -760,14 +778,32 @@ static int path_set_acl(const char *path, acl_type_t type, acl_t acl, bool modif
 static int path_set_acls(Item *item, const char *path) {
         int r = 0;
 #ifdef HAVE_ACL
+        char fn[strlen("/proc/self/fd/") + DECIMAL_STR_MAX(int)];
+        _cleanup_close_ int fd = -1;
+        struct stat st;
+
         assert(item);
         assert(path);
 
+        fd = open(path, O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_PATH|O_NOATIME);
+        if (fd < 0)
+                return log_error_errno(errno, "Adjusting ACL of %s failed: %m", path);
+
+        if (fstatat(fd, "", &st, AT_EMPTY_PATH) < 0)
+                return log_error_errno(errno, "Failed to fstat() file %s: %m", path);
+
+        if (S_ISLNK(st.st_mode)) {
+                log_debug("Skipping ACL fix for symlink %s.", path);
+                return 0;
+        }
+
+        xsprintf(fn, "/proc/self/fd/%i", fd);
+
         if (item->acl_access)
-                r = path_set_acl(path, ACL_TYPE_ACCESS, item->acl_access, item->force);
+                r = path_set_acl(fn, path, ACL_TYPE_ACCESS, item->acl_access, item->force);
 
         if (r == 0 && item->acl_default)
-                r = path_set_acl(path, ACL_TYPE_DEFAULT, item->acl_default, item->force);
+                r = path_set_acl(fn, path, ACL_TYPE_DEFAULT, item->acl_default, item->force);
 
         if (r > 0)
                 return -r; /* already warned */
@@ -891,9 +927,13 @@ static int path_set_attribute(Item *item, const char *path) {
         if (!item->attribute_set || item->attribute_mask == 0)
                 return 0;
 
-        fd = open(path, O_RDONLY|O_NONBLOCK|O_CLOEXEC);
-        if (fd < 0)
+        fd = open(path, O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_NOATIME|O_NOFOLLOW);
+        if (fd < 0) {
+                if (errno == ELOOP)
+                        return log_error_errno(errno, "Skipping file attributes adjustment on symlink %s.", path);
+
                 return log_error_errno(errno, "Cannot open '%s': %m", path);
+        }
 
         if (fstat(fd, &st) < 0)
                 return log_error_errno(errno, "Cannot stat '%s': %m", path);



More information about the systemd-commits mailing list