From f41defc034f9ce1fbb498fce136841e26940dd4c Mon Sep 17 00:00:00 2001 From: Armaan Sandhu Date: Wed, 23 Sep 2026 18:39:16 -0400 Subject: [PATCH] report: make report-files listing and lookup agree on masked entries The metric list is built with conf_files_list_strv(), but each metric's value is then looked up again by walking report_files_dirs[] by hand, and the two disagreed whenever an earlier directory had an entry that is not a regular file: - conf_files_list_strv() skipped such an entry and listed the one from a later directory, so the metric was advertised by Describe(); - file_metric_generate() only moved on to the next directory on -ENOENT, so it hit the non-regular entry, logged a warning and produced nothing. The practical case is masking a vendor-supplied entry with a symlink to /dev/null: the metric kept being described, was never listed, and every scrape logged "Failed to open ..., skipping: File descriptor in bad state". Honour masks in the enumeration via CONF_FILES_FILTER_MASKED_BY_SYMLINK, as other conf dirs do, and make the lookup skip non-regular entries the same way the enumeration does. Document the masking in the man page and cover both cases in TEST-74-AUX-UTILS.report. (cherry picked from commit 08be8677539642f45c6127085283cf08b6a410b7) --- man/systemd-report-files@.service.xml | 4 ++- src/report/report-files.c | 41 ++++++++++++++++++++++---- test/units/TEST-74-AUX-UTILS.report.sh | 23 +++++++++++++++ 3 files changed, 62 insertions(+), 6 deletions(-) diff --git a/man/systemd-report-files@.service.xml b/man/systemd-report-files@.service.xml index 9686e46cc1..a44b189fb6 100644 --- a/man/systemd-report-files@.service.xml +++ b/man/systemd-report-files@.service.xml @@ -45,7 +45,9 @@ /usr/local/lib/systemd/report.files/ and /usr/lib/systemd/report.files/. Typically these directories contain symlinks pointing to the actual files in the file system whose contents shall be reported. Entries with the same name in an - earlier directory override those in a later one. + earlier directory override those in a later one. An entry that is a symlink to + /dev/null masks the identically named entries in later directories, so that no + metric is reported for that name. Entries that are not regular files are ignored. Each entry is reported as a metric named io.systemd.Files.NAME, where diff --git a/src/report/report-files.c b/src/report/report-files.c index af2c38b5fe..1bf5bee81a 100644 --- a/src/report/report-files.c +++ b/src/report/report-files.c @@ -12,6 +12,7 @@ #include "metrics.h" #include "path-util.h" #include "report-files.h" +#include "stat-util.h" #include "string-util.h" #include "strv.h" #include "utf8.h" @@ -68,15 +69,40 @@ static int file_metric_generate(const MetricFamily *mf, sd_varlink *link, void * if (!path) return log_oom(); + /* Mirror what conf_files_list_strv() does when building the metric list: skip entries that + * cannot be resolved or are not regular files, and stop at a mask, which hides the entries in + * all later directories. The mask check is repeated here since the directories may have + * changed since the list was built. */ _cleanup_free_ char *resolved = NULL; - _cleanup_close_ int fd = chase_and_open(path, /* root= */ NULL, CHASE_MUST_BE_REGULAR, O_RDONLY|O_CLOEXEC, &resolved); - if (fd == -ENOENT) /* Not in this directory (or dangling symlink): try the next one. */ + _cleanup_close_ int pfd = -EBADF; + r = chase(path, /* root= */ NULL, /* flags= */ 0, &resolved, &pfd); + if (r == -ENOENT) continue; - if (fd < 0) { - log_warning_errno(fd, "Failed to open '%s', skipping: %m", path); + if (r < 0) { + log_warning_errno(r, "Failed to resolve '%s', ignoring: %m", path); + continue; + } + + struct stat st; + if (fstat(pfd, &st) < 0) { + log_warning_errno(errno, "Failed to stat '%s', ignoring: %m", path); + continue; + } + + if (stat_may_be_dev_null(&st)) { + log_debug("File for metric '%s' is masked, skipping.", mf->name); return 0; } + if (!S_ISREG(st.st_mode)) + continue; + + _cleanup_close_ int fd = fd_reopen(pfd, O_RDONLY|O_CLOEXEC); + if (fd < 0) { + log_warning_errno(fd, "Failed to open '%s', ignoring: %m", path); + continue; + } + r = read_full_file_full( fd, /* filename= */ NULL, @@ -120,7 +146,12 @@ static int build_file_metrics(MetricFamily **ret) { /* Enumerate the files to report across all our directories, deduplicated by name. The entry name is * used as the metric field name. */ - r = conf_files_list_strv(&files, /* suffix= */ NULL, /* root= */ NULL, CONF_FILES_REGULAR, report_files_dirs); + r = conf_files_list_strv( + &files, + /* suffix= */ NULL, + /* root= */ NULL, + CONF_FILES_REGULAR|CONF_FILES_FILTER_MASKED_BY_SYMLINK, + report_files_dirs); if (r < 0) return log_error_errno(r, "Failed to enumerate report files: %m"); diff --git a/test/units/TEST-74-AUX-UTILS.report.sh b/test/units/TEST-74-AUX-UTILS.report.sh index 5cf8cb7ac8..1da6dea557 100755 --- a/test/units/TEST-74-AUX-UTILS.report.sh +++ b/test/units/TEST-74-AUX-UTILS.report.sh @@ -249,6 +249,9 @@ at_exit() { systemctl stop fake-report-server fake-report-server-tls systemctl stop systemd-report.socket systemd-report-files.socket systemd-report-sign-plain.socket rm -f /run/systemd/report.files/testreportfile /run/systemd/report.files/binaryfile + rm -f /run/systemd/report.files/maskedfile /var/lib/systemd/report.files/maskedfile /var/lib/systemd/report.files/dirshadowed + rm -f /run/systemd/report.files/fifoshadowed /var/lib/systemd/report.files/fifoshadowed + rmdir /run/systemd/report.files/dirshadowed 2>/dev/null rm -rf "$CERTDIR" "${SIGN_WORK:-}" } trap at_exit EXIT @@ -301,6 +304,26 @@ files_metrics="$(varlinkctl call --more /run/systemd/report/io.systemd.Files io. [ -z "$(files_value io.systemd.Files.binaryfile)" ] rm -f /run/systemd/report.files/binaryfile +# An entry in an earlier directory that is a symlink to /dev/null masks the one +# in a later directory: it must be neither listed nor described. An entry that +# isn't a regular file is ignored, so the later one is reported instead. +mkdir -p /var/lib/systemd/report.files +echo masked >/var/lib/systemd/report.files/maskedfile +echo shadowed >/var/lib/systemd/report.files/dirshadowed +ln -s /dev/null /run/systemd/report.files/maskedfile +mkdir /run/systemd/report.files/dirshadowed +echo fifo-shadowed >/var/lib/systemd/report.files/fifoshadowed +mkfifo /run/systemd/report.files/fifoshadowed +files_metrics="$(varlinkctl call --more /run/systemd/report/io.systemd.Files io.systemd.Metrics.List {})" +files_describe="$(varlinkctl call --more /run/systemd/report/io.systemd.Files io.systemd.Metrics.Describe {})" +[ -z "$(files_value io.systemd.Files.maskedfile)" ] +(! echo "$files_describe" | jq --seq -r .name | grep -wx io.systemd.Files.maskedfile >/dev/null) +[ "$(files_value io.systemd.Files.dirshadowed)" = "shadowed" ] +[ "$(files_value io.systemd.Files.fifoshadowed)" = "fifo-shadowed" ] +rm -f /run/systemd/report.files/fifoshadowed /var/lib/systemd/report.files/fifoshadowed +rm -f /run/systemd/report.files/maskedfile /var/lib/systemd/report.files/maskedfile /var/lib/systemd/report.files/dirshadowed +rmdir /run/systemd/report.files/dirshadowed + # ----------------------------------------------------------------------------- # Test report signing through the plain software backend, driven entirely via # the io.systemd.Report Varlink interface (the GenerateSigned method). The plain