[systemd-commits] 3 commits - TODO src/import src/journal src/shared src/tmpfiles
Lennart Poettering
lennart at kemper.freedesktop.org
Wed Apr 8 13:39:51 PDT 2015
TODO | 6 +
src/import/import-raw.c | 4
src/import/pull-raw.c | 6 -
src/journal/journal-file.c | 4
src/journal/journalctl.c | 2
src/shared/copy.c | 2
src/shared/util.c | 36 +------
src/shared/util.h | 5 -
src/tmpfiles/tmpfiles.c | 212 ++++++++++++++++++++++++---------------------
9 files changed, 138 insertions(+), 139 deletions(-)
New commits:
commit aeb1e2c5b0a9d8cb48de766b6c0afff43a3b8331
Author: Lennart Poettering <lennart at poettering.net>
Date: Wed Apr 8 22:39:40 2015 +0200
update TODO
diff --git a/TODO b/TODO
index 39c6548..c787b54 100644
--- a/TODO
+++ b/TODO
@@ -41,8 +41,14 @@ Before 220:
* timer units triggering services with failing conditions run busy:
http://lists.freedesktop.org/archives/systemd-devel/2015-April/030095.html
+* figure out what to do about systemd.pc
+
Features:
+* check for the various fs-specific ioctls we call that we do so only
+ after verifying they are regular files or directories, and not
+ device files, so that we don't confuse drivers.
+
* docs: bring http://www.freedesktop.org/wiki/Software/systemd/MyServiceCantGetRealtime up to date
* systemctl should set EFI firmware flag via logind
commit 88ec4dfa289cd97496dbb9670365a3d4be13d41c
Author: Lennart Poettering <lennart at poettering.net>
Date: Wed Apr 8 22:35:52 2015 +0200
tmpfiles: rework file attribute code
- Stick to one type for the flags field: unsigned. This appears to be
what the kernel uses, and there's no point in using something else.
- compress the flags array by avoiding sparse entries
- extend some error messages to not use abbreviated words
- avoid TTOCTTOU issues by invoking fstat() after open() when applying
file flags
- add explanation why we need to check the file type with fstat().
- don't needlessly abbreviate "attribute" as "attrib", in particually as
"chattr" abbreviates it as "attr" rather than "attrib".
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 16ed100..decc0a8 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -88,8 +88,8 @@ typedef enum ItemType {
ADJUST_MODE = 'm', /* legacy, 'z' is identical to this */
RELABEL_PATH = 'z',
RECURSIVE_RELABEL_PATH = 'Z',
- SET_ATTRIB = 'h',
- RECURSIVE_SET_ATTRIB = 'H',
+ SET_ATTRIBUTE = 'h',
+ RECURSIVE_SET_ATTRIBUTE = 'H',
} ItemType;
typedef struct Item {
@@ -108,15 +108,15 @@ typedef struct Item {
usec_t age;
dev_t major_minor;
- unsigned long attrib_value;
- unsigned long attrib_mask;
+ unsigned attribute_value;
+ unsigned attribute_mask;
bool uid_set:1;
bool gid_set:1;
bool mode_set:1;
bool age_set:1;
bool mask_perms:1;
- bool attrib_set:1;
+ bool attribute_set:1;
bool keep_first_level:1;
@@ -764,122 +764,143 @@ static int path_set_acls(Item *item, const char *path) {
return r;
}
-#define ALL_ATTRIBS \
- 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 get_attrib_from_arg(Item *item) {
- static const unsigned attributes[] = {
- [(uint8_t)'A'] = FS_NOATIME_FL, /* do not update atime */
- [(uint8_t)'S'] = FS_SYNC_FL, /* Synchronous updates */
- [(uint8_t)'D'] = FS_DIRSYNC_FL, /* dirsync behaviour (directories only) */
- [(uint8_t)'a'] = FS_APPEND_FL, /* writes to file may only append */
- [(uint8_t)'c'] = FS_COMPR_FL, /* Compress file */
- [(uint8_t)'d'] = FS_NODUMP_FL, /* do not dump file */
- [(uint8_t)'e'] = FS_EXTENT_FL, /* Top of directory hierarchies*/
- [(uint8_t)'i'] = FS_IMMUTABLE_FL, /* Immutable file */
- [(uint8_t)'j'] = FS_JOURNAL_DATA_FL, /* Reserved for ext3 */
- [(uint8_t)'s'] = FS_SECRM_FL, /* Secure deletion */
- [(uint8_t)'u'] = FS_UNRM_FL, /* Undelete */
- [(uint8_t)'t'] = FS_NOTAIL_FL, /* file tail should not be merged */
- [(uint8_t)'T'] = FS_TOPDIR_FL, /* Top of directory hierarchies*/
- [(uint8_t)'C'] = FS_NOCOW_FL, /* Do not cow file */
+#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 get_attribute_from_arg(Item *item) {
+
+ static const struct {
+ char character;
+ unsigned value;
+ } attributes[] = {
+ { 'A', FS_NOATIME_FL }, /* do not update atime */
+ { 'S', FS_SYNC_FL }, /* Synchronous updates */
+ { 'D', FS_DIRSYNC_FL }, /* dirsync behaviour (directories only) */
+ { 'a', FS_APPEND_FL }, /* writes to file may only append */
+ { 'c', FS_COMPR_FL }, /* Compress file */
+ { 'd', FS_NODUMP_FL }, /* do not dump file */
+ { 'e', FS_EXTENT_FL }, /* Top of directory hierarchies*/
+ { 'i', FS_IMMUTABLE_FL }, /* Immutable file */
+ { 'j', FS_JOURNAL_DATA_FL }, /* Reserved for ext3 */
+ { 's', FS_SECRM_FL }, /* Secure deletion */
+ { 'u', FS_UNRM_FL }, /* Undelete */
+ { '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 */
};
- char *p = item->argument;
+
enum {
MODE_ADD,
MODE_DEL,
MODE_SET
} mode = MODE_ADD;
- unsigned long value = 0, mask = 0;
- if (!p) {
- log_error("\"%s\": setting ATTR need an argument", item->path);
- return -EINVAL;
- }
+ unsigned value = 0, mask = 0;
+ const char *p;
- if (*p == '+') {
- mode = MODE_ADD;
- p++;
- } else if (*p == '-') {
- mode = MODE_DEL;
- p++;
- } else if (*p == '=') {
- mode = MODE_SET;
- p++;
+ assert(item);
+
+ p = item->argument;
+ if (p) {
+ if (*p == '+') {
+ mode = MODE_ADD;
+ p++;
+ } else if (*p == '-') {
+ mode = MODE_DEL;
+ p++;
+ } else if (*p == '=') {
+ mode = MODE_SET;
+ p++;
+ }
}
- if (!*p && mode != MODE_SET) {
- log_error("\"%s\": setting ATTR: argument is empty", item->path);
+ if (isempty(p) && mode != MODE_SET) {
+ log_error("Setting file attribute on '%s' needs an attribute specification.", item->path);
return -EINVAL;
}
- for (; *p ; p++) {
- if ((uint8_t)*p >= ELEMENTSOF(attributes) || attributes[(uint8_t)*p] == 0) {
- log_error("\"%s\": setting ATTR: unknown attr '%c'", item->path, *p);
+
+ for (; p && *p ; p++) {
+ unsigned i, v;
+
+ for (i = 0; i < ELEMENTSOF(attributes); i++)
+ if (*p == attributes[i].character)
+ break;
+
+ if (i >= ELEMENTSOF(attributes)) {
+ log_error("Unknown file attribute '%c' on '%s'.", *p, item->path);
return -EINVAL;
}
+
+ v = attributes[i].value;
+
if (mode == MODE_ADD || mode == MODE_SET)
- value |= attributes[(uint8_t)*p];
+ value |= v;
else
- value &= ~attributes[(uint8_t)*p];
- mask |= attributes[(uint8_t)*p];
+ value &= ~v;
+
+ mask |= v;
}
if (mode == MODE_SET)
- mask |= ALL_ATTRIBS;
+ mask |= ATTRIBUTES_ALL;
- assert(mask);
+ assert(mask != 0);
- item->attrib_mask = mask;
- item->attrib_value = value;
- item->attrib_set = true;
+ item->attribute_mask = mask;
+ item->attribute_value = value;
+ item->attribute_set = true;
return 0;
-
}
-static int path_set_attrib(Item *item, const char *path) {
+static int path_set_attribute(Item *item, const char *path) {
_cleanup_close_ int fd = -1;
- int r;
- unsigned f;
struct stat st;
+ unsigned f;
+ int r;
- /* do nothing */
- if (item->attrib_mask == 0 || !item->attrib_set)
- return 0;
- /*
- * It is OK to ignore an lstat() error, because the error
- * will be catch by the open() below anyway
- */
- if (lstat(path, &st) == 0 &&
- !S_ISREG(st.st_mode) && !S_ISDIR(st.st_mode)) {
+ if (!item->attribute_set || item->attribute_mask == 0)
return 0;
- }
fd = open(path, O_RDONLY|O_NONBLOCK|O_CLOEXEC);
if (fd < 0)
- return log_error_errno(errno, "Cannot open \"%s\": %m", path);
+ return log_error_errno(errno, "Cannot open '%s': %m", path);
- f = item->attrib_value & item->attrib_mask;
+ if (fstat(fd, &st) < 0)
+ return log_error_errno(errno, "Cannot stat '%s': %m", path);
+
+ /* Issuing the file attribute ioctls on device nodes is not
+ * safe, as that will be delivered to the drivers, not the
+ * file system containing the device node. */
+ if (!S_ISREG(st.st_mode) && !S_ISDIR(st.st_mode)) {
+ log_error("Setting file flags is only supported on regular files and directories, cannot set on '%s'.", path);
+ return -EINVAL;
+ }
+
+ f = item->attribute_value & item->attribute_mask;
+
+ /* Mask away directory-specific flags */
if (!S_ISDIR(st.st_mode))
f &= ~FS_DIRSYNC_FL;
- r = chattr_fd(fd, f, item->attrib_mask);
+
+ r = chattr_fd(fd, f, item->attribute_mask);
if (r < 0)
- return log_error_errno(errno,
- "Cannot set attrib for \"%s\", value=0x%08lx, mask=0x%08lx: %m",
- path, item->attrib_value, item->attrib_mask);
+ return log_error_errno(r,
+ "Cannot set file attribute for '%s', value=0x%08x, mask=0x%08x: %m",
+ path, item->attribute_value, item->attribute_mask);
return 0;
}
@@ -1325,14 +1346,14 @@ static int create_item(Item *i) {
return r;
break;
- case SET_ATTRIB:
- r = glob_item(i, path_set_attrib, false);
+ case SET_ATTRIBUTE:
+ r = glob_item(i, path_set_attribute, false);
if (r < 0)
return r;
break;
- case RECURSIVE_SET_ATTRIB:
- r = glob_item(i, path_set_attrib, true);
+ case RECURSIVE_SET_ATTRIBUTE:
+ r = glob_item(i, path_set_attribute, true);
if (r < 0)
return r;
break;
@@ -1400,8 +1421,8 @@ static int remove_item(Item *i) {
case RECURSIVE_SET_XATTR:
case SET_ACL:
case RECURSIVE_SET_ACL:
- case SET_ATTRIB:
- case RECURSIVE_SET_ATTRIB:
+ case SET_ATTRIBUTE:
+ case RECURSIVE_SET_ATTRIBUTE:
break;
case REMOVE_PATH:
@@ -1786,13 +1807,13 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) {
return r;
break;
- case SET_ATTRIB:
- case RECURSIVE_SET_ATTRIB:
+ case SET_ATTRIBUTE:
+ case RECURSIVE_SET_ATTRIBUTE:
if (!i.argument) {
- log_error("[%s:%u] Set attrib requires argument.", fname, line);
+ log_error("[%s:%u] Set file attribute requires argument.", fname, line);
return -EBADMSG;
}
- r = get_attrib_from_arg(&i);
+ r = get_attribute_from_arg(&i);
if (r < 0)
return r;
break;
commit 1ed8f8c16da99551c6731764759878a0bc243fde
Author: Lennart Poettering <lennart at poettering.net>
Date: Wed Apr 8 20:47:35 2015 +0200
util: merge change_attr_fd() and chattr_fd()
diff --git a/src/import/import-raw.c b/src/import/import-raw.c
index ad8f806..97e1254 100644
--- a/src/import/import-raw.c
+++ b/src/import/import-raw.c
@@ -188,7 +188,7 @@ static int raw_import_maybe_convert_qcow2(RawImport *i) {
if (converted_fd < 0)
return log_error_errno(errno, "Failed to create %s: %m", t);
- r = chattr_fd(converted_fd, true, FS_NOCOW_FL);
+ r = chattr_fd(converted_fd, FS_NOCOW_FL, FS_NOCOW_FL);
if (r < 0)
log_warning_errno(errno, "Failed to set file attributes on %s: %m", t);
@@ -277,7 +277,7 @@ static int raw_import_open_disk(RawImport *i) {
if (i->output_fd < 0)
return log_error_errno(errno, "Failed to open destination %s: %m", i->temp_path);
- r = chattr_fd(i->output_fd, true, FS_NOCOW_FL);
+ r = chattr_fd(i->output_fd, FS_NOCOW_FL, FS_NOCOW_FL);
if (r < 0)
log_warning_errno(errno, "Failed to set file attributes on %s: %m", i->temp_path);
diff --git a/src/import/pull-raw.c b/src/import/pull-raw.c
index ebbe6a9..b65bb0c 100644
--- a/src/import/pull-raw.c
+++ b/src/import/pull-raw.c
@@ -216,7 +216,7 @@ static int raw_pull_maybe_convert_qcow2(RawPull *i) {
if (converted_fd < 0)
return log_error_errno(errno, "Failed to create %s: %m", t);
- r = chattr_fd(converted_fd, true, FS_NOCOW_FL);
+ r = chattr_fd(converted_fd, FS_NOCOW_FL, FS_NOCOW_FL);
if (r < 0)
log_warning_errno(errno, "Failed to set file attributes on %s: %m", t);
@@ -292,7 +292,7 @@ static int raw_pull_make_local_copy(RawPull *i) {
* performance on COW file systems like btrfs, since it
* reduces fragmentation caused by not allowing in-place
* writes. */
- r = chattr_fd(dfd, true, FS_NOCOW_FL);
+ r = chattr_fd(dfd, FS_NOCOW_FL, FS_NOCOW_FL);
if (r < 0)
log_warning_errno(errno, "Failed to set file attributes on %s: %m", tp);
@@ -434,7 +434,7 @@ static int raw_pull_job_on_open_disk(PullJob *j) {
if (j->disk_fd < 0)
return log_error_errno(errno, "Failed to create %s: %m", i->temp_path);
- r = chattr_fd(j->disk_fd, true, FS_NOCOW_FL);
+ r = chattr_fd(j->disk_fd, FS_NOCOW_FL, FS_NOCOW_FL);
if (r < 0)
log_warning_errno(errno, "Failed to set file attributes on %s: %m", i->temp_path);
diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c
index 5a4e7cb..6bbcc6d 100644
--- a/src/journal/journal-file.c
+++ b/src/journal/journal-file.c
@@ -149,7 +149,7 @@ void journal_file_close(JournalFile *f) {
* reenable all the good bits COW usually provides
* (such as data checksumming). */
- (void) chattr_fd(f->fd, false, FS_NOCOW_FL);
+ (void) chattr_fd(f->fd, 0, FS_NOCOW_FL);
(void) btrfs_defrag_fd(f->fd);
}
@@ -2608,7 +2608,7 @@ int journal_file_open(
* the expense of data integrity features (which
* shouldn't be too bad, given that we do our own
* checksumming). */
- r = chattr_fd(f->fd, true, FS_NOCOW_FL);
+ r = chattr_fd(f->fd, FS_NOCOW_FL, FS_NOCOW_FL);
if (r < 0 && r != -ENOTTY)
log_warning_errno(r, "Failed to set file attributes: %m");
diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index b4f88bc..40c84b9 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -1377,7 +1377,7 @@ static int setup_keys(void) {
/* Enable secure remove, exclusion from dump, synchronous
* writing and in-place updating */
- r = chattr_fd(fd, true, FS_SECRM_FL|FS_NODUMP_FL|FS_SYNC_FL|FS_NOCOW_FL);
+ r = chattr_fd(fd, FS_SECRM_FL|FS_NODUMP_FL|FS_SYNC_FL|FS_NOCOW_FL, FS_SECRM_FL|FS_NODUMP_FL|FS_SYNC_FL|FS_NOCOW_FL);
if (r < 0)
log_warning_errno(errno, "Failed to set file attributes: %m");
diff --git a/src/shared/copy.c b/src/shared/copy.c
index 775a339..1282cb8 100644
--- a/src/shared/copy.c
+++ b/src/shared/copy.c
@@ -372,7 +372,7 @@ int copy_file(const char *from, const char *to, int flags, mode_t mode, unsigned
}
if (chattr_flags != 0)
- (void) chattr_fd(fdt, true, chattr_flags);
+ (void) chattr_fd(fdt, chattr_flags, (unsigned) -1);
r = copy_file_fd(from, fdt, true);
if (r < 0) {
diff --git a/src/shared/util.c b/src/shared/util.c
index de56d98..da75667 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -7678,7 +7678,7 @@ int same_fd(int a, int b) {
return fa == fb;
}
-int chattr_fd(int fd, bool b, unsigned mask) {
+int chattr_fd(int fd, unsigned value, unsigned mask) {
unsigned old_attr, new_attr;
assert(fd >= 0);
@@ -7689,21 +7689,17 @@ int chattr_fd(int fd, bool b, unsigned mask) {
if (ioctl(fd, FS_IOC_GETFLAGS, &old_attr) < 0)
return -errno;
- if (b)
- new_attr = old_attr | mask;
- else
- new_attr = old_attr & ~mask;
-
+ new_attr = (old_attr & ~mask) | (value & mask);
if (new_attr == old_attr)
return 0;
if (ioctl(fd, FS_IOC_SETFLAGS, &new_attr) < 0)
return -errno;
- return 0;
+ return 1;
}
-int chattr_path(const char *p, bool b, unsigned mask) {
+int chattr_path(const char *p, unsigned value, unsigned mask) {
_cleanup_close_ int fd = -1;
assert(p);
@@ -7715,29 +7711,7 @@ int chattr_path(const char *p, bool b, unsigned mask) {
if (fd < 0)
return -errno;
- return chattr_fd(fd, b, mask);
-}
-
-int change_attr_fd(int fd, unsigned value, unsigned mask) {
- unsigned old_attr, new_attr;
-
- assert(fd >= 0);
-
- if (mask == 0)
- return 0;
-
- if (ioctl(fd, FS_IOC_GETFLAGS, &old_attr) < 0)
- return -errno;
-
- new_attr = (old_attr & ~mask) |(value & mask);
-
- if (new_attr == old_attr)
- return 0;
-
- if (ioctl(fd, FS_IOC_SETFLAGS, &new_attr) < 0)
- return -errno;
-
- return 0;
+ return chattr_fd(fd, value, mask);
}
int read_attr_fd(int fd, unsigned *ret) {
diff --git a/src/shared/util.h b/src/shared/util.h
index e7a5d63..b41090b 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -1058,9 +1058,8 @@ int fd_getcrtime_at(int dirfd, const char *name, usec_t *usec, int flags);
int same_fd(int a, int b);
-int chattr_fd(int fd, bool b, unsigned mask);
-int chattr_path(const char *p, bool b, unsigned mask);
-int change_attr_fd(int fd, unsigned value, unsigned mask);
+int chattr_fd(int fd, unsigned value, unsigned mask);
+int chattr_path(const char *p, unsigned value, unsigned mask);
int read_attr_fd(int fd, unsigned *ret);
int read_attr_path(const char *p, unsigned *ret);
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 8c89fb3..16ed100 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -869,14 +869,13 @@ static int path_set_attrib(Item *item, const char *path) {
}
fd = open(path, O_RDONLY|O_NONBLOCK|O_CLOEXEC);
-
if (fd < 0)
return log_error_errno(errno, "Cannot open \"%s\": %m", path);
f = item->attrib_value & item->attrib_mask;
if (!S_ISDIR(st.st_mode))
f &= ~FS_DIRSYNC_FL;
- r = change_attr_fd(fd, f, item->attrib_mask);
+ r = chattr_fd(fd, f, item->attrib_mask);
if (r < 0)
return log_error_errno(errno,
"Cannot set attrib for \"%s\", value=0x%08lx, mask=0x%08lx: %m",
More information about the systemd-commits
mailing list