[PATCH bpf,v2,0/2] bpf: fix incorrect name check pass logic in btf_name_valid_section

[PATCH bpf,v2,0/2] bpf: fix incorrect name check pass logic in btf_name_valid_section

[Jeongjun Park]

This patch was written to fix an issue where btf_name_valid_section() would 
not properly check names with certain conditions and would throw an OOB vuln. 
And selftest was added to verify this patch.

Jeongjun Park (2):
  bpf: add check for invalid name in btf_name_valid_section()
  selftest/bpf : Add a selftest test case to check for incorrect names

  kernel/bpf/btf.c                             |  4 ++-
  tools/testing/selftests/bpf/prog_tests/btf.c | 34 ++++++++++++++++++++
  2 files changed, 37 insertions(+), 1 deletion(-)

[PATCH bpf,v2,1/2] bpf: add check for invalid name in btf_name_valid_section()

[Jeongjun Park]

If the length of the name string is 1 and the value of name[0] is NULL
byte, an OOB vulnerability occurs in btf_name_valid_section() and the
return value is true, so the invalid name passes the check.

To solve this, you need to check if the first position is NULL byte and 
if the first character is printable.

Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Fixes: bd70a8fb7ca4 ("bpf: Allow all printable characters in BTF DATASEC names")
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 kernel/bpf/btf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 520f49f422fe..f1e91bf367fa 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -823,9 +823,11 @@ static bool btf_name_valid_section(const struct btf *btf, u32 offset)
 	const char *src = btf_str_by_offset(btf, offset);
 	const char *src_limit;
 
+	if (!*src)
+		return false;
+
 	/* set a limit on identifier length */
 	src_limit = src + KSYM_NAME_LEN;
-	src++;
 	while (*src && src < src_limit) {
 		if (!isprint(*src))
 			return false;
--

[PATCH bpf,v2,2/2] selftest/bpf : Add a selftest test case to check for incorrect names

[Jeongjun Park]

Add selftest for cases where btf_name_valid_section() does not properly 
check for certain types of names.

Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/btf.c | 34 ++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
index 00965a6e83bb..61de88cf4ad0 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -3550,6 +3550,40 @@ static struct btf_raw_test raw_tests[] = {
 	},
 	BTF_STR_SEC("\0x\0?.foo bar:buz"),
 },
+{
+	.descr = "datasec: name with non-printable first char not is ok",
+	.raw_types = {
+		/* int */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),  /* [1] */
+		/* VAR x */                                     /* [2] */
+		BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_VAR, 0, 0), 1),
+		BTF_VAR_STATIC,
+		/* DATASEC ?.data */                            /* [3] */
+		BTF_TYPE_ENC(3, BTF_INFO_ENC(BTF_KIND_DATASEC, 0, 1), 4),
+		BTF_VAR_SECINFO_ENC(2, 0, 4),
+		BTF_END_RAW,
+	},
+	BTF_STR_SEC("\0x\0\7foo"),
+	.err_str = "Invalid name",
+	.btf_load_err = true,
+},
+{
+	.descr = "datasec: name '\\0' is not ok",
+	.raw_types = {
+		/* int */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),  /* [1] */
+		/* VAR x */                                     /* [2] */
+		BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_VAR, 0, 0), 1),
+		BTF_VAR_STATIC,
+		/* DATASEC \0 */                                /* [3] */
+		BTF_TYPE_ENC(3, BTF_INFO_ENC(BTF_KIND_DATASEC, 0, 1), 4),
+		BTF_VAR_SECINFO_ENC(2, 0, 4),
+		BTF_END_RAW,
+	},
+	BTF_STR_SEC("\0x\0"),
+	.err_str = "Invalid name",
+	.btf_load_err = true,
+},
 {
 	.descr = "type name '?foo' is not ok",
 	.raw_types = {
--

Re: [PATCH bpf,v2,0/2] bpf: fix incorrect name check pass logic in btf_name_valid_section

[Eduard Zingerman]

On Sat, 2024-08-31 at 14:45 +0900, Jeongjun Park wrote:
> This patch was written to fix an issue where btf_name_valid_section() would 
> not properly check names with certain conditions and would throw an OOB vuln. 
> And selftest was added to verify this patch.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

Re: [PATCH bpf,v2,0/2] bpf: fix incorrect name check pass logic in btf_name_valid_section

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Sat, 31 Aug 2024 14:45:25 +0900 you wrote:
> This patch was written to fix an issue where btf_name_valid_section() would
> not properly check names with certain conditions and would throw an OOB vuln.
> And selftest was added to verify this patch.
> 
> Jeongjun Park (2):
>   bpf: add check for invalid name in btf_name_valid_section()
>   selftest/bpf : Add a selftest test case to check for incorrect names
> 
> [...]

Here is the summary with links:
  - [bpf,v2,1/2] bpf: add check for invalid name in btf_name_valid_section()
    https://git.kernel.org/bpf/bpf/c/bb6705c3f93b
  - [bpf,v2,2/2] selftest/bpf : Add a selftest test case to check for incorrect names
    https://git.kernel.org/bpf/bpf/c/743070894724

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html