errno-util: include -ENOENT in ERRNO_IS_XATTR_ABSENT()

The getxattr(2) man page only enumerates xattr-specific errors (ENODATA,
ENOTSUP, ERANGE, E2BIG, ...) in its own ERRORS section, but at the
bottom of that section notes that "the errors documented in stat(2) can
also be returned." stat(2) returns -ENOENT when a component of the path
does not exist, so any xattr lookup against a path can fail with -ENOENT
exactly the same way as -ENODATA — both mean "there is nothing here for
me to read." The previous definition of ERRNO_IS_NEG_XATTR_ABSENT()
reflected only the directly-enumerated errors and missed -ENOENT, so
callers that should semantically swallow "the xattr is absent" instead
bubbled -ENOENT up as a hard error.

The most visible consequence on real fleets has been systemd-journald
spamming dmesg with one line per dispatched log message whenever a
unit's cgroup directory cannot be found at the time
client_context_read_log_filter_patterns() is called — typically inside
containers whose journald observes clients whose unit cgroup is no
longer present in its view (cgroup-namespace boundary, unit teardown
race, transient sub-scope already collapsed back to its unit cgroup,
etc.). The same bug pattern lurks at every other cgroup-xattr callsite:
systemd-oomd reading user.oomd_avoid / user.oomd_omit / user.oomd_ooms
on cgroups it is concurrently killing; killall reading
user.survive_final_kill_signal during shutdown; cg_is_delegated() /
cg_has_coredump_receive() / cgroup_get_managed_oom_kill_last(); etc. For
these, "path is gone" is by construction the same answer as "xattr is
not set" — there is no way for the user to have attached an xattr to a
path that does not exist.

A quick survey of non-cgroup callers (src/portable/portable.c,
src/home/{homework-luks,user-record-util}.c,
src/random-seed/random-seed-tool.c, src/basic/os-util.c) confirms they
all operate on fds or on paths whose absence is already the desired
silent-skip outcome, so widening the macro to also fold in -ENOENT does
not change observable behavior at any other site.

Extend test-xattr-util's getxattr_at_malloc test with an explicit
non-existent-path lookup that asserts ERRNO_IS_NEG_XATTR_ABSENT() now
matches, alongside the pre-existing non-existent-xattr (-ENODATA) check.

diff --git a/src/basic/errno-util.h b/src/basic/errno-util.h
index eb2941253dd73e04bfedb83c8cffc359731116ac..195d13208b4605613540a0f40ddd698c049f22e3 100644 (file)
--- a/src/basic/errno-util.h
+++ b/src/basic/errno-util.h
@@ -218,10 +218,13 @@ static inline bool ERRNO_IS_NEG_DEVICE_ABSENT_OR_EMPTY(intmax_t r) {
 }
 _DEFINE_ABS_WRAPPER(DEVICE_ABSENT_OR_EMPTY);
 
-/* Quite often we want to handle cases where the backing FS doesn't support extended attributes at all and
- * where it simply doesn't have the requested xattr the same way */
+/* Quite often we want to handle cases where the backing FS doesn't support extended attributes at all,
+ * where the path component carrying the xattr is missing, and where it simply doesn't have the requested
+ * xattr the same way. Note that getxattr(2) does not enumerate -ENOENT in its own error list, but inherits
+ * it via stat(2) (see ERRORS in getxattr(2)) for the path-component-missing case. */
 static inline bool ERRNO_IS_NEG_XATTR_ABSENT(intmax_t r) {
         return r == -ENODATA ||
+                r == -ENOENT ||
                 ERRNO_IS_NEG_NOT_SUPPORTED(r);
 }
 _DEFINE_ABS_WRAPPER(XATTR_ABSENT);

diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c
index 23f804bd659436543b0137f096831554ef75927a..c5bb52b2afe27a94342238055341870b8135a291 100644 (file)
--- a/src/shared/dissect-image.c
+++ b/src/shared/dissect-image.c
@@ -3820,7 +3820,7 @@ int verity_settings_load(
                                 if (r < 0) {
                                         _cleanup_free_ char *p = NULL;
 
-                                        if (r != -ENOENT && !ERRNO_IS_XATTR_ABSENT(r))
+                                        if (!ERRNO_IS_XATTR_ABSENT(r))
                                                 return r;
 
                                         p = build_auxiliary_path(image, ".roothash");
@@ -3849,7 +3849,7 @@ int verity_settings_load(
                                 if (r < 0) {
                                         _cleanup_free_ char *p = NULL;
 
-                                        if (r != -ENOENT && !ERRNO_IS_XATTR_ABSENT(r))
+                                        if (!ERRNO_IS_XATTR_ABSENT(r))
                                                 return r;
 
                                         p = build_auxiliary_path(image, ".usrhash");

diff --git a/src/test/test-xattr-util.c b/src/test/test-xattr-util.c
index eeb0d8a8afc807db2322b7c656145c1a5a4845a6..4d39a0a1f10a6ad6bd05bf628fd375b87498e66c 100644 (file)
--- a/src/test/test-xattr-util.c
+++ b/src/test/test-xattr-util.c
@@ -46,6 +46,16 @@ TEST(getxattr_at_malloc) {
         r = getxattr_at_malloc(fd, "usr", "user.idontexist", 0, &value, /* ret_size= */ NULL);
         ASSERT_TRUE(ERRNO_IS_NEG_XATTR_ABSENT(r));
 
+        /* A non-existent path component must also be treated as xattr-absent. The kernel
+         * returns -ENOENT in this case, which is inherited from stat(2)'s error list
+         * (see ERRORS in getxattr(2)). Without this, callers that semantically just want
+         * to know "is the xattr there?" end up logging spurious errors for paths that
+         * are simply gone — e.g. journald reading a unit's cgroup xattr after the
+         * cgroup directory has been torn down. */
+        r = getxattr_at_malloc(fd, "this-path-does-not-exist-XXXXXX", "user.idontexist",
+                               0, &value, /* ret_size= */ NULL);
+        ASSERT_TRUE(ERRNO_IS_NEG_XATTR_ABSENT(r));
+
         safe_close(fd);
         ASSERT_OK_ERRNO(fd = open(x, O_PATH|O_CLOEXEC));
         ASSERT_OK(getxattr_at_malloc(fd, NULL, "user.foo", 0, &value, /* ret_size= */ NULL));