From c53e07e249d4635babe901fd40dc1ecdfc32f0d4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 Oct 2021 16:31:49 +0200 Subject: [PATCH 1/8] xattr-util: merge various getxattr()/listxattr() helpers into getxattr_at_malloc() + listxattr_at_malloc() Unfortunately fgetxattr() and flistxattr() don't work via O_PATH fds. Let's thus add fallbacks to go via /proc/self/fd/ in these cases. Also, let's merge all the various flavours we have here into singular implementations that can do everything we need: 1. malloc() loop handling 2. by fd, by path, or combination (i.e. a proper openat() like API) 3. work on O_PATH --- src/basic/cgroup-util.c | 2 +- src/basic/os-util.c | 2 +- src/basic/xattr-util.c | 286 ++++++++++----------- src/basic/xattr-util.h | 44 ++-- src/libsystemd/sd-journal/journal-vacuum.c | 20 +- src/shared/cgroup-show.c | 2 +- src/shared/discover-image.c | 2 +- src/shared/dissect-image.c | 4 +- src/shared/smack-util.c | 4 +- src/test/test-copy.c | 2 +- src/test/test-xattr-util.c | 23 +- 11 files changed, 193 insertions(+), 198 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 221157d57a..1d577a24ec 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -669,7 +669,7 @@ int cg_get_xattr_malloc(const char *controller, const char *path, const char *na if (r < 0) return r; - r = getxattr_malloc(fs, name, ret, false); + r = lgetxattr_malloc(fs, name, ret); if (r < 0) return r; diff --git a/src/basic/os-util.c b/src/basic/os-util.c index d161f8d33c..2a218ad145 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -118,7 +118,7 @@ int open_extension_release(const char *root, const char *extension, char **ret_p /* No xattr or cannot parse it? Then skip this. */ _cleanup_free_ char *extension_release_xattr = NULL; - k = fgetxattrat_fake_malloc(extension_release_fd, NULL, "user.extension-release.strict", AT_EMPTY_PATH, &extension_release_xattr); + k = fgetxattr_malloc(extension_release_fd, "user.extension-release.strict", &extension_release_xattr); if (k < 0 && !ERRNO_IS_NOT_SUPPORTED(k) && k != -ENODATA) log_debug_errno(k, "Failed to read 'user.extension-release.strict' extended attribute from extension-release file %s/%s: %m", diff --git a/src/basic/xattr-util.c b/src/basic/xattr-util.c index 5ae6cf8772..7bb5b3f6df 100644 --- a/src/basic/xattr-util.c +++ b/src/basic/xattr-util.c @@ -18,31 +18,84 @@ #include "time-util.h" #include "xattr-util.h" -int getxattr_malloc( +int getxattr_at_malloc( + int fd, const char *path, const char *name, - char **ret, - bool allow_symlink) { + int flags, + char **ret) { + _cleanup_close_ int opened_fd = -1; + unsigned n_attempts = 7; + bool by_procfs = false; size_t l = 100; - assert(path); + assert(fd >= 0 || fd == AT_FDCWD); assert(name); + assert((flags & ~(AT_SYMLINK_FOLLOW|AT_EMPTY_PATH)) == 0); assert(ret); + /* So, this is single function that does what getxattr()/lgetxattr()/fgetxattr() does, but in one go, + * and with additional bells and whistles. Specifically: + * + * 1. This works on O_PATH fds (which fgetxattr() does not) + * 2. Provides full openat()-style semantics, i.e. by-fd, by-path and combination thereof + * 3. As extension to openat()-style semantics implies AT_EMPTY_PATH if path is NULL. + * 4. Does a malloc() loop, automatically sizing the allocation + * 5. NUL-terminates the returned buffer (for safety) + */ + + if (!path) /* If path is NULL, imply AT_EMPTY_PATH. – But if it's "", don't — for safety reasons. */ + flags |= AT_EMPTY_PATH; + + if (isempty(path)) { + if (!FLAGS_SET(flags, AT_EMPTY_PATH)) + return -EINVAL; + + if (fd == AT_FDCWD) /* Both unspecified? Then operate on current working directory */ + path = "."; + else + path = NULL; + + } else if (fd != AT_FDCWD) { + + /* If both have been specified, then we go via O_PATH */ + opened_fd = openat(fd, path, O_PATH|O_CLOEXEC|(FLAGS_SET(flags, AT_SYMLINK_FOLLOW) ? 0 : O_NOFOLLOW)); + if (opened_fd < 0) + return -errno; + + fd = opened_fd; + path = NULL; + by_procfs = true; /* fgetxattr() is not going to work, go via /proc/ link right-away */ + } + for(;;) { _cleanup_free_ char *v = NULL; ssize_t n; + if (n_attempts == 0) /* If someone is racing against us, give up eventually */ + return -EBUSY; + n_attempts--; + v = new0(char, l+1); if (!v) return -ENOMEM; - if (allow_symlink) - n = lgetxattr(path, name, v, l); + l = MALLOC_ELEMENTSOF(v) - 1; + + if (path) + n = FLAGS_SET(flags, AT_SYMLINK_FOLLOW) ? getxattr(path, name, v, l) : lgetxattr(path, name, v, l); else - n = getxattr(path, name, v, l); + n = by_procfs ? getxattr(FORMAT_PROC_FD_PATH(fd), name, v, l) : fgetxattr(fd, name, v, l); if (n < 0) { + if (errno == EBADF) { + if (by_procfs || path) + return -EBADF; + + by_procfs = true; /* Might be an O_PATH fd, try again via /proc/ link */ + continue; + } + if (errno != ERANGE) return -errno; } else { @@ -51,10 +104,10 @@ int getxattr_malloc( return (int) n; } - if (allow_symlink) - n = lgetxattr(path, name, NULL, 0); + if (path) + n = FLAGS_SET(flags, AT_SYMLINK_FOLLOW) ? getxattr(path, name, NULL, 0) : lgetxattr(path, name, NULL, 0); else - n = getxattr(path, name, NULL, 0); + n = by_procfs ? getxattr(FORMAT_PROC_FD_PATH(fd), name, NULL, 0) : fgetxattr(fd, name, NULL, 0); if (n < 0) return -errno; if (n > INT_MAX) /* We couldn't return this as 'int' anymore */ @@ -64,126 +117,6 @@ int getxattr_malloc( } } -int fgetxattr_malloc( - int fd, - const char *name, - char **ret) { - - size_t l = 100; - - assert(fd >= 0); - assert(name); - assert(ret); - - for (;;) { - _cleanup_free_ char *v = NULL; - ssize_t n; - - v = new(char, l+1); - if (!v) - return -ENOMEM; - - n = fgetxattr(fd, name, v, l); - if (n < 0) { - if (errno != ERANGE) - return -errno; - } else { - v[n] = 0; /* NUL terminate */ - *ret = TAKE_PTR(v); - return (int) n; - } - - n = fgetxattr(fd, name, NULL, 0); - if (n < 0) - return -errno; - if (n > INT_MAX) /* We couldn't return this as 'int' anymore */ - return -E2BIG; - - l = (size_t) n; - } -} - -/* Note: ret_fn should already be allocated for the usual xsprintf and /proc/self/fd/%i pattern. */ -static int getxattrat_fake_prepare( - int dirfd, - const char *filename, - int flags, - char ret_fn[static PROC_FD_PATH_MAX], - int *ret_fd) { - - _cleanup_close_ int fd = -1; - assert(ret_fn); - assert(ret_fd); - - /* The kernel doesn't have a fgetxattrat() command, hence let's emulate one */ - - if (flags & ~(AT_SYMLINK_NOFOLLOW|AT_EMPTY_PATH)) - return -EINVAL; - - if (isempty(filename)) { - if (!(flags & AT_EMPTY_PATH)) - return -EINVAL; - - assert(dirfd >= 0); - - format_proc_fd_path(ret_fn, dirfd); - } else { - fd = openat(dirfd, filename, O_CLOEXEC|O_PATH|(flags & AT_SYMLINK_NOFOLLOW ? O_NOFOLLOW : 0)); - if (fd < 0) - return -errno; - - format_proc_fd_path(ret_fn, fd); - } - - /* Pass the FD to the caller, since in case we do openat() the filename depends on it. */ - *ret_fd = TAKE_FD(fd); - - return 0; -} - -int fgetxattrat_fake( - int dirfd, - const char *filename, - const char *attribute, - void *value, size_t size, - int flags, - size_t *ret_size) { - - _cleanup_close_ int fd = -1; - char fn[PROC_FD_PATH_MAX]; - ssize_t l; - int r; - - r = getxattrat_fake_prepare(dirfd, filename, flags, fn, &fd); - if (r < 0) - return r; - - l = getxattr(fn, attribute, value, size); - if (l < 0) - return -errno; - - *ret_size = l; - return 0; -} - -int fgetxattrat_fake_malloc( - int dirfd, - const char *filename, - const char *attribute, - int flags, - char **value) { - - _cleanup_close_ int fd = -1; - char fn[PROC_FD_PATH_MAX]; - int r; - - r = getxattrat_fake_prepare(dirfd, filename, flags, fn, &fd); - if (r < 0) - return r; - - return getxattr_malloc(fn, attribute, value, false); -} - static int parse_crtime(le64_t le, usec_t *usec) { uint64_t u; @@ -197,17 +130,23 @@ static int parse_crtime(le64_t le, usec_t *usec) { return 0; } -int fd_getcrtime_at(int dirfd, const char *name, usec_t *ret, int flags) { +int fd_getcrtime_at( + int fd, + const char *path, + int flags, + usec_t *ret) { + + _cleanup_free_ le64_t *le = NULL; STRUCT_STATX_DEFINE(sx); usec_t a, b; - le64_t le; - size_t n; int r; + assert(fd >= 0 || fd == AT_FDCWD); + assert((flags & ~(AT_SYMLINK_FOLLOW|AT_EMPTY_PATH)) == 0); assert(ret); - if (flags & ~(AT_EMPTY_PATH|AT_SYMLINK_NOFOLLOW)) - return -EINVAL; + if (!path) + flags |= AT_EMPTY_PATH; /* So here's the deal: the creation/birth time (crtime/btime) of a file is a relatively newly supported concept * on Linux (or more strictly speaking: a concept that only recently got supported in the API, it was @@ -219,7 +158,10 @@ int fd_getcrtime_at(int dirfd, const char *name, usec_t *ret, int flags) { * concept is useful for determining how "old" a file really is, and hence using the older of the two makes * most sense. */ - if (statx(dirfd, strempty(name), flags|AT_STATX_DONT_SYNC, STATX_BTIME, &sx) >= 0 && + if (statx(fd, strempty(path), + (flags & ~AT_SYMLINK_FOLLOW)|(FLAGS_SET(flags, AT_SYMLINK_FOLLOW) ? 0 : AT_SYMLINK_NOFOLLOW)|AT_STATX_DONT_SYNC, + STATX_BTIME, + &sx) >= 0 && (sx.stx_mask & STATX_BTIME) && sx.stx_btime.tv_sec != 0) a = (usec_t) sx.stx_btime.tv_sec * USEC_PER_SEC + @@ -227,12 +169,12 @@ int fd_getcrtime_at(int dirfd, const char *name, usec_t *ret, int flags) { else a = USEC_INFINITY; - r = fgetxattrat_fake(dirfd, name, "user.crtime_usec", &le, sizeof(le), flags, &n); + r = getxattr_at_malloc(fd, path, "user.crtime_usec", flags, (char**) &le); if (r >= 0) { - if (n != sizeof(le)) + if (r != sizeof(*le)) r = -EIO; else - r = parse_crtime(le, &b); + r = parse_crtime(*le, &b); } if (r < 0) { if (a != USEC_INFINITY) { @@ -251,10 +193,6 @@ int fd_getcrtime_at(int dirfd, const char *name, usec_t *ret, int flags) { return 0; } -int fd_getcrtime(int fd, usec_t *ret) { - return fd_getcrtime_at(fd, NULL, ret, AT_EMPTY_PATH); -} - int fd_setcrtime(int fd, usec_t usec) { le64_t le; @@ -270,22 +208,73 @@ int fd_setcrtime(int fd, usec_t usec) { return 0; } -int flistxattr_malloc(int fd, char **ret) { +int listxattr_at_malloc( + int fd, + const char *path, + int flags, + char **ret) { + + _cleanup_close_ int opened_fd = -1; + bool by_procfs = false; + unsigned n_attempts = 7; size_t l = 100; - assert(fd >= 0); + assert(fd >= 0 || fd == AT_FDCWD); + assert((flags & ~(AT_SYMLINK_FOLLOW|AT_EMPTY_PATH)) == 0); assert(ret); + /* This is to listxattr()/llistattr()/flistattr() what getxattr_at_malloc() is to getxattr()/… */ + + if (!path) /* If path is NULL, imply AT_EMPTY_PATH. – But if it's "", don't. */ + flags |= AT_EMPTY_PATH; + + if (isempty(path)) { + if (!FLAGS_SET(flags, AT_EMPTY_PATH)) + return -EINVAL; + + if (fd == AT_FDCWD) /* Both unspecified? Then operate on current working directory */ + path = "."; + else + path = NULL; + + } else if (fd != AT_FDCWD) { + /* If both have been specified, then we go via O_PATH */ + opened_fd = openat(fd, path, O_PATH|O_CLOEXEC|(FLAGS_SET(flags, AT_SYMLINK_FOLLOW) ? 0 : O_NOFOLLOW)); + if (opened_fd < 0) + return -errno; + + fd = opened_fd; + path = NULL; + by_procfs = true; + } + for (;;) { _cleanup_free_ char *v = NULL; ssize_t n; + if (n_attempts == 0) /* If someone is racing against us, give up eventually */ + return -EBUSY; + n_attempts--; + v = new(char, l+1); if (!v) return -ENOMEM; - n = flistxattr(fd, v, l); + l = MALLOC_ELEMENTSOF(v) - 1; + + if (path) + n = FLAGS_SET(flags, AT_SYMLINK_FOLLOW) ? listxattr(path, v, l) : llistxattr(path, v, l); + else + n = by_procfs ? listxattr(FORMAT_PROC_FD_PATH(fd), v, l) : flistxattr(fd, v, l); if (n < 0) { + if (errno == EBADF) { + if (by_procfs || path) + return -EBADF; + + by_procfs = true; /* Might be an O_PATH fd, try again via /proc/ link */ + continue; + } + if (errno != ERANGE) return -errno; } else { @@ -294,7 +283,10 @@ int flistxattr_malloc(int fd, char **ret) { return (int) n; } - n = flistxattr(fd, NULL, 0); + if (path) + n = FLAGS_SET(flags, AT_SYMLINK_FOLLOW) ? listxattr(path, NULL, 0) : llistxattr(path, NULL, 0); + else + n = by_procfs ? listxattr(FORMAT_PROC_FD_PATH(fd), NULL, 0) : flistxattr(fd, NULL, 0); if (n < 0) return -errno; if (n > INT_MAX) /* We couldn't return this as 'int' anymore */ diff --git a/src/basic/xattr-util.h b/src/basic/xattr-util.h index 87caec9800..0eb745a7a3 100644 --- a/src/basic/xattr-util.h +++ b/src/basic/xattr-util.h @@ -7,26 +7,32 @@ #include "time-util.h" -int getxattr_malloc(const char *path, const char *name, char **value, bool allow_symlink); -int fgetxattr_malloc(int fd, const char *name, char **value); - -int fgetxattrat_fake( - int dirfd, - const char *filename, - const char *attribute, - void *value, size_t size, - int flags, - size_t *ret_size); -int fgetxattrat_fake_malloc( - int dirfd, - const char *filename, - const char *attribute, - int flags, - char **value); +int getxattr_at_malloc(int fd, const char *path, const char *name, int flags, char **ret); +static inline int getxattr_malloc(const char *path, const char *name, char **ret) { + return getxattr_at_malloc(AT_FDCWD, path, name, AT_SYMLINK_FOLLOW, ret); +} +static inline int lgetxattr_malloc(const char *path, const char *name, char **ret) { + return getxattr_at_malloc(AT_FDCWD, path, name, 0, ret); +} +static inline int fgetxattr_malloc(int fd, const char *name, char **ret) { + return getxattr_at_malloc(fd, NULL, name, AT_EMPTY_PATH, ret); +} int fd_setcrtime(int fd, usec_t usec); -int fd_getcrtime(int fd, usec_t *usec); -int fd_getcrtime_at(int dirfd, const char *name, usec_t *usec, int flags); +int fd_getcrtime_at(int fd, const char *name, int flags, usec_t *ret); +static inline int fd_getcrtime(int fd, usec_t *ret) { + return fd_getcrtime_at(fd, NULL, 0, ret); +} -int flistxattr_malloc(int fd, char **ret); + +int listxattr_at_malloc(int fd, const char *path, int flags, char **ret); +static inline int listxattr_malloc(const char *path, char **ret) { + return listxattr_at_malloc(AT_FDCWD, path, AT_SYMLINK_FOLLOW, ret); +} +static inline int llistxattr_malloc(const char *path, char **ret) { + return listxattr_at_malloc(AT_FDCWD, path, 0, ret); +} +static inline int flistxattr_malloc(int fd, char **ret) { + return listxattr_at_malloc(fd, NULL, AT_EMPTY_PATH, ret); +} diff --git a/src/libsystemd/sd-journal/journal-vacuum.c b/src/libsystemd/sd-journal/journal-vacuum.c index 6d761b78eb..889caa1905 100644 --- a/src/libsystemd/sd-journal/journal-vacuum.c +++ b/src/libsystemd/sd-journal/journal-vacuum.c @@ -53,11 +53,10 @@ static void patch_realtime( const struct stat *st, unsigned long long *realtime) { - usec_t x, crtime = 0; + usec_t x; - /* The timestamp was determined by the file name, but let's - * see if the file might actually be older than the file name - * suggested... */ + /* The timestamp was determined by the file name, but let's see if the file might actually be older + * than the file name suggested... */ assert(fd >= 0); assert(fn); @@ -76,15 +75,12 @@ static void patch_realtime( if (x > 0 && x != USEC_INFINITY && x < *realtime) *realtime = x; - /* Let's read the original creation time, if possible. Ideally - * we'd just query the creation time the FS might provide, but - * unfortunately there's currently no sane API to query - * it. Hence let's implement this manually... */ + /* Let's read the original creation time, if possible. Ideally we'd just query the creation time the + * FS might provide, but unfortunately there's currently no sane API to query it. Hence let's + * implement this manually... */ - if (fd_getcrtime_at(fd, fn, &crtime, 0) >= 0) { - if (crtime < *realtime) - *realtime = crtime; - } + if (fd_getcrtime_at(fd, fn, AT_SYMLINK_FOLLOW, &x) >= 0 && x < *realtime) + *realtime = x; } static int journal_file_empty(int dir_fd, const char *name) { diff --git a/src/shared/cgroup-show.c b/src/shared/cgroup-show.c index c7e63be508..bdc35492d0 100644 --- a/src/shared/cgroup-show.c +++ b/src/shared/cgroup-show.c @@ -127,7 +127,7 @@ static int show_cgroup_name( bool delegate = false; int r; - r = getxattr_malloc(path, "trusted.delegate", &b, false); + r = lgetxattr_malloc(path, "trusted.delegate", &b); if (r < 0) { if (r != -ENODATA) log_debug_errno(r, "Failed to read trusted.delegate extended attribute: %m"); diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c index 129db21eb0..60833cec45 100644 --- a/src/shared/discover-image.c +++ b/src/shared/discover-image.c @@ -334,7 +334,7 @@ static int image_make( if (!ret) return 0; - (void) fd_getcrtime_at(dfd, filename, &crtime, 0); + (void) fd_getcrtime_at(dfd, filename, AT_SYMLINK_FOLLOW, &crtime); if (!pretty) { r = extract_pretty(filename, ".raw", &pretty_buffer); diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index ccbe1aced4..c734b9609a 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -2765,7 +2765,7 @@ int verity_settings_load( * that doesn't exist for /usr */ if (designator < 0 || designator == PARTITION_ROOT) { - r = getxattr_malloc(image, "user.verity.roothash", &text, true); + r = getxattr_malloc(image, "user.verity.roothash", &text); if (r < 0) { _cleanup_free_ char *p = NULL; @@ -2794,7 +2794,7 @@ int verity_settings_load( * `usrhash`, because `usrroothash` or `rootusrhash` would just be too * confusing. We thus drop the reference to the root of the Merkle tree, and * just indicate which file system it's about. */ - r = getxattr_malloc(image, "user.verity.usrhash", &text, true); + r = getxattr_malloc(image, "user.verity.usrhash", &text); if (r < 0) { _cleanup_free_ char *p = NULL; diff --git a/src/shared/smack-util.c b/src/shared/smack-util.c index 1d04020027..d7fbbdc913 100644 --- a/src/shared/smack-util.c +++ b/src/shared/smack-util.c @@ -52,7 +52,7 @@ int mac_smack_read(const char *path, SmackAttr attr, char **label) { if (!mac_smack_use()) return 0; - return getxattr_malloc(path, smack_attr_to_string(attr), label, true); + return getxattr_malloc(path, smack_attr_to_string(attr), label); } int mac_smack_read_fd(int fd, SmackAttr attr, char **label) { @@ -166,7 +166,7 @@ static int smack_fix_fd(int fd, const char *abspath, LabelFixFlags flags) { return 0; /* If the old label is identical to the new one, suppress any kind of error */ - if (getxattr_malloc(FORMAT_PROC_FD_PATH(fd), "security.SMACK64", &old_label, false) >= 0 && + if (lgetxattr_malloc(FORMAT_PROC_FD_PATH(fd), "security.SMACK64", &old_label) >= 0 && streq(old_label, label)) return 0; diff --git a/src/test/test-copy.c b/src/test/test-copy.c index 362f210e6a..445a4b2a6e 100644 --- a/src/test/test-copy.c +++ b/src/test/test-copy.c @@ -149,7 +149,7 @@ static void test_copy_tree(void) { assert_se(read_full_file(f, &buf, &sz) == 0); assert_se(streq(buf, "file\n")); - k = getxattr_malloc(f, "user.testxattr", &c, false); + k = lgetxattr_malloc(f, "user.testxattr", &c); assert_se(xattr_worked < 0 || ((k >= 0) == !!xattr_worked)); if (k >= 0) { diff --git a/src/test/test-xattr-util.c b/src/test/test-xattr-util.c index 3fbae5738f..b30a639ff3 100644 --- a/src/test/test-xattr-util.c +++ b/src/test/test-xattr-util.c @@ -15,14 +15,12 @@ #include "tmpfile-util.h" #include "xattr-util.h" -static void test_fgetxattrat_fake(void) { +static void test_getxattr_at_malloc(void) { char t[] = "/var/tmp/xattrtestXXXXXX"; _cleanup_free_ char *value = NULL; _cleanup_close_ int fd = -1; const char *x; - char v[3]; int r; - size_t size; log_info("/* %s */", __func__); @@ -38,21 +36,24 @@ static void test_fgetxattrat_fake(void) { fd = open(t, O_RDONLY|O_DIRECTORY|O_CLOEXEC|O_NOCTTY); assert_se(fd >= 0); - assert_se(fgetxattrat_fake(fd, "test", "user.foo", v, 3, 0, &size) >= 0); - assert_se(size == 3); - assert_se(memcmp(v, "bar", 3) == 0); + assert_se(getxattr_at_malloc(fd, "test", "user.foo", 0, &value) == 3); + assert_se(memcmp(value, "bar", 3) == 0); + value = mfree(value); + + assert_se(getxattr_at_malloc(AT_FDCWD, x, "user.foo", 0, &value) == 3); + assert_se(memcmp(value, "bar", 3) == 0); + value = mfree(value); safe_close(fd); fd = open("/", O_RDONLY|O_DIRECTORY|O_CLOEXEC|O_NOCTTY); assert_se(fd >= 0); - r = fgetxattrat_fake(fd, "usr", "user.idontexist", v, 3, 0, &size); + r = getxattr_at_malloc(fd, "usr", "user.idontexist", 0, &value); assert_se(r == -ENODATA || ERRNO_IS_NOT_SUPPORTED(r)); safe_close(fd); fd = open(x, O_PATH|O_CLOEXEC); assert_se(fd >= 0); - r = fgetxattrat_fake_malloc(fd, NULL, "user.foo", AT_EMPTY_PATH, &value); - assert_se(r == 3); + assert_se(getxattr_at_malloc(fd, NULL, "user.foo", 0, &value) == 3); assert_se(streq(value, "bar")); cleanup: @@ -68,7 +69,7 @@ static void test_getcrtime(void) { log_info("/* %s */", __func__); - assert_se(tmp_dir(&vt) >= 0); + assert_se(var_tmp_dir(&vt) >= 0); fd = open_tmpfile_unlinkable(vt, O_RDWR); assert_se(fd >= 0); @@ -92,7 +93,7 @@ static void test_getcrtime(void) { int main(void) { test_setup_logging(LOG_DEBUG); - test_fgetxattrat_fake(); + test_getxattr_at_malloc(); test_getcrtime(); return 0; From a5edf95ec2e38c01abbc237ca4c906f102c224d8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 Oct 2021 17:02:08 +0200 Subject: [PATCH 2/8] cgroup-util: add reusable union type for cgroupfs file_handle structs That way we can easily call name_to_handle_at() on cgroupfs2 elsewhere. --- src/basic/cgroup-util.c | 13 +++---------- src/basic/cgroup-util.h | 10 ++++++++++ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 1d577a24ec..e60db5e48c 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -1368,25 +1368,18 @@ int cg_pid_get_machine_name(pid_t pid, char **machine) { } int cg_path_get_cgroupid(const char *path, uint64_t *ret) { + cg_file_handle fh = CG_FILE_HANDLE_INIT; int mnt_id = -1; assert(path); assert(ret); - union { - struct file_handle f_handle; - uint8_t space[offsetof(struct file_handle, f_handle) + sizeof(uint64_t)]; - } buf = { - .f_handle.handle_bytes = sizeof(uint64_t), - }; - /* This is cgroupfs so we know the size of the handle, thus no need to loop around like * name_to_handle_at_loop() does in mountpoint-util.c */ - if (name_to_handle_at(AT_FDCWD, path, &buf.f_handle, &mnt_id, 0) < 0) + if (name_to_handle_at(AT_FDCWD, path, &fh.file_handle, &mnt_id, 0) < 0) return -errno; - *ret = *(uint64_t *) buf.f_handle.f_handle; - + *ret = CG_FILE_HANDLE_CGROUPID(fh); return 0; } diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index 43801ee0f4..21275e9eaa 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -2,6 +2,7 @@ #pragma once #include +#include #include #include #include @@ -315,3 +316,12 @@ typedef enum ManagedOOMPreference { const char* managed_oom_preference_to_string(ManagedOOMPreference a) _const_; ManagedOOMPreference managed_oom_preference_from_string(const char *s) _pure_; + +/* The structure to pass to name_to_handle_at() on cgroupfs2 */ +typedef union { + struct file_handle file_handle; + uint8_t space[offsetof(struct file_handle, f_handle) + sizeof(uint64_t)]; +} cg_file_handle; + +#define CG_FILE_HANDLE_INIT { .file_handle.handle_bytes = sizeof(uint64_t) } +#define CG_FILE_HANDLE_CGROUPID(fh) (*(uint64_t*) (fh).file_handle.f_handle) From 40d4320deae38db40e0c728cd07b0d7c1bb95606 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 Oct 2021 17:04:16 +0200 Subject: [PATCH 3/8] cgroups-show: validate specified hostname before including it in fs path let's make sure the specified hostname is really valid before we build an fs path from it. Just as a safety future, so that people can't trick us with hostnames including "/../" or so. --- src/shared/cgroup-show.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/shared/cgroup-show.c b/src/shared/cgroup-show.c index bdc35492d0..4c0bb49108 100644 --- a/src/shared/cgroup-show.c +++ b/src/shared/cgroup-show.c @@ -14,6 +14,7 @@ #include "env-file.h" #include "fd-util.h" #include "format-util.h" +#include "hostname-util.h" #include "locale-util.h" #include "macro.h" #include "output-mode.h" @@ -355,14 +356,17 @@ int show_cgroup_get_path_and_warn( const char *prefix, char **ret) { - int r; _cleanup_free_ char *root = NULL; + int r; if (machine) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; _cleanup_free_ char *unit = NULL; const char *m; + if (!hostname_is_valid(machine, 0)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Machine name is not valid: %s", machine); + m = strjoina("/run/systemd/machines/", machine); r = parse_env_file(NULL, m, "SCOPE", &unit); if (r < 0) From aa13d3841728ff5212725c245616bfbf20cbeb0a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 Oct 2021 17:06:05 +0200 Subject: [PATCH 4/8] cgroups-show: use path_join() when concatenating cgroup paths --- src/shared/cgroup-show.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/cgroup-show.c b/src/shared/cgroup-show.c index 4c0bb49108..060c19c57c 100644 --- a/src/shared/cgroup-show.c +++ b/src/shared/cgroup-show.c @@ -384,14 +384,14 @@ int show_cgroup_get_path_and_warn( if (r == -ENOMEDIUM) return log_error_errno(r, "Failed to get root control group path.\n" "No cgroup filesystem mounted on /sys/fs/cgroup"); - else if (r < 0) + if (r < 0) return log_error_errno(r, "Failed to get root control group path: %m"); } if (prefix) { char *t; - t = strjoin(root, prefix); + t = path_join(root, prefix); if (!t) return log_oom(); From 5a5a5d29147f9e87f05e1b3a6124c64d2a53350e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 Oct 2021 17:06:50 +0200 Subject: [PATCH 5/8] cgroups-show: optionally show cgroup xattrs + cgroup id in cgroup tree output --- src/shared/cgroup-show.c | 101 +++++++++++++++++++++++++++++++++------ src/shared/output-mode.h | 16 +++++-- 2 files changed, 98 insertions(+), 19 deletions(-) diff --git a/src/shared/cgroup-show.c b/src/shared/cgroup-show.c index 060c19c57c..c3eaf5387f 100644 --- a/src/shared/cgroup-show.c +++ b/src/shared/cgroup-show.c @@ -12,11 +12,13 @@ #include "cgroup-show.h" #include "cgroup-util.h" #include "env-file.h" +#include "escape.h" #include "fd-util.h" #include "format-util.h" #include "hostname-util.h" #include "locale-util.h" #include "macro.h" +#include "nulstr-util.h" #include "output-mode.h" #include "parse-util.h" #include "path-util.h" @@ -122,38 +124,109 @@ static int show_cgroup_one_by_path( static int show_cgroup_name( const char *path, const char *prefix, - const char *glyph) { + SpecialGlyph glyph, + OutputFlags flags) { + uint64_t cgroupid = UINT64_MAX; _cleanup_free_ char *b = NULL; + _cleanup_close_ int fd = -1; bool delegate = false; int r; - r = lgetxattr_malloc(path, "trusted.delegate", &b); + if (FLAGS_SET(flags, OUTPUT_CGROUP_XATTRS) || FLAGS_SET(flags, OUTPUT_CGROUP_ID)) { + fd = open(path, O_PATH|O_CLOEXEC|O_NOFOLLOW|O_DIRECTORY, 0); + if (fd < 0) + log_debug_errno(errno, "Failed to open cgroup '%s', ignoring: %m", path); + } + + r = getxattr_malloc(fd < 0 ? path : FORMAT_PROC_FD_PATH(fd), "trusted.delegate", &b); if (r < 0) { if (r != -ENODATA) - log_debug_errno(r, "Failed to read trusted.delegate extended attribute: %m"); + log_debug_errno(r, "Failed to read trusted.delegate extended attribute, ignoring: %m"); } else { r = parse_boolean(b); if (r < 0) - log_debug_errno(r, "Failed to parse trusted.delegate extended attribute boolean value: %m"); + log_debug_errno(r, "Failed to parse trusted.delegate extended attribute boolean value, ignoring: %m"); else delegate = r > 0; b = mfree(b); } - b = strdup(basename(path)); - if (!b) - return -ENOMEM; + if (FLAGS_SET(flags, OUTPUT_CGROUP_ID)) { + cg_file_handle fh = CG_FILE_HANDLE_INIT; + int mnt_id = -1; - printf("%s%s%s%s%s %s%s%s\n", - prefix, glyph, + if (name_to_handle_at( + fd < 0 ? AT_FDCWD : fd, + fd < 0 ? path : "", + &fh.file_handle, + &mnt_id, + fd < 0 ? 0 : AT_EMPTY_PATH) < 0) + log_debug_errno(errno, "Failed to determine cgroup ID of %s, ignoring: %m", path); + else + cgroupid = CG_FILE_HANDLE_CGROUPID(fh); + } + + r = path_extract_filename(path, &b); + if (r < 0) + return log_error_errno(r, "Failed to extract filename from cgroup path: %m"); + + printf("%s%s%s%s%s", + prefix, special_glyph(glyph), delegate ? ansi_underline() : "", cg_unescape(b), - delegate ? ansi_normal() : "", - delegate ? ansi_highlight() : "", - delegate ? special_glyph(SPECIAL_GLYPH_ELLIPSIS) : "", delegate ? ansi_normal() : ""); + + if (delegate) + printf(" %s%s%s", + ansi_highlight(), + special_glyph(SPECIAL_GLYPH_ELLIPSIS), + ansi_normal()); + + if (cgroupid != UINT64_MAX) + printf(" %s(#%" PRIu64 ")%s", ansi_grey(), cgroupid, ansi_normal()); + + printf("\n"); + + if (FLAGS_SET(flags, OUTPUT_CGROUP_XATTRS) && fd >= 0) { + _cleanup_free_ char *nl = NULL; + char *xa; + + r = flistxattr_malloc(fd, &nl); + if (r < 0) + log_debug_errno(r, "Failed to enumerate xattrs on '%s', ignoring: %m", path); + + NULSTR_FOREACH(xa, nl) { + _cleanup_free_ char *x = NULL, *y = NULL, *buf = NULL; + int n; + + if (!STARTSWITH_SET(xa, "user.", "trusted.")) + continue; + + n = fgetxattr_malloc(fd, xa, &buf); + if (n < 0) { + log_debug_errno(r, "Failed to read xattr '%s' off '%s', ignoring: %m", xa, path); + continue; + } + + x = cescape(xa); + if (!x) + return -ENOMEM; + + y = cescape_length(buf, n); + if (!y) + return -ENOMEM; + + printf("%s%s%s %s%s%s: %s\n", + prefix, + glyph == SPECIAL_GLYPH_TREE_BRANCH ? special_glyph(SPECIAL_GLYPH_TREE_VERTICAL) : " ", + special_glyph(SPECIAL_GLYPH_ARROW), + ansi_blue(), x, ansi_normal(), + y); + } + } + return 0; } @@ -201,7 +274,7 @@ int show_cgroup_by_path( } if (last) { - r = show_cgroup_name(last, prefix, special_glyph(SPECIAL_GLYPH_TREE_BRANCH)); + r = show_cgroup_name(last, prefix, SPECIAL_GLYPH_TREE_BRANCH, flags); if (r < 0) return r; @@ -225,7 +298,7 @@ int show_cgroup_by_path( show_cgroup_one_by_path(path, prefix, n_columns, !!last, flags); if (last) { - r = show_cgroup_name(last, prefix, special_glyph(SPECIAL_GLYPH_TREE_RIGHT)); + r = show_cgroup_name(last, prefix, SPECIAL_GLYPH_TREE_RIGHT, flags); if (r < 0) return r; diff --git a/src/shared/output-mode.h b/src/shared/output-mode.h index ba067c9f9c..4b4abfe75f 100644 --- a/src/shared/output-mode.h +++ b/src/shared/output-mode.h @@ -33,14 +33,20 @@ static inline bool OUTPUT_MODE_IS_JSON(OutputMode m) { typedef enum OutputFlags { OUTPUT_SHOW_ALL = 1 << 0, - OUTPUT_WARN_CUTOFF = 1 << 1, - OUTPUT_FULL_WIDTH = 1 << 2, - OUTPUT_COLOR = 1 << 3, + OUTPUT_FULL_WIDTH = 1 << 1, + OUTPUT_COLOR = 1 << 2, + + /* Specific to log output */ + OUTPUT_WARN_CUTOFF = 1 << 3, OUTPUT_CATALOG = 1 << 4, OUTPUT_BEGIN_NEWLINE = 1 << 5, OUTPUT_UTC = 1 << 6, - OUTPUT_KERNEL_THREADS = 1 << 7, - OUTPUT_NO_HOSTNAME = 1 << 8, + OUTPUT_NO_HOSTNAME = 1 << 7, + + /* Specific to process tree output */ + OUTPUT_KERNEL_THREADS = 1 << 8, + OUTPUT_CGROUP_XATTRS = 1 << 9, + OUTPUT_CGROUP_ID = 1 << 10, } OutputFlags; JsonFormatFlags output_mode_to_json_format_flags(OutputMode m); From 87843de48bf2a5425ede1ff8aa9e16aff0214170 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 Oct 2021 17:07:43 +0200 Subject: [PATCH 6/8] cgls: enable cgroupid/xattr output by default (but make it configurable) --- src/cgls/cgls.c | 53 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/src/cgls/cgls.c b/src/cgls/cgls.c index ab5f153d9d..e2c746f7e7 100644 --- a/src/cgls/cgls.c +++ b/src/cgls/cgls.c @@ -16,6 +16,7 @@ #include "main-func.h" #include "output-mode.h" #include "pager.h" +#include "parse-util.h" #include "path-util.h" #include "pretty-print.h" #include "strv.h" @@ -23,8 +24,7 @@ #include "util.h" static PagerFlags arg_pager_flags = 0; -static bool arg_kernel_threads = false; -static bool arg_all = false; +static OutputFlags arg_output_flags = OUTPUT_CGROUP_XATTRS | OUTPUT_CGROUP_ID; static enum { SHOW_UNIT_NONE, @@ -54,6 +54,8 @@ static int help(void) { " -a --all Show all groups, including empty\n" " -u --unit Show the subtrees of specified system units\n" " --user-unit Show the subtrees of specified user units\n" + " --xattr=BOOL Show cgroup extended attributes\n" + " --cgroup-id=BOOL Show cgroup ID\n" " -l --full Do not ellipsize output\n" " -k Include kernel threads in output\n" " -M --machine= Show container\n" @@ -70,6 +72,8 @@ static int parse_argv(int argc, char *argv[]) { ARG_NO_PAGER = 0x100, ARG_VERSION, ARG_USER_UNIT, + ARG_XATTR, + ARG_CGROUP_ID, }; static const struct option options[] = { @@ -81,10 +85,12 @@ static int parse_argv(int argc, char *argv[]) { { "machine", required_argument, NULL, 'M' }, { "unit", optional_argument, NULL, 'u' }, { "user-unit", optional_argument, NULL, ARG_USER_UNIT }, + { "xattr", required_argument, NULL, ARG_XATTR }, + { "cgroup-id", required_argument, NULL, ARG_CGROUP_ID }, {} }; - int c; + int c, r; assert(argc >= 1); assert(argv); @@ -104,7 +110,7 @@ static int parse_argv(int argc, char *argv[]) { break; case 'a': - arg_all = true; + arg_output_flags |= OUTPUT_SHOW_ALL; break; case 'u': @@ -130,13 +136,29 @@ static int parse_argv(int argc, char *argv[]) { break; case 'k': - arg_kernel_threads = true; + arg_output_flags |= OUTPUT_KERNEL_THREADS; break; case 'M': arg_machine = optarg; break; + case ARG_XATTR: + r = parse_boolean(optarg); + if (r < 0) + return log_error_errno(r, "Failed to parse --xattr= value: %s", optarg); + + SET_FLAG(arg_output_flags, OUTPUT_CGROUP_XATTRS, r); + break; + + case ARG_CGROUP_ID: + r = parse_boolean(optarg); + if (r < 0) + return log_error_errno(r, "Failed to parse --cgroup-id= value: %s", optarg); + + SET_FLAG(arg_output_flags, OUTPUT_CGROUP_ID, r); + break; + case '?': return -EINVAL; @@ -161,7 +183,7 @@ static void show_cg_info(const char *controller, const char *path) { } static int run(int argc, char *argv[]) { - int r, output_flags; + int r; log_setup(); @@ -173,10 +195,8 @@ static int run(int argc, char *argv[]) { if (r > 0 && arg_full < 0) arg_full = true; - output_flags = - arg_all * OUTPUT_SHOW_ALL | - (arg_full > 0) * OUTPUT_FULL_WIDTH | - arg_kernel_threads * OUTPUT_KERNEL_THREADS; + if (arg_full > 0) + arg_output_flags |= OUTPUT_FULL_WIDTH; if (arg_names) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; @@ -204,22 +224,21 @@ static int run(int argc, char *argv[]) { goto failed; if (isempty(cgroup)) { - log_warning("Unit %s not found.", *name); - q = -ENOENT; + q = log_warning_errno(SYNTHETIC_ERRNO(ENOENT), "Unit %s not found.", *name); goto failed; } printf("Unit %s (%s):\n", *name, cgroup); fflush(stdout); - q = show_cgroup_by_path(cgroup, NULL, 0, output_flags); + q = show_cgroup_by_path(cgroup, NULL, 0, arg_output_flags); } else if (path_startswith(*name, "/sys/fs/cgroup")) { printf("Directory %s:\n", *name); fflush(stdout); - q = show_cgroup_by_path(*name, NULL, 0, output_flags); + q = show_cgroup_by_path(*name, NULL, 0, arg_output_flags); } else { _cleanup_free_ char *c = NULL, *p = NULL, *j = NULL; const char *controller, *path; @@ -250,7 +269,7 @@ static int run(int argc, char *argv[]) { show_cg_info(controller, path); - q = show_cgroup(controller, path, NULL, 0, output_flags); + q = show_cgroup(controller, path, NULL, 0, arg_output_flags); } failed: @@ -272,7 +291,7 @@ static int run(int argc, char *argv[]) { printf("Working directory %s:\n", cwd); fflush(stdout); - r = show_cgroup_by_path(cwd, NULL, 0, output_flags); + r = show_cgroup_by_path(cwd, NULL, 0, arg_output_flags); done = true; } } @@ -287,7 +306,7 @@ static int run(int argc, char *argv[]) { show_cg_info(SYSTEMD_CGROUP_CONTROLLER, root); printf("-.slice\n"); - r = show_cgroup(SYSTEMD_CGROUP_CONTROLLER, root, NULL, 0, output_flags); + r = show_cgroup(SYSTEMD_CGROUP_CONTROLLER, root, NULL, 0, arg_output_flags); } } if (r < 0) From bd3bfc88567d26cd3266905db1efc7d8fb3d8eea Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 Oct 2021 17:12:26 +0200 Subject: [PATCH 7/8] man: document the two new switches for systemd-cgls --- man/systemd-cgls.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/man/systemd-cgls.xml b/man/systemd-cgls.xml index da853ec5aa..3fa0a35770 100644 --- a/man/systemd-cgls.xml +++ b/man/systemd-cgls.xml @@ -105,6 +105,20 @@ MACHINE. + + + + Controls whether to include information about extended attributes of the listed + control groups in the output. Expects a boolean value, defaults to yes. + + + + + + Controls whether to include the numeric ID of the listed control groups in the + output. Expects a boolean value, defaults to yes. + + From d83a8ed00201a413de109451a5902043d40370a7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 7 Oct 2021 11:46:10 +0200 Subject: [PATCH 8/8] bash-completion: add the two new switches to systemd-cgls (completion hookup for zsh doesn't appear to exist, hence not adding there.) --- shell-completion/bash/systemd-cgls | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell-completion/bash/systemd-cgls b/shell-completion/bash/systemd-cgls index 0b6a8fda0d..f3a334f02d 100644 --- a/shell-completion/bash/systemd-cgls +++ b/shell-completion/bash/systemd-cgls @@ -41,7 +41,7 @@ _systemd_cgls() { local i verb comps local -A OPTS=( - [STANDALONE]='-h --help --version --all -l --full -k --no-pager' + [STANDALONE]='-h --help --version --all -l --full -k --no-pager --xattr=no --cgroup-id=no' [ARG]='-M --machine -u --unit --user-unit' )