| Message ID | 20221123035422.872531-2-sdf@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f17472d4599697d701aa239b4c475a506bccfd19 |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,1/2] selftests/bpf: Add reproducer for decl_tag in func_proto argument | expand |
On 11/22/22 7:54 PM, Stanislav Fomichev wrote: > Syzkaller managed to hit anoher decl_tag issue: > > btf_func_proto_check kernel/bpf/btf.c:4506 [inline] > btf_check_all_types kernel/bpf/btf.c:4734 [inline] > btf_parse_type_sec+0x1175/0x1980 kernel/bpf/btf.c:4763 > btf_parse kernel/bpf/btf.c:5042 [inline] > btf_new_fd+0x65a/0xb00 kernel/bpf/btf.c:6709 > bpf_btf_load+0x6f/0x90 kernel/bpf/syscall.c:4342 > __sys_bpf+0x50a/0x6c0 kernel/bpf/syscall.c:5034 > __do_sys_bpf kernel/bpf/syscall.c:5093 [inline] > __se_sys_bpf kernel/bpf/syscall.c:5091 [inline] > __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5091 > do_syscall_64+0x54/0x70 arch/x86/entry/common.c:48 > > This seems similar to commit ea68376c8bed ("bpf: prevent decl_tag from > being referenced in func_proto") but for the argument. > > Reported-by: syzbot+8dd0551dda6020944c5d@syzkaller.appspotmail.com > Signed-off-by: Stanislav Fomichev <sdf@google.com> Acked-by: Yonghong Song <yhs@fb.com> > --- > kernel/bpf/btf.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > index 1a59cc7ad730..cb43cb842e16 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -4792,6 +4792,11 @@ static int btf_func_proto_check(struct btf_verifier_env *env, > break; > } > > + if (btf_type_is_resolve_source_only(arg_type)) { > + btf_verifier_log_type(env, t, "Invalid arg#%u", i + 1); > + return -EINVAL; > + } > + > if (args[i].name_off && > (!btf_name_offset_valid(btf, args[i].name_off) || > !btf_name_valid_identifier(btf, args[i].name_off))) {
On 11/23/22 4:54 AM, Stanislav Fomichev wrote: > Syzkaller managed to hit anoher decl_tag issue: > > btf_func_proto_check kernel/bpf/btf.c:4506 [inline] > btf_check_all_types kernel/bpf/btf.c:4734 [inline] > btf_parse_type_sec+0x1175/0x1980 kernel/bpf/btf.c:4763 > btf_parse kernel/bpf/btf.c:5042 [inline] > btf_new_fd+0x65a/0xb00 kernel/bpf/btf.c:6709 > bpf_btf_load+0x6f/0x90 kernel/bpf/syscall.c:4342 > __sys_bpf+0x50a/0x6c0 kernel/bpf/syscall.c:5034 > __do_sys_bpf kernel/bpf/syscall.c:5093 [inline] > __se_sys_bpf kernel/bpf/syscall.c:5091 [inline] > __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5091 > do_syscall_64+0x54/0x70 arch/x86/entry/common.c:48 > > This seems similar to commit ea68376c8bed ("bpf: prevent decl_tag from > being referenced in func_proto") but for the argument. > > Reported-by: syzbot+8dd0551dda6020944c5d@syzkaller.appspotmail.com > Signed-off-by: Stanislav Fomichev <sdf@google.com> > --- > kernel/bpf/btf.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > index 1a59cc7ad730..cb43cb842e16 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -4792,6 +4792,11 @@ static int btf_func_proto_check(struct btf_verifier_env *env, > break; > } > > + if (btf_type_is_resolve_source_only(arg_type)) { > + btf_verifier_log_type(env, t, "Invalid arg#%u", i + 1); > + return -EINVAL; > + } > + Applied, could you do a small follow-up cleanup: most of the error cases in the loop in btf_func_proto_check() bail out with err = -EINVAL + break and the above now deviates from that, but rightfully so given there's no good reason as we just return err anyway. Would be good to make this consistent with return -EINVAL / return err also for the other cases. > if (args[i].name_off && > (!btf_name_offset_valid(btf, args[i].name_off) || > !btf_name_valid_identifier(btf, args[i].name_off))) { > Thanks, Daniel
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 1a59cc7ad730..cb43cb842e16 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -4792,6 +4792,11 @@ static int btf_func_proto_check(struct btf_verifier_env *env, break; } + if (btf_type_is_resolve_source_only(arg_type)) { + btf_verifier_log_type(env, t, "Invalid arg#%u", i + 1); + return -EINVAL; + } + if (args[i].name_off && (!btf_name_offset_valid(btf, args[i].name_off) || !btf_name_valid_identifier(btf, args[i].name_off))) {
Syzkaller managed to hit anoher decl_tag issue: btf_func_proto_check kernel/bpf/btf.c:4506 [inline] btf_check_all_types kernel/bpf/btf.c:4734 [inline] btf_parse_type_sec+0x1175/0x1980 kernel/bpf/btf.c:4763 btf_parse kernel/bpf/btf.c:5042 [inline] btf_new_fd+0x65a/0xb00 kernel/bpf/btf.c:6709 bpf_btf_load+0x6f/0x90 kernel/bpf/syscall.c:4342 __sys_bpf+0x50a/0x6c0 kernel/bpf/syscall.c:5034 __do_sys_bpf kernel/bpf/syscall.c:5093 [inline] __se_sys_bpf kernel/bpf/syscall.c:5091 [inline] __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5091 do_syscall_64+0x54/0x70 arch/x86/entry/common.c:48 This seems similar to commit ea68376c8bed ("bpf: prevent decl_tag from being referenced in func_proto") but for the argument. Reported-by: syzbot+8dd0551dda6020944c5d@syzkaller.appspotmail.com Signed-off-by: Stanislav Fomichev <sdf@google.com> --- kernel/bpf/btf.c | 5 +++++ 1 file changed, 5 insertions(+)