[PATCH selftests/timers] selftests/timers: clocksource-switch: fix unchecked open()/read()

Qingshuang Fu posted 1 patch 1 month ago
.../selftests/timers/clocksource-switch.c     | 23 ++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
[PATCH selftests/timers] selftests/timers: clocksource-switch: fix unchecked open()/read()
Posted by Qingshuang Fu 1 month ago
From: Qingshuang Fu <fuqingshuang@kylinos.cn>

get_clocksources() and get_cur_clocksource() open and read the
clocksource sysfs files without checking the return values and without
NUL-terminating the buffer.

If open() fails, read() is called on fd -1 and returns -1, which is
stored in a size_t as SIZE_MAX.  The token-scanning loop in
get_clocksources() then walks past the uninitialized stack buffer,
reading and writing out of bounds.  Even on success the buffer is not
NUL-terminated, so a fully read buffer makes the inner scan run past the
data, and get_cur_clocksource() lets change_clocksource() call strlen()
on a non-terminated buffer.

Check the open()/read() results, use ssize_t for the read length,
NUL-terminate the buffer, and close the fd in get_cur_clocksource().

Fixes: 7290ce1423c3 ("selftests/timers: Add clocksource-switch test from timetest suite")
Signed-off-by: Qingshuang Fu <fuqingshuang@kylinos.cn>
---
 .../selftests/timers/clocksource-switch.c     | 23 ++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/timers/clocksource-switch.c b/tools/testing/selftests/timers/clocksource-switch.c
index db62a764c29e..2e86f56d953e 100644
--- a/tools/testing/selftests/timers/clocksource-switch.c
+++ b/tools/testing/selftests/timers/clocksource-switch.c
@@ -40,16 +40,23 @@
 int get_clocksources(char list[][30])
 {
 	int fd, i;
-	size_t size;
+	ssize_t size;
 	char buf[512];
 	char *head, *tmp;
 
 	fd = open("/sys/devices/system/clocksource/clocksource0/available_clocksource", O_RDONLY);
+	if (fd < 0)
+		return 0;
 
-	size = read(fd, buf, 512);
+	size = read(fd, buf, sizeof(buf) - 1);
 
 	close(fd);
 
+	if (size <= 0)
+		return 0;
+
+	buf[size] = '\0';
+
 	for (i = 0; i < 10; i++)
 		list[i][0] = '\0';
 
@@ -74,11 +81,21 @@ int get_clocksources(char list[][30])
 
 int get_cur_clocksource(char *buf, size_t size)
 {
+	ssize_t len;
 	int fd;
 
 	fd = open("/sys/devices/system/clocksource/clocksource0/current_clocksource", O_RDONLY);
+	if (fd < 0)
+		return -1;
+
+	len = read(fd, buf, size - 1);
+
+	close(fd);
+
+	if (len <= 0)
+		return -1;
 
-	size = read(fd, buf, size);
+	buf[len] = '\0';
 
 	return 0;
 }

base-commit: 66498c75b4f8017f62d720d9b59675bdf3abce91
-- 
2.25.1
Re: [PATCH selftests/timers] selftests/timers: clocksource-switch: fix unchecked open()/read()
Posted by John Stultz 1 month ago
On Mon, Aug 24, 2026 at 11:46 PM Qingshuang Fu <fffsqian@163.com> wrote:
>
> From: Qingshuang Fu <fuqingshuang@kylinos.cn>
>
> get_clocksources() and get_cur_clocksource() open and read the
> clocksource sysfs files without checking the return values and without
> NUL-terminating the buffer.
>
> If open() fails, read() is called on fd -1 and returns -1, which is
> stored in a size_t as SIZE_MAX.  The token-scanning loop in
> get_clocksources() then walks past the uninitialized stack buffer,
> reading and writing out of bounds.  Even on success the buffer is not
> NUL-terminated, so a fully read buffer makes the inner scan run past the
> data, and get_cur_clocksource() lets change_clocksource() call strlen()
> on a non-terminated buffer.
>
> Check the open()/read() results, use ssize_t for the read length,
> NUL-terminate the buffer, and close the fd in get_cur_clocksource().
>
> Fixes: 7290ce1423c3 ("selftests/timers: Add clocksource-switch test from timetest suite")
> Signed-off-by: Qingshuang Fu <fuqingshuang@kylinos.cn>

Acked-by: John Stultz <jstultz@google.com>