Merge pull request #12140 from poettering/copy-early

chattr/copy.c fixes

NEWS

@@ -104,6 +104,10 @@ CHANGES WITH 242 in spe:
 
           https://systemd.io/TEMPORARY_DIRECTORIES
 
+        * systemd-tmpfiles' h line type gained support for the
+          FS_PROJINHERIT_FL ('P') file attribute (introduced in kernel 4.5),
+          controlling project quota inheritance.
+
         * sd-boot and bootctl now implement support for an Extended Boot Loader
           (XBOOTLDR) partition, that is intended to be mounted to /boot, in
           addition to the ESP partition mounted to /efi or /boot/efi.

TODO

@@ -1,7 +1,5 @@
 Bugfixes:
 
-* copy.c: set the right chattrs before copying files and others after
-
 * Many manager configuration settings that are only applicable to user
   manager or system manager can be always set. It would be better to reject
   them when parsing config.

man/tmpfiles.d.xml

@@ -368,15 +368,11 @@ L     /tmp/foobar -    -    -     -   /dev/null</programlisting>
           <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
+          <para>The format of the argument field is <varname>[+-=][aAcCdDeijPsStTu] </varname>. The prefix
-          <varname>[+-=][aAcCdDeijsStTu] </varname>. The prefix
+          <varname>+</varname> (the default one) causes the attribute(s) to be added; <varname>-</varname>
-          <varname>+</varname> (the default one) causes the
+          causes the attribute(s) to be removed; <varname>=</varname> causes the attributes to be set exactly
-          attribute(s) to be added; <varname>-</varname> causes the
+          as the following letters. The letters <literal>aAcCdDeijPsStTu</literal> select the new attributes
-          attribute(s) to be removed; <varname>=</varname> causes the
+          for the files, see <citerefentry project='man-pages'><refentrytitle>chattr</refentrytitle>
-          attributes to be set exactly as the following letters. The
-          letters <literal>aAcCdDeijsStTu</literal> select the new
-          attributes for the files, see
-          <citerefentry project='man-pages'><refentrytitle>chattr</refentrytitle>
           <manvolnum>1</manvolnum></citerefentry> for further information.
           </para>
           <para>Passing only <varname>=</varname> as argument resets

src/basic/chattr-util.h

@@ -1,6 +1,37 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
 #pragma once
 
+#include <linux/fs.h>
+
+#include "missing_fs.h"
+
+/* The chattr() flags to apply when creating a new file *before* writing to it. In particular, flags such as
+ * FS_NOCOW_FL don't work if applied a-posteriori. All other flags are fine (or even necessary, think
+ * FS_IMMUTABLE_FL!) to apply after writing to the files. */
+#define CHATTR_EARLY_FL                         \
+        (FS_NOATIME_FL |                        \
+         FS_COMPR_FL   |                        \
+         FS_NOCOW_FL   |                        \
+         FS_NOCOMP_FL  |                        \
+         FS_PROJINHERIT_FL)
+
+#define CHATTR_ALL_FL                           \
+        (FS_NOATIME_FL      |                   \
+         FS_SYNC_FL         |                   \
+         FS_DIRSYNC_FL      |                   \
+         FS_APPEND_FL       |                   \
+         FS_COMPR_FL        |                   \
+         FS_NODUMP_FL       |                   \
+         FS_EXTENT_FL       |                   \
+         FS_IMMUTABLE_FL    |                   \
+         FS_JOURNAL_DATA_FL |                   \
+         FS_SECRM_FL        |                   \
+         FS_UNRM_FL         |                   \
+         FS_NOTAIL_FL       |                   \
+         FS_TOPDIR_FL       |                   \
+         FS_NOCOW_FL        |                   \
+         FS_PROJINHERIT_FL)
+
 int chattr_fd(int fd, unsigned value, unsigned mask, unsigned *previous);
 int chattr_path(const char *p, unsigned value, unsigned mask, unsigned *previous);
 

src/basic/copy.c

@@ -755,6 +755,7 @@ int copy_file_full(
                 int flags,
                 mode_t mode,
                 unsigned chattr_flags,
+                unsigned chattr_mask,
                 CopyFlags copy_flags,
                 copy_progress_bytes_t progress_bytes,
                 void *userdata) {
@@ -770,8 +771,8 @@ int copy_file_full(
                         return -errno;
         }
 
-        if (chattr_flags != 0)
+        if (chattr_mask != 0)
-                (void) chattr_fd(fdt, chattr_flags, (unsigned) -1, NULL);
+                (void) chattr_fd(fdt, chattr_flags, chattr_mask & CHATTR_EARLY_FL, NULL);
 
         r = copy_file_fd_full(from, fdt, copy_flags, progress_bytes, userdata);
         if (r < 0) {
@@ -780,6 +781,9 @@ int copy_file_full(
                 return r;
         }
 
+        if (chattr_mask != 0)
+                (void) chattr_fd(fdt, chattr_flags, chattr_mask & ~CHATTR_EARLY_FL, NULL);
+
         if (close(fdt) < 0) {
                 unlink_noerrno(to);
                 return -errno;
@@ -793,6 +797,7 @@ int copy_file_atomic_full(
                 const char *to,
                 mode_t mode,
                 unsigned chattr_flags,
+                unsigned chattr_mask,
                 CopyFlags copy_flags,
                 copy_progress_bytes_t progress_bytes,
                 void *userdata) {
@@ -826,8 +831,8 @@ int copy_file_atomic_full(
                         return fdt;
         }
 
-        if (chattr_flags != 0)
+        if (chattr_mask != 0)
-                (void) chattr_fd(fdt, chattr_flags, (unsigned) -1, NULL);
+                (void) chattr_fd(fdt, chattr_flags, chattr_mask & CHATTR_EARLY_FL, NULL);
 
         r = copy_file_fd_full(from, fdt, copy_flags, progress_bytes, userdata);
         if (r < 0)
@@ -845,6 +850,9 @@ int copy_file_atomic_full(
                         return r;
         }
 
+        if (chattr_mask != 0)
+                (void) chattr_fd(fdt, chattr_flags, chattr_mask & ~CHATTR_EARLY_FL, NULL);
+
         t = mfree(t);
         return 0;
 }

src/basic/copy.h

@@ -25,14 +25,14 @@ static inline int copy_file_fd(const char *from, int to, CopyFlags copy_flags) {
         return copy_file_fd_full(from, to, copy_flags, NULL, NULL);
 }
 
-int copy_file_full(const char *from, const char *to, int open_flags, mode_t mode, unsigned chattr_flags, CopyFlags copy_flags, copy_progress_bytes_t progress, void *userdata);
+int copy_file_full(const char *from, const char *to, int open_flags, mode_t mode, unsigned chattr_flags, unsigned chattr_mask, CopyFlags copy_flags, copy_progress_bytes_t progress, void *userdata);
-static inline int copy_file(const char *from, const char *to, int open_flags, mode_t mode, unsigned chattr_flags, CopyFlags copy_flags) {
+static inline int copy_file(const char *from, const char *to, int open_flags, mode_t mode, unsigned chattr_flags, unsigned chattr_mask, CopyFlags copy_flags) {
-        return copy_file_full(from, to, open_flags, mode, chattr_flags, copy_flags, NULL, NULL);
+        return copy_file_full(from, to, open_flags, mode, chattr_flags, chattr_mask, copy_flags, NULL, NULL);
 }
 
-int copy_file_atomic_full(const char *from, const char *to, mode_t mode, unsigned chattr_flags, CopyFlags copy_flags, copy_progress_bytes_t progress, void *userdata);
+int copy_file_atomic_full(const char *from, const char *to, mode_t mode, unsigned chattr_flags, unsigned chattr_mask, CopyFlags copy_flags, copy_progress_bytes_t progress, void *userdata);
-static inline int copy_file_atomic(const char *from, const char *to, mode_t mode, unsigned chattr_flags, CopyFlags copy_flags) {
+static inline int copy_file_atomic(const char *from, const char *to, mode_t mode, unsigned chattr_flags, unsigned chattr_mask, CopyFlags copy_flags) {
-        return copy_file_atomic_full(from, to, mode, chattr_flags, copy_flags, NULL, NULL);
+        return copy_file_atomic_full(from, to, mode, chattr_flags, chattr_mask, copy_flags, NULL, NULL);
 }
 
 int copy_tree_at_full(int fdf, const char *from, int fdt, const char *to, uid_t override_uid, gid_t override_gid, CopyFlags copy_flags, copy_progress_path_t progress_path, copy_progress_bytes_t progress_bytes, void *userdata);

src/basic/missing_fs.h

@@ -61,3 +61,7 @@
 #ifndef NS_GET_NSTYPE /* d95fa3c76a66b6d76b1e109ea505c55e66360f3c (4.11) */
 #define NS_GET_NSTYPE _IO(0xb7, 0x3)
 #endif
+
+#ifndef FS_PROJINHERIT_FL
+#define FS_PROJINHERIT_FL 0x20000000
+#endif

src/firstboot/firstboot.c

@@ -254,7 +254,7 @@ static int process_locale(void) {
         if (arg_copy_locale && arg_root) {
 
                 mkdir_parents(etc_localeconf, 0755);
-                r = copy_file("/etc/locale.conf", etc_localeconf, 0, 0644, 0, COPY_REFLINK);
+                r = copy_file("/etc/locale.conf", etc_localeconf, 0, 0644, 0, 0, COPY_REFLINK);
                 if (r != -ENOENT) {
                         if (r < 0)
                                 return log_error_errno(r, "Failed to copy %s: %m", etc_localeconf);
@@ -328,7 +328,7 @@ static int process_keymap(void) {
         if (arg_copy_keymap && arg_root) {
 
                 mkdir_parents(etc_vconsoleconf, 0755);
-                r = copy_file("/etc/vconsole.conf", etc_vconsoleconf, 0, 0644, 0, COPY_REFLINK);
+                r = copy_file("/etc/vconsole.conf", etc_vconsoleconf, 0, 0644, 0, 0, COPY_REFLINK);
                 if (r != -ENOENT) {
                         if (r < 0)
                                 return log_error_errno(r, "Failed to copy %s: %m", etc_vconsoleconf);

src/import/pull-raw.c

@@ -299,7 +299,7 @@ static int raw_pull_copy_auxiliary_file(
 
         local = strjoina(i->image_root, "/", i->local, suffix);
 
-        r = copy_file_atomic(*path, local, 0644, 0, COPY_REFLINK | (i->force_local ? COPY_REPLACE : 0));
+        r = copy_file_atomic(*path, local, 0644, 0, 0, COPY_REFLINK | (i->force_local ? COPY_REPLACE : 0));
         if (r == -EEXIST)
                 log_warning_errno(r, "File %s already exists, not replacing.", local);
         else if (r == -ENOENT)

src/import/pull-tar.c

@@ -244,7 +244,7 @@ static int tar_pull_make_local_copy(TarPull *i) {
 
                 local_settings = strjoina(i->image_root, "/", i->local, ".nspawn");
 
-                r = copy_file_atomic(i->settings_path, local_settings, 0664, 0, COPY_REFLINK | (i->force_local ? COPY_REPLACE : 0));
+                r = copy_file_atomic(i->settings_path, local_settings, 0664, 0, 0, COPY_REFLINK | (i->force_local ? COPY_REPLACE : 0));
                 if (r == -EEXIST)
                         log_warning_errno(r, "Settings file %s already exists, not replacing.", local_settings);
                 else if (r == -ENOENT)

src/nspawn/nspawn.c

@@ -1701,7 +1701,7 @@ static int setup_timezone(const char *dest) {
 
         case TIMEZONE_COPY:
                 /* If mounting failed, try to copy */
-                r = copy_file_atomic("/etc/localtime", where, 0644, 0, COPY_REFLINK|COPY_REPLACE);
+                r = copy_file_atomic("/etc/localtime", where, 0644, 0, 0, COPY_REFLINK|COPY_REPLACE);
                 if (r < 0) {
                         log_full_errno(IN_SET(r, -EROFS, -EACCES, -EPERM) ? LOG_DEBUG : LOG_WARNING, r,
                                        "Failed to copy /etc/localtime to %s, ignoring: %m", where);
@@ -1828,7 +1828,7 @@ static int setup_resolv_conf(const char *dest) {
         }
 
         /* If that didn't work, let's copy the file */
-        r = copy_file(what, where, O_TRUNC|O_NOFOLLOW, 0644, 0, COPY_REFLINK);
+        r = copy_file(what, where, O_TRUNC|O_NOFOLLOW, 0644, 0, 0, COPY_REFLINK);
         if (r < 0) {
                 /* If the file already exists as symlink, let's suppress the warning, under the assumption that
                  * resolved or something similar runs inside and the symlink points there.
@@ -4874,7 +4874,7 @@ static int run(int argc, char *argv[]) {
                                 goto finish;
                         }
 
-                        r = copy_file(arg_image, np, O_EXCL, arg_read_only ? 0400 : 0600, FS_NOCOW_FL, COPY_REFLINK|COPY_CRTIME);
+                        r = copy_file(arg_image, np, O_EXCL, arg_read_only ? 0400 : 0600, FS_NOCOW_FL, FS_NOCOW_FL, COPY_REFLINK|COPY_CRTIME);
                         if (r < 0) {
                                 r = log_error_errno(r, "Failed to copy image file: %m");
                                 goto finish;

src/portable/portable.c

@@ -786,7 +786,7 @@ static int install_profile_dropin(
 
         if (flags & PORTABLE_PREFER_COPY) {
 
-                r = copy_file_atomic(from, dropin, 0644, 0, COPY_REFLINK);
+                r = copy_file_atomic(from, dropin, 0644, 0, 0, COPY_REFLINK);
                 if (r < 0)
                         return log_debug_errno(r, "Failed to copy %s %s %s: %m", from, special_glyph(SPECIAL_GLYPH_ARROW), dropin);
 

src/shared/machine-image.c

@@ -808,7 +808,7 @@ static int clone_auxiliary_file(const char *path, const char *new_name, const ch
         if (!rs)
                 return -ENOMEM;
 
-        return copy_file_atomic(path, rs, 0664, 0, COPY_REFLINK);
+        return copy_file_atomic(path, rs, 0664, 0, 0, COPY_REFLINK);
 }
 
 int image_clone(Image *i, const char *new_name, bool read_only) {
@@ -870,7 +870,7 @@ int image_clone(Image *i, const char *new_name, bool read_only) {
         case IMAGE_RAW:
                 new_path = strjoina("/var/lib/machines/", new_name, ".raw");
 
-                r = copy_file_atomic(i->path, new_path, read_only ? 0444 : 0644, FS_NOCOW_FL, COPY_REFLINK|COPY_CRTIME);
+                r = copy_file_atomic(i->path, new_path, read_only ? 0444 : 0644, FS_NOCOW_FL, FS_NOCOW_FL, COPY_REFLINK|COPY_CRTIME);
                 break;
 
         case IMAGE_BLOCK:

src/systemctl/systemctl.c

@@ -7210,7 +7210,7 @@ static int create_edit_temp_file(const char *new_path, const char *original_path
         if (r < 0)
                 return log_error_errno(r, "Failed to create directories for \"%s\": %m", new_path);
 
-        r = copy_file(original_path, t, 0, 0644, 0, COPY_REFLINK);
+        r = copy_file(original_path, t, 0, 0644, 0, 0, COPY_REFLINK);
         if (r == -ENOENT) {
 
                 r = touch(t);

src/test/test-copy.c

@@ -38,7 +38,7 @@ static void test_copy_file(void) {
 
         assert_se(write_string_file(fn, "foo bar bar bar foo", WRITE_STRING_FILE_CREATE) == 0);
 
-        assert_se(copy_file(fn, fn_copy, 0, 0644, 0, COPY_REFLINK) == 0);
+        assert_se(copy_file(fn, fn_copy, 0, 0644, 0, 0, COPY_REFLINK) == 0);
 
         assert_se(read_full_file(fn_copy, &buf, &sz) == 0);
         assert_se(streq(buf, "foo bar bar bar foo\n"));
@@ -246,13 +246,13 @@ static void test_copy_atomic(void) {
 
         q = strjoina(p, "/fstab");
 
-        r = copy_file_atomic("/etc/fstab", q, 0644, 0, COPY_REFLINK);
+        r = copy_file_atomic("/etc/fstab", q, 0644, 0, 0, COPY_REFLINK);
         if (r == -ENOENT)
                 return;
 
-        assert_se(copy_file_atomic("/etc/fstab", q, 0644, 0, COPY_REFLINK) == -EEXIST);
+        assert_se(copy_file_atomic("/etc/fstab", q, 0644, 0, 0, COPY_REFLINK) == -EEXIST);
 
-        assert_se(copy_file_atomic("/etc/fstab", q, 0644, 0, COPY_REPLACE) >= 0);
+        assert_se(copy_file_atomic("/etc/fstab", q, 0644, 0, 0, COPY_REPLACE) >= 0);
 }
 
 int main(int argc, char *argv[]) {

src/tmpfiles/tmpfiles.c

@@ -1096,22 +1096,6 @@ static int path_set_acls(Item *item, const char *path) {
         return r;
 }
 
-#define ATTRIBUTES_ALL                          \
-        (FS_NOATIME_FL      |                   \
-         FS_SYNC_FL         |                   \
-         FS_DIRSYNC_FL      |                   \
-         FS_APPEND_FL       |                   \
-         FS_COMPR_FL        |                   \
-         FS_NODUMP_FL       |                   \
-         FS_EXTENT_FL       |                   \
-         FS_IMMUTABLE_FL    |                   \
-         FS_JOURNAL_DATA_FL |                   \
-         FS_SECRM_FL        |                   \
-         FS_UNRM_FL         |                   \
-         FS_NOTAIL_FL       |                   \
-         FS_TOPDIR_FL       |                   \
-         FS_NOCOW_FL)
-
 static int parse_attribute_from_arg(Item *item) {
 
         static const struct {
@@ -1132,6 +1116,7 @@ static int parse_attribute_from_arg(Item *item) {
                 { 't', FS_NOTAIL_FL },       /* file tail should not be merged */
                 { 'T', FS_TOPDIR_FL },       /* Top of directory hierarchies */
                 { 'C', FS_NOCOW_FL },        /* Do not cow file */
+                { 'P', FS_PROJINHERIT_FL },  /* Inherit the quota project ID */
         };
 
         enum {
@@ -1184,7 +1169,7 @@ static int parse_attribute_from_arg(Item *item) {
         }
 
         if (mode == MODE_SET)
-                mask |= ATTRIBUTES_ALL;
+                mask |= CHATTR_ALL_FL;
 
         assert(mask != 0);