From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 13 Mar 2025 08:30:23 +0000 (+0100)
Subject: coredump,analyze: use read_full_file() for reading various top-level /proc/ files
X-Git-Tag: v258-rc1~1085
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=da65941c3ee03495541c3bffbccc9012c8d9a5f8;p=thirdparty%2Fsystemd.git

coredump,analyze: use read_full_file() for reading various top-level /proc/ files

Kernel API file systems typically use either "raw" or "seq_file" to
implement their various interface files. The former are really simple
(to point I'd call them broken), in that they have no understanding of
file offsets, and return their contents again and again on every read(),
and thus EOF is indicated by a short read, not by a zero read. The
latter otoh works like a typical file: you read until you get a
zero-sized read back.

We have read_virtual_file() to read the "raw" files, and can use regular
read_full_file() to read the "seq_file" ones.

Apparently all files in the top-level /proc/ directory use 'seq_file'.
but we accidentally used read_virtual_file() for them. Fix that.

Also clarify in a comment what the rules are.

Fixes: #36131
---

diff --git a/src/analyze/analyze-filesystems.c b/src/analyze/analyze-filesystems.c
index 582e04eac91..b09cd9fa0ee 100644
--- a/src/analyze/analyze-filesystems.c
+++ b/src/analyze/analyze-filesystems.c
@@ -17,7 +17,7 @@ static int load_available_kernel_filesystems(Set **ret) {
 
         /* Let's read the available filesystems */
 
-        r = read_virtual_file("/proc/filesystems", SIZE_MAX, &t, NULL);
+        r = read_full_file("/proc/filesystems", &t, /* ret_size= */ NULL);
         if (r < 0)
                 return r;
 
diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index 25b4c352355..b3fdc05c127 100644
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -491,7 +491,11 @@ int read_virtual_file_at(
          * max_size specifies a limit on the bytes read. If max_size is SIZE_MAX, the full file is read. If
          * the full file is too large to read, an error is returned. For other values of max_size, *partial
          * contents* may be returned. (Though the read is still done using one syscall.) Returns 0 on
-         * partial success, 1 if untruncated contents were read. */
+         * partial success, 1 if untruncated contents were read.
+         *
+         * Rule: for kernfs files using "seq_file" → use regular read_full_file_at()
+         *       for kernfs files using "raw" → use read_virtual_file_at()
+         */
 
         assert(dir_fd >= 0 || dir_fd == AT_FDCWD);
         assert(max_size <= READ_VIRTUAL_BYTES_MAX || max_size == SIZE_MAX);
diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c
index 93d2d0e2026..7f8e32f6795 100644
--- a/src/basic/proc-cmdline.c
+++ b/src/basic/proc-cmdline.c
@@ -122,7 +122,7 @@ int proc_cmdline(char **ret) {
         if (detect_container() > 0)
                 return pid_get_cmdline(1, SIZE_MAX, 0, ret);
 
-        return read_virtual_file("/proc/cmdline", SIZE_MAX, ret, NULL);
+        return read_full_file("/proc/cmdline", ret, /* ret_size= */  NULL);
 }
 
 static int proc_cmdline_strv_internal(char ***ret, bool filter_pid1_args) {
diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c
index 21fb48a9ac7..03690a0837b 100644
--- a/src/coredump/coredump.c
+++ b/src/coredump/coredump.c
@@ -1399,28 +1399,28 @@ static int gather_pid_metadata_from_procfs(struct iovec_wrapper *iovw, Context *
                 (void) iovw_put_string_field_free(iovw, "COREDUMP_OPEN_FDS=", t);
 
         p = procfs_file_alloca(pid, "status");
-        if (read_full_virtual_file(p, &t, NULL) >= 0)
+        if (read_full_file(p, &t, /* ret_size= */ NULL) >= 0)
                 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_STATUS=", t);
 
         p = procfs_file_alloca(pid, "maps");
-        if (read_full_virtual_file(p, &t, NULL) >= 0)
+        if (read_full_file(p, &t, /* ret_size= */ NULL) >= 0)
                 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_MAPS=", t);
 
-        p = procfs_file_alloca(pid, "limits");
-        if (read_full_virtual_file(p, &t, NULL) >= 0)
+        p = procfs_file_alloca(pid, "limits"); /* this uses 'seq_file' in kernel, use read_full_file_at() */
+        if (read_full_file(p, &t, /* ret_size= */ NULL) >= 0)
                 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_LIMITS=", t);
 
         p = procfs_file_alloca(pid, "cgroup");
-        if (read_full_virtual_file(p, &t, NULL) >= 0)
+        if (read_full_file(p, &t, /* ret_size= */ NULL) >= 0)
                 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_CGROUP=", t);
 
         p = procfs_file_alloca(pid, "mountinfo");
-        if (read_full_virtual_file(p, &t, NULL) >= 0)
+        if (read_full_file(p, &t, /* ret_size= */ NULL) >= 0)
                 (void) iovw_put_string_field_free(iovw, "COREDUMP_PROC_MOUNTINFO=", t);
 
         /* We attach /proc/auxv here. ELF coredumps also contain a note for this (NT_AUXV), see elf(5). */
         p = procfs_file_alloca(pid, "auxv");
-        if (read_full_virtual_file(p, &t, &size) >= 0) {
+        if (read_full_file(p, &t, &size) >= 0) {
                 char *buf = malloc(strlen("COREDUMP_PROC_AUXV=") + size + 1);
                 if (buf) {
                         /* Add a dummy terminator to make context_parse_iovw() happy. */