Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] selftests/bpf: Ineffective error check for libbpf_find_prog_btf_id due to __u32 cast --
commit 208e2b6f875995feff87983db367727d95ea373b Author: Leon Hwang <[email protected]> selftests/bpf: Add tracing_multi bpf prog link info test Verify the link info works well for bpf prog targets of tracing_multi link. > diff --git a/tools/testing/selftests/bpf/prog_tests/fill_link_info.c > b/tools/testing/selftests/bpf/prog_tests/fill_link_info.c > index 0918321c8e63d..422e94f134094 100644 > --- a/tools/testing/selftests/bpf/prog_tests/fill_link_info.c > +++ b/tools/testing/selftests/bpf/prog_tests/fill_link_info.c > @@ -467,13 +489,49 @@ static int setup_tmulti_targets(const struct > bpf_program *prog, > return -1; > } > > +static int setup_tmulti_bpf_prog_targets(const struct bpf_program *prog, int > token_fd, > + const __u64 *cookies, > + const struct bpf_program * const > *target_progs, > + struct tmulti_target *targets, > + __u32 *btf_obj_id) > +{ > + struct bpf_prog_info prog_info = {}; > + __u32 len = sizeof(prog_info), func_btf_id; [Severity: Medium] libbpf_find_prog_btf_id() returns an int which can be negative to indicate an error. Since func_btf_id is declared as an unsigned 32-bit integer, does assigning the return value here cast any negative error code to a large positive value? > + int err, i, prog_fd; > + > + for (i = 0; i < TRACING_MULTI_BPF_CNT; i++) { > + memset(&prog_info, 0, sizeof(prog_info)); > + len = sizeof(prog_info); > + prog_info.jited_ksyms = ptr_to_u64(&targets[i].addr); > + prog_info.nr_jited_ksyms = 1; > + prog_fd = bpf_program__fd(target_progs[i]); > + err = bpf_prog_get_info_by_fd(prog_fd, &prog_info, &len); > + if (!ASSERT_OK(err, "bpf_prog_get_info_by_fd")) > + return -1; > + > + func_btf_id = libbpf_find_prog_btf_id(tmulti_bpf_funcs[i], > prog_fd, token_fd); > + if (!ASSERT_GT(func_btf_id, 0, "libbpf_find_prog_btf_id")) [Severity: Medium] If a negative error code is cast to a large positive value in the unsigned func_btf_id, will this greater-than-zero check incorrectly pass, masking test failures during BTF ID lookup and allowing the test to proceed with an invalid BTF ID? > + return -1; > + > + targets[i].cookie = cookies[i]; > + targets[i].id = prog_info.id; > + targets[i].func_btf_id = func_btf_id; > + } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=19
