On 10/12/22 3:04 PM, Stanislav Fomichev wrote:
Syzkaller was able to hit it with the following reproducer:
bpf$BPF_BTF_LOAD(0x12, &(0x7f0000000140)={&(0x7f0000001680)={{0xeb9f, 0x1, 0x0, 0x18, 0x0, 0x34, 0x34, 0xc, [@fwd={0xa}, @var={0x3, 0x0, 0x0, 0x11, 0x4, 0xffffffff}, @func_proto={0x0, 0x0, 0x0, 0xd, 0x2}, @struct]}, {0x0, [0x0, 0x0, 0x5f, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x6c]}}, &(0x7f00000004c0)=""/4096, 0x58, 0x1000, 0x1}, 0x20)
------------[ cut here ]------------
WARNING: CPU: 0 PID: 3609 at kernel/bpf/btf.c:1946
btf_type_id_size+0x2d5/0x9d0 kernel/bpf/btf.c:1946
Modules linked in:
CPU: 0 PID: 3609 Comm: syz-executor361 Not tainted
6.0.0-syzkaller-02734-g0326074ff465 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 09/22/2022
RIP: 0010:btf_type_id_size+0x2d5/0x9d0 kernel/bpf/btf.c:1946
Code: ef e8 7f 8e e4 ff 41 83 ff 0b 77 28 f6 44 24 10 18 75 3f e8 6d 91
e4 ff 44 89 fe bf 0e 00 00 00 e8 20 8e e4 ff e8 5b 91 e4 ff <0f> 0b 45
31 f6 e9 98 02 00 00 41 83 ff 12 74 18 e8 46 91 e4 ff 44
RSP: 0018:ffffc90003cefb40 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 0000000000000002 RCX: 0000000000000000
RDX: ffff8880259c0000 RSI: ffffffff81968415 RDI: 0000000000000005
RBP: ffff88801270ca00 R08: 0000000000000005 R09: 000000000000000e
R10: 0000000000000011 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000011 R14: ffff888026ee6424 R15: 0000000000000011
FS: 000055555641b300(0000) GS:ffff8880b9a00000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000f2e258 CR3: 000000007110e000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
btf_func_proto_check kernel/bpf/btf.c:4447 [inline]
btf_check_all_types kernel/bpf/btf.c:4723 [inline]
btf_parse_type_sec kernel/bpf/btf.c:4752 [inline]
btf_parse kernel/bpf/btf.c:5026 [inline]
btf_new_fd+0x1926/0x1e70 kernel/bpf/btf.c:6892
bpf_btf_load kernel/bpf/syscall.c:4324 [inline]
__sys_bpf+0xb7d/0x4cf0 kernel/bpf/syscall.c:5010
__do_sys_bpf kernel/bpf/syscall.c:5069 [inline]
__se_sys_bpf kernel/bpf/syscall.c:5067 [inline]
__x64_sys_bpf+0x75/0xb0 kernel/bpf/syscall.c:5067
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7f0fbae41c69
Code: 28 c3 e8 2a 14 00 00 66 2e 0f 1f 84 00 00 00 00 00 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 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffc8aeb6228 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f0fbae41c69
RDX: 0000000000000020 RSI: 0000000020000140 RDI: 0000000000000012
RBP: 00007f0fbae05e10 R08: 0000000000000000 R09: 0000000000000000
R10: 00000000ffffffff R11: 0000000000000246 R12: 00007f0fbae05ea0
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
</TASK>
Any reason we need that WARN_ON_ONCE in this place?
All callers except btf_array_check_member check the return value,
so it should be safe. Assuming btf_array_check_member should also be fine
because it hits 'btf_type_is_array()' condition.
Reported-by: syzbot+d8bd751aef7c6b39a344@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxx>
---
kernel/bpf/btf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index eba603cec2c5..999f62c697a7 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -1943,8 +1943,8 @@ const struct btf_type *btf_type_id_size(const struct btf *btf,
} else if (btf_type_is_ptr(size_type)) {
size = sizeof(void *);
} else {
- if (WARN_ON_ONCE(!btf_type_is_modifier(size_type) &&
- !btf_type_is_var(size_type)))
Thanks for the report.
Trying to recall the reason...
After the above "if...else if...", the modifier and var should be the only ones
left that may be able to resolve to a type with a size.
I suspect the type that failed the WARN_ON_ONCE here is the BTF_KIND_DECL_TAG
which was added after this original WARN_ON_ONCE. Could you confirm in the
above syzkaller BTF that BTF_KIND_DECL_TAG is the one triggering here? and could
you help to turn the above syzkaller BTF into a unittest in
tools/testing/selftests/bpf/prog_tests/btf.c.
If that is the case, it seems we have missed checking BTF_KIND_DECL_TAG earlier
before doing the actual btf_resolve(). The problem is in the
btf_func_proto_check(). I talked to Yonghong offline a little on how the
non-func-proto type is handling the invalid decl tag. There is a
btf_type_is_resolve_source_only() to ensure a few types (decl_tag is one of
them) can never be referred by other types. This check is needed in
bpf_func_proto_check() before doing the btf_resolve().
Something like this (may not compile) and probably need similar check in the
nr_args for loop a few lines below also:
diff --git i/kernel/bpf/btf.c w/kernel/bpf/btf.c
index eba603cec2c5..a19dbeecd2a4 100644
--- i/kernel/bpf/btf.c
+++ w/kernel/bpf/btf.c
@@ -4436,6 +4436,11 @@ static int btf_func_proto_check(struct btf_verifier_env *env,
return -EINVAL;
}
+ if (btf_type_is_resolve_source_only(ret_type)) {
+ btf_verifier_log_type(env, t, "Invalid return type");
+ return -EINVAL;
+ }
+
if (btf_type_needs_resolve(ret_type) &&
!env_type_is_resolved(env, ret_type_id)) {
err = btf_resolve(env, ret_type, ret_type_id);
