[systemd-commits] 6 commits - TODO src/journal src/libsystemd src/shared
Lennart Poettering
lennart at kemper.freedesktop.org
Wed Apr 22 04:33:00 PDT 2015
TODO | 19 -------
src/journal/journal-file.c | 46 +++++++++++++----
src/libsystemd/sd-bus/bus-creds.c | 11 ++--
src/shared/btrfs-util.c | 100 +++++++++++++++++++++++++++++++++++---
src/shared/btrfs-util.h | 3 -
src/shared/machine-image.c | 9 +--
src/shared/util.c | 21 +++++++
7 files changed, 163 insertions(+), 46 deletions(-)
New commits:
commit a4b756b1ed4ae9983b9d4df77973679480870c8b
Author: Lennart Poettering <lennart at poettering.net>
Date: Wed Apr 22 13:30:42 2015 +0200
Update TODO
diff --git a/TODO b/TODO
index b0e7ea6..d0a65ce 100644
--- a/TODO
+++ b/TODO
@@ -44,8 +44,6 @@ 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:
* rework C11 utf8.[ch] to use char32_t instead of uint32_t when referring
@@ -65,14 +63,8 @@ Features:
* fstab-generator: default to tmpfs-as-root if only usr= is specified on the kernel cmdline
-* 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
-
* mounting and unmounting mount points manually with different source
devices will result in collected collected on all devices used.
http://lists.freedesktop.org/archives/systemd-devel/2015-April/030225.html
@@ -739,17 +731,6 @@ Features:
* when breaking cycles drop sysv services first, then services from /run, then from /etc, then from /usr
-* automount: implement expire:
- - set superblock timeout AUTOFS_DEV_IOCTL_TIMEOUT_CMD
- - periodically run AUTOFS_DEV_IOCTL_EXPIRE_CMD
- - every timeout/4 (original autofs logic)
- - blocking, needs a thread
- - run until -EAGAIN
- - receive expire packet on pipe if kernel tells the timeout is over
- - call umount
- - answer expire packet on pipe with AUTOFS_DEV_IOCTL_{READY,FAIL}_CMD
- - AUTOFS_DEV_IOCTL_EXPIRE_CMD returns
-
* ExecOnFailure=/usr/bin/foo
* udev:
commit 3a698817043ca8f4059c52b7ff30097462b1f7bc
Author: Lennart Poettering <lennart at poettering.net>
Date: Wed Apr 22 13:27:29 2015 +0200
sd-bus: handle ppid=0 more gracefully (which happens for pid=1)
diff --git a/src/libsystemd/sd-bus/bus-creds.c b/src/libsystemd/sd-bus/bus-creds.c
index 6cb47f5..a68b0d0 100644
--- a/src/libsystemd/sd-bus/bus-creds.c
+++ b/src/libsystemd/sd-bus/bus-creds.c
@@ -768,11 +768,14 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) {
if (p) {
p += strspn(p, WHITESPACE);
- r = parse_pid(p, &c->ppid);
- if (r < 0)
- return r;
+ /* Explicitly check for PPID 0 (which is the case for PID 1) */
+ if (!streq(p, "0")) {
+ r = parse_pid(p, &c->ppid);
+ if (r < 0)
+ return r;
- c->mask |= SD_BUS_CREDS_PPID;
+ c->mask |= SD_BUS_CREDS_PPID;
+ }
continue;
}
}
commit fc68c92973e5437ee0489c1bc80d80f0a7b6ca0b
Author: Lennart Poettering <lennart at poettering.net>
Date: Wed Apr 22 13:20:49 2015 +0200
journal: don't force FS_NOCOW_FL on new journal files, but warn if it is missing
This way users have the freedom to set or unset the FS_NOCOW_FL flag on
their journal files by setting it on the journal directory. Since our
default tmpfiles configuration now sets this flag on the directory the
flag is set by default on new files, however people can opt-out of this
by masking the tmpfiles file for it.
diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c
index a432eb0..be6a552 100644
--- a/src/journal/journal-file.c
+++ b/src/journal/journal-file.c
@@ -2522,6 +2522,41 @@ void journal_file_print_header(JournalFile *f) {
printf("Disk usage: %s\n", format_bytes(bytes, sizeof(bytes), (off_t) st.st_blocks * 512ULL));
}
+static int journal_file_warn_btrfs(JournalFile *f) {
+ unsigned attrs;
+ int r;
+
+ assert(f);
+
+ /* Before we write anything, check if the COW logic is turned
+ * off on btrfs. Given our write pattern that is quite
+ * unfriendly to COW file systems this should greatly improve
+ * performance on COW file systems, such as btrfs, at the
+ * expense of data integrity features (which shouldn't be too
+ * bad, given that we do our own checksumming). */
+
+ r = btrfs_is_filesystem(f->fd);
+ if (r < 0)
+ return log_warning_errno(r, "Failed to determine if journal is on btrfs: %m");
+ if (!r)
+ return 0;
+
+ r = read_attr_fd(f->fd, &attrs);
+ if (r < 0)
+ return log_warning_errno(r, "Failed to read file attributes: %m");
+
+ if (attrs & FS_NOCOW_FL) {
+ log_debug("Detected btrfs file system with copy-on-write disabled, all is good.");
+ return 0;
+ }
+
+ log_notice("Creating journal file %s on a btrfs file system, and copy-on-write is enabled. "
+ "This is likely to slow down journal access substantially, please consider turning "
+ "off the copy-on-write file attribute on the journal directory, using chattr +C.", f->path);
+
+ return 1;
+}
+
int journal_file_open(
const char *fname,
int flags,
@@ -2602,16 +2637,7 @@ int journal_file_open(
if (f->last_stat.st_size == 0 && f->writable) {
- /* Before we write anything, turn off COW logic. Given
- * our write pattern that is quite unfriendly to COW
- * file systems this should greatly improve
- * performance on COW file systems, such as btrfs, at
- * the expense of data integrity features (which
- * shouldn't be too bad, given that we do our own
- * checksumming). */
- 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");
+ (void) journal_file_warn_btrfs(f);
/* Let's attach the creation time to the journal file,
* so that the vacuuming code knows the age of this
commit 625728941d02d0556856d7ef92979f1a6f5edbf3
Author: Lennart Poettering <lennart at poettering.net>
Date: Wed Apr 22 13:11:19 2015 +0200
btrfs-util: be more careful when invoking btrfs file system ioctls
If we get passed an fd that does not refer to a regular file or
directory, we should not issue btrfs ioctls on it, since it might end up
in a device driver or similar (note that DRM for example uses the same
ioctl numbers as some file system ioctls).
Hence, let's make sure to always check if something is a regular file or
directory, or is on btrfs before invoking the respective ioctls. It's
better to be safe than sorry.
diff --git a/src/shared/btrfs-util.c b/src/shared/btrfs-util.c
index 5a1ed60..ac1907d 100644
--- a/src/shared/btrfs-util.c
+++ b/src/shared/btrfs-util.c
@@ -38,6 +38,14 @@
#include "btrfs-ctree.h"
#include "btrfs-util.h"
+/* WARNING: Be careful with file system ioctls! When we get an fd, we
+ * need to make sure it either refers to only a regular file or
+ * directory, or that it is located on btrfs, before invoking any
+ * btrfs ioctls. The ioctl numbers are reused by some device drivers
+ * (such as DRM), and hence might have bad effects when invoked on
+ * device nodes (that reference drivers) rather than fds to normal
+ * files or directories. */
+
static int validate_subvolume_name(const char *name) {
if (!filename_is_valid(name))
@@ -193,6 +201,15 @@ int btrfs_subvol_set_read_only(const char *path, bool b) {
int btrfs_subvol_get_read_only_fd(int fd) {
uint64_t flags;
+ struct stat st;
+
+ assert(fd >= 0);
+
+ if (fstat(fd, &st) < 0)
+ return -errno;
+
+ if (!S_ISDIR(st.st_mode) || st.st_ino != 256)
+ return -EINVAL;
if (ioctl(fd, BTRFS_IOC_SUBVOL_GETFLAGS, &flags) < 0)
return -errno;
@@ -201,11 +218,21 @@ int btrfs_subvol_get_read_only_fd(int fd) {
}
int btrfs_reflink(int infd, int outfd) {
+ struct stat st;
int r;
assert(infd >= 0);
assert(outfd >= 0);
+ /* Make sure we invoke the ioctl on a regular file, so that no
+ * device driver accidentally gets it. */
+
+ if (fstat(outfd, &st) < 0)
+ return -errno;
+
+ if (!S_ISREG(st.st_mode))
+ return -EINVAL;
+
r = ioctl(outfd, BTRFS_IOC_CLONE, infd);
if (r < 0)
return -errno;
@@ -220,12 +247,19 @@ int btrfs_clone_range(int infd, uint64_t in_offset, int outfd, uint64_t out_offs
.src_length = sz,
.dest_offset = out_offset,
};
+ struct stat st;
int r;
assert(infd >= 0);
assert(outfd >= 0);
assert(sz > 0);
+ if (fstat(outfd, &st) < 0)
+ return -errno;
+
+ if (!S_ISREG(st.st_mode))
+ return -EINVAL;
+
r = ioctl(outfd, BTRFS_IOC_CLONE_RANGE, &args);
if (r < 0)
return -errno;
@@ -236,10 +270,17 @@ int btrfs_clone_range(int infd, uint64_t in_offset, int outfd, uint64_t out_offs
int btrfs_get_block_device_fd(int fd, dev_t *dev) {
struct btrfs_ioctl_fs_info_args fsi = {};
uint64_t id;
+ int r;
assert(fd >= 0);
assert(dev);
+ r = btrfs_is_filesystem(fd);
+ if (r < 0)
+ return r;
+ if (!r)
+ return -ENOTTY;
+
if (ioctl(fd, BTRFS_IOC_FS_INFO, &fsi) < 0)
return -errno;
@@ -293,10 +334,17 @@ int btrfs_subvol_get_id_fd(int fd, uint64_t *ret) {
struct btrfs_ioctl_ino_lookup_args args = {
.objectid = BTRFS_FIRST_FREE_OBJECTID
};
+ int r;
assert(fd >= 0);
assert(ret);
+ r = btrfs_is_filesystem(fd);
+ if (r < 0)
+ return r;
+ if (!r)
+ return -ENOTTY;
+
if (ioctl(fd, BTRFS_IOC_INO_LOOKUP, &args) < 0)
return -errno;
@@ -562,8 +610,16 @@ finish:
}
int btrfs_defrag_fd(int fd) {
+ struct stat st;
+
assert(fd >= 0);
+ if (fstat(fd, &st) < 0)
+ return -errno;
+
+ if (!S_ISREG(st.st_mode))
+ return -EINVAL;
+
if (ioctl(fd, BTRFS_IOC_DEFRAG, NULL) < 0)
return -errno;
@@ -584,9 +640,16 @@ int btrfs_quota_enable_fd(int fd, bool b) {
struct btrfs_ioctl_quota_ctl_args args = {
.cmd = b ? BTRFS_QUOTA_CTL_ENABLE : BTRFS_QUOTA_CTL_DISABLE,
};
+ int r;
assert(fd >= 0);
+ r = btrfs_is_filesystem(fd);
+ if (r < 0)
+ return r;
+ if (!r)
+ return -ENOTTY;
+
if (ioctl(fd, BTRFS_IOC_QUOTA_CTL, &args) < 0)
return -errno;
@@ -610,9 +673,16 @@ int btrfs_quota_limit_fd(int fd, uint64_t referenced_max) {
referenced_max == 0 ? 1 : referenced_max,
.lim.flags = BTRFS_QGROUP_LIMIT_MAX_RFER,
};
+ int r;
assert(fd >= 0);
+ r = btrfs_is_filesystem(fd);
+ if (r < 0)
+ return r;
+ if (!r)
+ return -ENOTTY;
+
if (ioctl(fd, BTRFS_IOC_QGROUP_LIMIT, &args) < 0)
return -errno;
@@ -732,11 +802,18 @@ static int subvol_remove_children(int fd, const char *subvolume, uint64_t subvol
struct btrfs_ioctl_vol_args vol_args = {};
_cleanup_close_ int subvol_fd = -1;
+ struct stat st;
int r;
assert(fd >= 0);
assert(subvolume);
+ if (fstat(fd, &st) < 0)
+ return -errno;
+
+ if (!S_ISDIR(st.st_mode))
+ return -EINVAL;
+
/* First, try to remove the subvolume. If it happens to be
* already empty, this will just work. */
strncpy(vol_args.name, subvolume, sizeof(vol_args.name)-1);
commit 21222ea5cdec65fa30a75bd5a78475459075b946
Author: Lennart Poettering <lennart at poettering.net>
Date: Wed Apr 22 13:08:19 2015 +0200
btrfs-util: introduce btrfs_is_filesystem() and make use of it where appropriate
Let's unify the code that checks whether an fd is on btrfs a bit.
(Also, rename btrfs_is_snapshot() to btrfs_is_subvol(), since that's
usually how this is referred to in our code)
diff --git a/src/shared/btrfs-util.c b/src/shared/btrfs-util.c
index 5bf87a3..5a1ed60 100644
--- a/src/shared/btrfs-util.c
+++ b/src/shared/btrfs-util.c
@@ -83,10 +83,22 @@ static int extract_subvolume_name(const char *path, const char **subvolume) {
return 0;
}
-int btrfs_is_snapshot(int fd) {
- struct stat st;
+int btrfs_is_filesystem(int fd) {
struct statfs sfs;
+ assert(fd >= 0);
+
+ if (fstatfs(fd, &sfs) < 0)
+ return -errno;
+
+ return F_TYPE_EQUAL(sfs.f_type, BTRFS_SUPER_MAGIC);
+}
+
+int btrfs_is_subvol(int fd) {
+ struct stat st;
+
+ assert(fd >= 0);
+
/* On btrfs subvolumes always have the inode 256 */
if (fstat(fd, &st) < 0)
@@ -95,10 +107,7 @@ int btrfs_is_snapshot(int fd) {
if (!S_ISDIR(st.st_mode) || st.st_ino != 256)
return 0;
- if (fstatfs(fd, &sfs) < 0)
- return -errno;
-
- return F_TYPE_EQUAL(sfs.f_type, BTRFS_SUPER_MAGIC);
+ return btrfs_is_filesystem(fd);
}
int btrfs_subvol_make(const char *path) {
@@ -970,7 +979,7 @@ int btrfs_subvol_snapshot_fd(int old_fd, const char *new_path, BtrfsSnapshotFlag
assert(old_fd >= 0);
assert(new_path);
- r = btrfs_is_snapshot(old_fd);
+ r = btrfs_is_subvol(old_fd);
if (r < 0)
return r;
if (r == 0) {
diff --git a/src/shared/btrfs-util.h b/src/shared/btrfs-util.h
index 02e46e3..a7eb895 100644
--- a/src/shared/btrfs-util.h
+++ b/src/shared/btrfs-util.h
@@ -49,7 +49,8 @@ typedef enum BtrfsSnapshotFlags {
BTRFS_SNAPSHOT_RECURSIVE = 4,
} BtrfsSnapshotFlags;
-int btrfs_is_snapshot(int fd);
+int btrfs_is_filesystem(int fd);
+int btrfs_is_subvol(int fd);
int btrfs_subvol_make(const char *path);
int btrfs_subvol_make_label(const char *path);
diff --git a/src/shared/machine-image.c b/src/shared/machine-image.c
index 0b41860..bc215f0 100644
--- a/src/shared/machine-image.c
+++ b/src/shared/machine-image.c
@@ -136,12 +136,11 @@ static int image_make(
/* btrfs subvolumes have inode 256 */
if (st.st_ino == 256) {
- struct statfs sfs;
- if (fstatfs(fd, &sfs) < 0)
- return -errno;
-
- if (F_TYPE_EQUAL(sfs.f_type, BTRFS_SUPER_MAGIC)) {
+ r = btrfs_is_filesystem(fd);
+ if (r < 0)
+ return r;
+ if (r) {
BtrfsSubvolInfo info;
BtrfsQuotaInfo quota;
commit 03091baac3c37094af6a30be31e4962dd26f8404
Author: Lennart Poettering <lennart at poettering.net>
Date: Wed Apr 22 13:05:26 2015 +0200
util: make sure fd refers to regular file or directory when applying file attributes
Before invoking file system ioctls we need to make sure that the
specified fd actually refers to a file system object, and not a device
node or similar. Otherwise we might by accident invoke unrelated device
driver ioctls. For example, DRM ioctls use the same ioctl numbers as the
various file system ioctls.
diff --git a/src/shared/util.c b/src/shared/util.c
index f14d9ee..4a04484 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -5988,9 +5988,22 @@ int same_fd(int a, int b) {
int chattr_fd(int fd, unsigned value, unsigned mask) {
unsigned old_attr, new_attr;
+ struct stat st;
assert(fd >= 0);
+ if (fstat(fd, &st) < 0)
+ return -errno;
+
+ /* Explicitly check whether this is a regular file or
+ * directory. If it is anything else (such as a device node or
+ * fifo), then the ioctl will not hit the file systems but
+ * possibly drivers, where the ioctl might have different
+ * effects. Notably, DRM is using the same ioctl() number. */
+
+ if (!S_ISDIR(st.st_mode) && !S_ISREG(st.st_mode))
+ return -ENOTTY;
+
if (mask == 0)
return 0;
@@ -6023,8 +6036,16 @@ int chattr_path(const char *p, unsigned value, unsigned mask) {
}
int read_attr_fd(int fd, unsigned *ret) {
+ struct stat st;
+
assert(fd >= 0);
+ if (fstat(fd, &st) < 0)
+ return -errno;
+
+ if (!S_ISDIR(st.st_mode) && !S_ISREG(st.st_mode))
+ return -ENOTTY;
+
if (ioctl(fd, FS_IOC_GETFLAGS, ret) < 0)
return -errno;
More information about the systemd-commits
mailing list