From 05587cece54dd37a5290ac8143c28f837661769c Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 31 Mar 2025 18:20:22 +0200 Subject: [PATCH 1/6] xattr-util: add missing assertions Follow-up for ed024abac6e4acad5a77943721e7622bb1b10b71 and 9fbe26cfa8fadb58e048aa6a57ad0dacf5de03fc Also, let's not get too tangled up in the style of defining variables in between. The functions are short enough, and vars involved are still effectively at the beginning... Put differently, the separation from 'int r' is too deliberate and brings no actual value in my eyes. --- src/basic/xattr-util.c | 13 ++++++++----- src/basic/xattr-util.h | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/basic/xattr-util.c b/src/basic/xattr-util.c index f958e480ff..deacc1025a 100644 --- a/src/basic/xattr-util.c +++ b/src/basic/xattr-util.c @@ -192,10 +192,11 @@ int getxattr_at_bool(int fd, const char *path, const char *name, int at_flags) { } int getxattr_at_strv(int fd, const char *path, const char *name, int at_flags, char ***ret_strv) { + _cleanup_free_ char *nulstr = NULL; + size_t nulstr_size; int r; - _cleanup_free_ char *nulstr = NULL; - size_t nulstr_size = 0; + assert(ret_strv); r = getxattr_at_malloc(fd, path, name, at_flags, &nulstr, &nulstr_size); if (r < 0) @@ -343,11 +344,13 @@ int xsetxattr_full( return 0; } -int xsetxattr_strv(int fd, const char *path, int at_flags, const char *name, char * const* l) { +int xsetxattr_strv(int fd, const char *path, int at_flags, const char *name, char * const *l) { + _cleanup_free_ char *nulstr = NULL; + size_t size; int r; - _cleanup_free_ char *nulstr = NULL; - size_t size = 0; + assert(name); + r = strv_make_nulstr(l, &nulstr, &size); if (r < 0) return r; diff --git a/src/basic/xattr-util.h b/src/basic/xattr-util.h index 02293e5007..773e736cfb 100644 --- a/src/basic/xattr-util.h +++ b/src/basic/xattr-util.h @@ -50,7 +50,7 @@ static inline int xsetxattr( return xsetxattr_full(fd, path, at_flags, name, value, SIZE_MAX, 0); } -int xsetxattr_strv(int fd, const char *path, int at_flags, const char *name, char *const*l); +int xsetxattr_strv(int fd, const char *path, int at_flags, const char *name, char * const *l); int xremovexattr(int fd, const char *path, int at_flags, const char *name); From 7b8d6fc8361871d9593b2e083bdb6c9749438d10 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 31 Mar 2025 18:33:56 +0200 Subject: [PATCH 2/6] validatefs: insert empty line after short description in help() text As per our usual coding style. --- src/validatefs/validatefs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/validatefs/validatefs.c b/src/validatefs/validatefs.c index fb516744d3..8e7753126c 100644 --- a/src/validatefs/validatefs.c +++ b/src/validatefs/validatefs.c @@ -33,7 +33,7 @@ static int help(void) { return log_oom(); printf("%1$s [OPTIONS...] /path/to/mountpoint\n" - "\n%3$sCheck file system validation constraints.%4$s\n" + "\n%3$sCheck file system validation constraints.%4$s\n\n" " -h --help Show this help and exit\n" " --version Print version string and exit\n" " --root=PATH|auto Operate relative to the specified path\n" From 8950f7651e6165e92cafb2f0b33752db4e7afd0c Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 31 Mar 2025 18:43:34 +0200 Subject: [PATCH 3/6] validatefs: do not use EXIT_* in run() The plain DEFINE_MAIN_FUNCTION is used, i.e. all >= 0 retvals are treated as success. No need for EXIT_* annotations. --- src/validatefs/validatefs.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/validatefs/validatefs.c b/src/validatefs/validatefs.c index 8e7753126c..22345e5303 100644 --- a/src/validatefs/validatefs.c +++ b/src/validatefs/validatefs.c @@ -318,14 +318,10 @@ static int run(int argc, char *argv[]) { return r; if (r == 0) { log_info("File system '%s' has no validation constraints set, not validating.", resolved); - return EXIT_SUCCESS; + return 0; } - r = validate_fields_check(target_fd, resolved, &f); - if (r < 0) - return r; - - return EXIT_SUCCESS; + return validate_fields_check(target_fd, resolved, &f); } DEFINE_MAIN_FUNCTION(run); From f361fad245161175af79a0fd8be7c1b5fa3f2587 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 31 Mar 2025 19:20:54 +0200 Subject: [PATCH 4/6] validatefs: use fgetxattr_malloc() where appropriate --- src/validatefs/validatefs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/validatefs/validatefs.c b/src/validatefs/validatefs.c index 22345e5303..2499a7e20f 100644 --- a/src/validatefs/validatefs.c +++ b/src/validatefs/validatefs.c @@ -136,7 +136,7 @@ static int validate_fields_read(int fd, ValidateFields *ret) { assert(ret); _cleanup_free_ char *t = NULL; - r = getxattr_at_malloc(fd, /* path= */ NULL, "user.validatefs.gpt_type_uuid", AT_EMPTY_PATH, &t, /* ret_size= */ NULL); + r = fgetxattr_malloc(fd, "user.validatefs.gpt_type_uuid", &t, /* ret_size= */ NULL); if (r < 0) { if (r != -ENODATA && !ERRNO_IS_NOT_SUPPORTED(r)) return log_error_errno(r, "Failed to read 'user.validatefs.gpt_type_uuid' xattr: %m"); @@ -146,7 +146,7 @@ static int validate_fields_read(int fd, ValidateFields *ret) { return log_error_errno(r, "Failed to parse 'user.validatefs.gpt_type_uuid' xattr: %s", t); } - r = getxattr_at_malloc(fd, /* path= */ NULL, "user.validatefs.gpt_label", AT_EMPTY_PATH, &f.gpt_label, /* ret_size= */ NULL); + r = fgetxattr_malloc(fd, "user.validatefs.gpt_label", &f.gpt_label, /* ret_size= */ NULL); if (r < 0) { if (r != -ENODATA && !ERRNO_IS_NOT_SUPPORTED(r)) return log_error_errno(r, "Failed to read 'user.validatefs.gpt_label' xattr: %m"); From 28bae3dbe3a5185a7d2303a0e5af3c3ce4394eb4 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 31 Mar 2025 19:04:49 +0200 Subject: [PATCH 5/6] man/systemd-validatefs@.service: fix typo --- man/systemd-validatefs@.service.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/systemd-validatefs@.service.xml b/man/systemd-validatefs@.service.xml index 397bb0d669..f0c7aabc6e 100644 --- a/man/systemd-validatefs@.service.xml +++ b/man/systemd-validatefs@.service.xml @@ -84,7 +84,7 @@ Takes an absolute path or the special string auto. The specified - path if removed as prefix from the specified mount point argument before the validation. If set to + path is removed as prefix from the specified mount point argument before the validation. If set to auto defaults to unspecified on the host and /sysroot/ when run in initrd context, in order to validate the mount constraint data relative to the future file system root. From 0a7d86d53d175b2ad7473b959d978e60543bbab1 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 31 Mar 2025 18:39:42 +0200 Subject: [PATCH 6/6] units/systemd-validatefs@.service: FailureAction= is a [Unit] knob --- units/systemd-validatefs@.service.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/units/systemd-validatefs@.service.in b/units/systemd-validatefs@.service.in index f94482f982..d0a0f5f9b0 100644 --- a/units/systemd-validatefs@.service.in +++ b/units/systemd-validatefs@.service.in @@ -15,9 +15,9 @@ BindsTo=%i.mount Conflicts=shutdown.target After=%i.mount Before=shutdown.target systemd-pcrfs@%i.service systemd-quotacheck@%i.service systemd-growfs@%i.service +FailureAction=reboot-immediate [Service] Type=oneshot RemainAfterExit=yes ExecStart={{LIBEXECDIR}}/systemd-validatefs --root=auto %f -FailureAction=reboot-immediate