mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] selftests/cgroup: fix truncated reads of /proc/self/mounts
@ 2026-09-21 13:15 Shaojie Sun
  2026-09-21 13:15 ` [PATCH 1/2] selftests/cgroup: read the entire file in read_text() Shaojie Sun
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Shaojie Sun @ 2026-09-21 13:15 UTC (permalink / raw)
  To: Tejun Heo, cgroups
  Cc: Johannes Weiner, mkoutny, Shuah Khan, linux-kselftest,
	linux-kernel, Li Wang, Hongfu Li, Shakeel Butt

The cgroup selftests read /proc/self/mounts with read_text(), which issued
a single read(2) and treated the result as the whole file.  For a seq_file
larger than one page it never is: seq_read_iter() copies out at most one
internal PAGE_SIZE buffer per call and leaves the rest for the next
read(2), no matter how much room the caller's buffer has.

On a machine with ~80 mounts /proc/self/mounts is 7177 bytes, a single
read(2) into the 40K buffer in cg_find_root() returns 4035 bytes, and the
cgroup2 entry starts at offset 4953.  cg_find_root() therefore never sees
the unified hierarchy, and all ten test binaries that call
cg_find_unified_root() exit with SKIP "cgroup v2 isn't mounted" without
running a single test.  The same truncation hits proc_mount_contains(),
so a mount option listed after the first 4K is reported as absent.

Behind that there is a second failure mode: if the buffer really is too
small and the cut lands inside an entry, strtok() returns NULL for the
missing fields and cg_find_root() dereferences it, so a short read ends in
a segfault rather than a plain "no cgroup2 entry found".

The series fixes both: read_text() reads until the buffer is full or EOF,
and cg_find_root() stops parsing as soon as a field is missing.  The write
path is untouched and files that fit in max_len - 1 bytes still return
identical bytes, so callers that read a prefix on purpose, such as
cg_read_strcmp() and the 1-byte read in test_zswap, behave as before.

Tested with the mount table above: test_kill goes from
"1..0 # SKIP cgroup v2 isn't mounted" to "1..4"; with BUF_SIZE forced to
6, the old code segfaults in cg_find_unified_root() where the patched one
returns -1.

Shaojie Sun (2):
  selftests/cgroup: read the entire file in read_text()
  selftests/cgroup: don't crash on a truncated mount entry in
    cg_find_root()

 .../selftests/cgroup/lib/cgroup_util.c        | 45 ++++++++++++++++---
 1 file changed, 39 insertions(+), 6 deletions(-)

-- 
2.50.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] selftests/cgroup: read the entire file in read_text()
  2026-09-21 13:15 [PATCH 0/2] selftests/cgroup: fix truncated reads of /proc/self/mounts Shaojie Sun
@ 2026-09-21 13:15 ` Shaojie Sun
  2026-09-21 13:15 ` [PATCH 2/2] selftests/cgroup: don't crash on a truncated mount entry in cg_find_root() Shaojie Sun
  2026-09-21 17:23 ` [PATCH 0/2] selftests/cgroup: fix truncated reads of /proc/self/mounts Tejun Heo
  2 siblings, 0 replies; 4+ messages in thread
From: Shaojie Sun @ 2026-09-21 13:15 UTC (permalink / raw)
  To: Tejun Heo, cgroups
  Cc: Johannes Weiner, mkoutny, Shuah Khan, linux-kselftest,
	linux-kernel, Li Wang, Hongfu Li, Shakeel Butt

read_text() issues a single read(2) and returns whatever it got, so a
caller that asks for a whole file silently gets a prefix of it instead.
For a seq_file larger than one page it never gets more than that prefix:
seq_read_iter() copies out at most one internal PAGE_SIZE buffer per call
and leaves the rest for the next read(2), no matter how much room the
caller's buffer has.

/proc/self/mounts is one of those files, so cg_find_root() only ever sees
the first 4K of the mount table.  On a machine with ~80 mounts the cgroup2
entry already sits past that limit: the file is 7177 bytes, the single
read(2) into the 40K buffer in cg_find_root() returns 4035 bytes, and the
cgroup2 line starts at offset 4953.  cg_find_root() then fails, and every
test that calls cg_find_unified_root() -- test_core, test_cpu, test_cpuset,
test_freezer, test_hugetlb_memcg, test_kill, test_kmem, test_memcontrol,
test_pids and test_zswap -- exits with SKIP "cgroup v2 isn't mounted"
without running a single test.

proc_mount_contains() searches only that same prefix, so a mount option
listed later in /proc/mounts is reported as absent, which is what the
probes in test_memcontrol and test_hugetlb_memcg are based on.

Read until the buffer is full or EOF instead.  A partial read is still
possible when max_len is too small, so callers that deliberately read a
prefix, such as cg_read_strcmp(), keep working.

Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
---
 .../selftests/cgroup/lib/cgroup_util.c        | 35 +++++++++++++++----
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/cgroup/lib/cgroup_util.c b/tools/testing/selftests/cgroup/lib/cgroup_util.c
index 2596c12cd864..65cd85c467bb 100644
--- a/tools/testing/selftests/cgroup/lib/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/lib/cgroup_util.c
@@ -24,20 +24,43 @@ bool cg_test_v1_named;
 /* Returns read len on success, or -errno on failure. */
 ssize_t read_text(const char *path, char *buf, size_t max_len)
 {
-	ssize_t len;
+	size_t total = 0;
+	ssize_t len, ret;
 	int fd;
 
 	fd = open(path, O_RDONLY);
 	if (fd < 0)
 		return -errno;
 
-	len = read(fd, buf, max_len - 1);
-
-	if (len >= 0)
-		buf[len] = 0;
+	/*
+	 * A single read() is not enough.  procfs and sysfs are backed by
+	 * seq_file, and seq_read_iter() copies out at most one internal
+	 * buffer (PAGE_SIZE) per call, leaving the rest for the next read().
+	 * Reading only once therefore silently drops everything past the
+	 * first page, no matter how big the caller's buffer is.
+	 *
+	 * Loop until the buffer is full or EOF.  A full buffer still means
+	 * the file may be longer than max_len, but that is now limited by
+	 * the caller's buffer rather than by a page of seq_file output.
+	 */
+	while (total < max_len - 1) {
+		len = read(fd, buf + total, max_len - 1 - total);
+		if (len < 0) {
+			if (errno == EINTR)
+				continue;
+			ret = -errno;
+			goto out;
+		}
+		if (!len)
+			break;
+		total += len;
+	}
 
+	buf[total] = 0;
+	ret = total;
+out:
 	close(fd);
-	return len < 0 ? -errno : len;
+	return ret;
 }
 
 /* Returns written len on success, or -errno on failure. */
-- 
2.50.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] selftests/cgroup: don't crash on a truncated mount entry in cg_find_root()
  2026-09-21 13:15 [PATCH 0/2] selftests/cgroup: fix truncated reads of /proc/self/mounts Shaojie Sun
  2026-09-21 13:15 ` [PATCH 1/2] selftests/cgroup: read the entire file in read_text() Shaojie Sun
@ 2026-09-21 13:15 ` Shaojie Sun
  2026-09-21 17:23 ` [PATCH 0/2] selftests/cgroup: fix truncated reads of /proc/self/mounts Tejun Heo
  2 siblings, 0 replies; 4+ messages in thread
From: Shaojie Sun @ 2026-09-21 13:15 UTC (permalink / raw)
  To: Tejun Heo, cgroups
  Cc: Johannes Weiner, mkoutny, Shuah Khan, linux-kselftest,
	linux-kernel, Li Wang, Hongfu Li, Shakeel Butt

cg_find_root() parses /proc/self/mounts with strtok() and assumes that
every field group it walks over is a complete "device mountpoint type
options freq passno" tuple.  That only holds while the buffer is big
enough for the whole file: if the buffer is too small, the last tuple is
cut short, strtok() returns NULL for the missing fields, and the
strcmp(type, ...) and strstr(options, ...) calls that follow dereference
it.

The current buffer is 10 * BUF_SIZE, so reaching this needs a mount table
over 40K, but the crash is easy to hit in a sandbox: with BUF_SIZE
overridden to 6, which leaves a 60-byte buffer, cg_find_unified_root()
segfaults instead of failing.

Stop parsing as soon as a field is missing.  Nothing follows a truncated
entry, so there is nothing to parse after it either.

Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
---
 tools/testing/selftests/cgroup/lib/cgroup_util.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/testing/selftests/cgroup/lib/cgroup_util.c b/tools/testing/selftests/cgroup/lib/cgroup_util.c
index 65cd85c467bb..cd73471e13d7 100644
--- a/tools/testing/selftests/cgroup/lib/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/lib/cgroup_util.c
@@ -302,6 +302,16 @@ static int cg_find_root(char *root, size_t len, const char *controller,
 		options = strtok(NULL, delim);
 		strtok(NULL, delim);
 		strtok(NULL, delim);
+
+		/*
+		 * A mount entry is "device mountpoint type options freq
+		 * passno".  A field can only be missing if the last entry was
+		 * cut short by the buffer being too small for the file, and
+		 * there is no complete entry left to look at.
+		 */
+		if (!mount || !type || !options)
+			break;
+
 		if (strcmp(type, "cgroup") == 0) {
 			if (!controller || !strstr(options, controller))
 				continue;
-- 
2.50.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 0/2] selftests/cgroup: fix truncated reads of /proc/self/mounts
  2026-09-21 13:15 [PATCH 0/2] selftests/cgroup: fix truncated reads of /proc/self/mounts Shaojie Sun
  2026-09-21 13:15 ` [PATCH 1/2] selftests/cgroup: read the entire file in read_text() Shaojie Sun
  2026-09-21 13:15 ` [PATCH 2/2] selftests/cgroup: don't crash on a truncated mount entry in cg_find_root() Shaojie Sun
@ 2026-09-21 17:23 ` Tejun Heo
  2 siblings, 0 replies; 4+ messages in thread
From: Tejun Heo @ 2026-09-21 17:23 UTC (permalink / raw)
  To: Shaojie Sun
  Cc: cgroups, hannes, mkoutny, shuah, linux-kselftest, linux-kernel,
	li.wang, lihongfu, shakeel.butt, tj

On Mon, Sep 21, 2026 at 09:15:54PM +0800, Shaojie Sun wrote:
> Shaojie Sun (2):
>   selftests/cgroup: read the entire file in read_text()
>   selftests/cgroup: don't crash on a truncated mount entry in
>     cg_find_root()

Applied 1-2 to cgroup/for-7.4.

Thanks.

--
tejun

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-21 17:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-21 13:15 [PATCH 0/2] selftests/cgroup: fix truncated reads of /proc/self/mounts Shaojie Sun
2026-09-21 13:15 ` [PATCH 1/2] selftests/cgroup: read the entire file in read_text() Shaojie Sun
2026-09-21 13:15 ` [PATCH 2/2] selftests/cgroup: don't crash on a truncated mount entry in cg_find_root() Shaojie Sun
2026-09-21 17:23 ` [PATCH 0/2] selftests/cgroup: fix truncated reads of /proc/self/mounts Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®