linux-kernel - [PATCH bpf] bpf: Refactoring btf_name_valid_identifier() and btf_name_valid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240819160758.296567-1-aha310510@gmail.com>
Date: Tue, 20 Aug 2024 01:07:58 +0900
From: Jeongjun Park <aha310510@...il.com>
To: martin.lau@...ux.dev,
	ast@...nel.org,
	daniel@...earbox.net,
	andrii@...nel.org
Cc: eddyz87@...il.com,
	song@...nel.org,
	yonghong.song@...ux.dev,
	john.fastabend@...il.com,
	kpsingh@...nel.org,
	sdf@...ichev.me,
	haoluo@...gle.com,
	jolsa@...nel.org,
	bpf@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Jeongjun Park <aha310510@...il.com>
Subject: [PATCH bpf] bpf: Refactoring btf_name_valid_identifier() and btf_name_valid_section()

Currently, btf_name_valid_identifier() and btf_name_valid_section() are
written in a while loop and use pointer operations, so it takes a long
time to understand the operation of the code. Therefore, I suggest
refactoring the code to make it easier to maintain.

In addition, btf_name_valid_section() does not check for the case where
src[0] is a NULL value, resulting in an out-of-bounds vuln. Therefore, a
check for this should be added.

Reported-by: Jeongjun Park <aha310510@...il.com>
Fixes: bd70a8fb7ca4 ("bpf: Allow all printable characters in BTF DATASEC names")
Signed-off-by: Jeongjun Park <aha310510@...il.com>
---
 kernel/bpf/btf.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 674b38c33c74..c1e2aead9141 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -794,21 +794,18 @@ static bool btf_name_valid_identifier(const struct btf *btf, u32 offset)
 {
 	/* offset must be valid */
 	const char *src = btf_str_by_offset(btf, offset);
-	const char *src_limit;
+	int i;
 
-	if (!__btf_name_char_ok(*src, true))
+	if (!__btf_name_char_ok(src[0], true))
 		return false;
 
 	/* set a limit on identifier length */
-	src_limit = src + KSYM_NAME_LEN;
-	src++;
-	while (*src && src < src_limit) {
-		if (!__btf_name_char_ok(*src, false))
+	for (i = 1; i < KSYM_NAME_LEN && src[i]; i++) {
+		if (!__btf_name_char_ok(src[i], false))
 			return false;
-		src++;
 	}
 
-	return !*src;
+	return !src[i];
 }
 
 /* Allow any printable character in DATASEC names */
@@ -816,18 +813,18 @@ static bool btf_name_valid_section(const struct btf *btf, u32 offset)
 {
 	/* offset must be valid */
 	const char *src = btf_str_by_offset(btf, offset);
-	const char *src_limit;
+	int i;
+
+	if (!src[0])
+		return false;
 
 	/* set a limit on identifier length */
-	src_limit = src + KSYM_NAME_LEN;
-	src++;
-	while (*src && src < src_limit) {
-		if (!isprint(*src))
+	for (i = 1; i < KSYM_NAME_LEN && src[i]; i++) {
+		if (!isprint(src[i]))
 			return false;
-		src++;
 	}
 
-	return !*src;
+	return !src[i];
 }
 
 static const char *__btf_name_by_offset(const struct btf *btf, u32 offset)
--