From: Eduard Zingerman <eddyz87@gmail.com>
To: Jeongjun Park <aha310510@gmail.com>, alexei.starovoitov@gmail.com
Cc: andrii@kernel.org, ast@kernel.org, bpf@vger.kernel.org,
daniel@iogearbox.net, haoluo@google.com,
john.fastabend@gmail.com, jolsa@kernel.org, kpsingh@kernel.org,
linux-kernel@vger.kernel.org, martin.lau@linux.dev,
sdf@fomichev.me, song@kernel.org, yonghong.song@linux.dev
Subject: Re: [PATCH bpf] bpf: add check for invalid name in btf_name_valid_section()
Date: Thu, 29 Aug 2024 18:26:00 -0700 [thread overview]
Message-ID: <3a48e38f29cc8c73e36a6d3339b9303571d522a8.camel@gmail.com> (raw)
In-Reply-To: <d1ca563d8f2f5b63e7b0ec8b91c57914c32f1679.camel@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]
On Wed, 2024-08-28 at 22:45 -0700, Eduard Zingerman wrote:
[...]
> I will prepare a test case.
> Probably tomorrow.
Please find test in the attachment. This test triggers KASAN error
report as in another attachment. (I enabled CONFIG_KASAN using
menuconfig on top of regular selftest config).
On Fri, Aug 23, 2024 at 3:43 AM Jeongjun Park <aha310510@gmail.com> wrote:
[...]
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 520f49f422fe..5c24ea1a65a4 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -823,6 +823,9 @@ 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;
> +
I think that correct fix would be as follows:
---
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index edad152cee8e..d583d76fcace 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -820,7 +820,6 @@ static bool btf_name_valid_section(const struct btf *btf, u32 offset)
/* set a limit on identifier length */
src_limit = src + KSYM_NAME_LEN;
- src++;
while (*src && src < src_limit) {
if (!isprint(*src))
return false;
[-- Attachment #2: bad-name-test.patch --]
[-- Type: text/x-patch, Size: 899 bytes --]
diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
index 00965a6e83bb..ea4d3f6d5de9 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -3550,6 +3550,21 @@ static struct btf_raw_test raw_tests[] = {
},
BTF_STR_SEC("\0x\0?.foo bar:buz"),
},
+{
+ .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"),
+},
{
.descr = "type name '?foo' is not ok",
.raw_types = {
[-- Attachment #3: bad-name-kasan-report.txt --]
[-- Type: text/plain, Size: 4997 bytes --]
[ 7.025728] ==================================================================
[ 7.025977] BUG: KASAN: slab-out-of-bounds in btf_datasec_check_meta+0x73c/0x7a0
[ 7.026185] Read of size 1 at addr ffff888108eaa5d4 by task test_progs/172
[ 7.026358]
[ 7.026430] CPU: 2 UID: 0 PID: 172 Comm: test_progs Tainted: G OE 6.11.0-rc4-00225-g0673888b8469-dirty #23
[ 7.026673] Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
[ 7.026673] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-2.fc40 04/01/2014
[ 7.026673] Call Trace:
[ 7.026673] <TASK>
[ 7.026673] dump_stack_lvl+0x5d/0x80
[ 7.026673] ? btf_datasec_check_meta+0x73c/0x7a0
[ 7.026673] print_report+0x171/0x502
[ 7.026673] ? btf_datasec_check_meta+0x73c/0x7a0
[ 7.026673] ? srso_alias_return_thunk+0x5/0xfbef5
[ 7.026673] ? __virt_addr_valid+0x20c/0x410
[ 7.026673] ? btf_datasec_check_meta+0x73c/0x7a0
[ 7.026673] kasan_report+0xda/0x1b0
[ 7.026673] ? btf_datasec_check_meta+0x73c/0x7a0
[ 7.026673] btf_datasec_check_meta+0x73c/0x7a0
[ 7.026673] btf_check_all_metas+0x32b/0xac0
[ 7.026673] btf_new_fd+0x6fb/0x2dc0
[ 7.026673] ? srso_alias_return_thunk+0x5/0xfbef5
[ 7.026673] ? cred_has_capability.isra.0+0x139/0x230
[ 7.026673] ? __pfx_cred_has_capability.isra.0+0x10/0x10
[ 7.026673] ? srso_alias_return_thunk+0x5/0xfbef5
[ 7.026673] ? __pfx_btf_new_fd+0x10/0x10
[ 7.026673] ? __pfx_lock_release+0x10/0x10
[ 7.026673] ? srso_alias_return_thunk+0x5/0xfbef5
[ 7.026673] ? security_capable+0x6f/0xb0
[ 7.026673] ? srso_alias_return_thunk+0x5/0xfbef5
[ 7.026673] __sys_bpf+0x1251/0x4650
[ 7.026673] ? srso_alias_return_thunk+0x5/0xfbef5
[ 7.026673] ? __pfx___sys_bpf+0x10/0x10
[ 7.026673] ? srso_alias_return_thunk+0x5/0xfbef5
[ 7.026673] ? find_held_lock+0x2d/0x110
[ 7.026673] ? srso_alias_return_thunk+0x5/0xfbef5
[ 7.026673] ? lock_release+0x45a/0x7a0
[ 7.026673] ? __pfx_lock_release+0x10/0x10
[ 7.026673] ? __pfx___rseq_handle_notify_resume+0x10/0x10
[ 7.026673] __x64_sys_bpf+0x7b/0xc0
[ 7.026673] ? srso_alias_return_thunk+0x5/0xfbef5
[ 7.026673] ? lockdep_hardirqs_on+0x7b/0x100
[ 7.026673] do_syscall_64+0x68/0x140
[ 7.026673] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 7.026673] RIP: 0033:0x7f9398d281dd
[ 7.026673] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 0b dc 0c 00 f7 d8 64 89 01 48
[ 7.026673] RSP: 002b:00007ffddc11b488 EFLAGS: 00000206 ORIG_RAX: 0000000000000141
[ 7.026673] RAX: ffffffffffffffda RBX: 00007ffddc11b9f8 RCX: 00007f9398d281dd
[ 7.026673] RDX: 0000000000000028 RSI: 00007ffddc11b500 RDI: 0000000000000012
[ 7.026673] RBP: 00007ffddc11b4a0 R08: 00007ffddc11b320 R09: 00007ffddc11b500
[ 7.026673] R10: 0000000000000007 R11: 0000000000000206 R12: 0000000000000003
[ 7.026673] R13: 0000000000000000 R14: 00007f93a0114000 R15: 0000000001090d90
[ 7.026673] </TASK>
[ 7.026673]
[ 7.026673] Allocated by task 172:
[ 7.026673] kasan_save_stack+0x30/0x50
[ 7.026673] kasan_save_track+0x14/0x30
[ 7.026673] __kasan_kmalloc+0x8f/0xa0
[ 7.026673] __kmalloc_node_noprof+0x1ac/0x490
[ 7.026673] btf_new_fd+0x3e4/0x2dc0
[ 7.026673] __sys_bpf+0x1251/0x4650
[ 7.026673] __x64_sys_bpf+0x7b/0xc0
[ 7.026673] do_syscall_64+0x68/0x140
[ 7.026673] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 7.026673]
[ 7.026673] The buggy address belongs to the object at ffff888108eaa580
[ 7.026673] which belongs to the cache kmalloc-96 of size 96
[ 7.026673] The buggy address is located 0 bytes to the right of
[ 7.026673] allocated 84-byte region [ffff888108eaa580, ffff888108eaa5d4)
[ 7.026673]
[ 7.026673] The buggy address belongs to the physical page:
[ 7.026673] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x108eaa
[ 7.026673] flags: 0x2fffe0000000000(node=0|zone=2|lastcpupid=0x7fff)
[ 7.026673] page_type: 0xfdffffff(slab)
[ 7.026673] raw: 02fffe0000000000 ffff888100042280 dead000000000122 0000000000000000
[ 7.026673] raw: 0000000000000000 0000000080200020 00000001fdffffff 0000000000000000
[ 7.026673] page dumped because: kasan: bad access detected
[ 7.026673]
[ 7.026673] Memory state around the buggy address:
[ 7.026673] ffff888108eaa480: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
[ 7.026673] ffff888108eaa500: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
[ 7.026673] >ffff888108eaa580: 00 00 00 00 00 00 00 00 00 00 04 fc fc fc fc fc
[ 7.026673] ^
[ 7.026673] ffff888108eaa600: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 7.026673] ffff888108eaa680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 7.026673] ==================================================================
next prev parent reply other threads:[~2024-08-30 1:26 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-23 10:43 [PATCH bpf] bpf: add check for invalid name in btf_name_valid_section() Jeongjun Park
2024-08-29 2:36 ` Alexei Starovoitov
2024-08-29 3:45 ` Jeongjun Park
2024-08-29 5:45 ` Eduard Zingerman
2024-08-30 1:26 ` Eduard Zingerman [this message]
2024-08-30 2:03 ` Jeongjun Park
2024-08-30 9:42 ` Eduard Zingerman
2024-08-30 11:41 ` Jeongjun Park
2024-08-30 18:04 ` Eduard Zingerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3a48e38f29cc8c73e36a6d3339b9303571d522a8.camel@gmail.com \
--to=eddyz87@gmail.com \
--cc=aha310510@gmail.com \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=sdf@fomichev.me \
--cc=song@kernel.org \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox