On Thu, Jul 27, 2023 at 9:49 PM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> On Thu, Jul 27, 2023 at 11:43:09AM +0000, Yafang Shao wrote:
>
> SNIP
>
> > +static int verify_link_info(int fd, enum bpf_perf_event_type type, long addr, ssize_t offset)
> > +{
> > + struct bpf_link_info info;
> > + __u32 len = sizeof(info);
> > + char buf[PATH_MAX];
> > + int err = 0;
> > +
> > + memset(&info, 0, sizeof(info));
> > + buf[0] = '\0';
> > +
> > +again:
> > + err = bpf_link_get_info_by_fd(fd, &info, &len);
> > + if (!ASSERT_OK(err, "get_link_info"))
> > + return -1;
> > +
> > + switch (info.type) {
> > + case BPF_LINK_TYPE_PERF_EVENT:
> > + if (!ASSERT_EQ(info.perf_event.type, type, "perf_type_match"))
> > + return -1;
> > +
> > + switch (info.perf_event.type) {
> > + case BPF_PERF_EVENT_KPROBE:
> > + case BPF_PERF_EVENT_KRETPROBE:
> > + ASSERT_EQ(info.perf_event.kprobe.offset, offset, "kprobe_offset");
> > +
> > + /* In case kptr setting is not permitted or MAX_SYMS is reached */
> > + if (addr) {
> > + long addrs[2] = {
> > + addr + offset,
> > + addr + 0x4, /* For ENDBDR */
> > + };
> > +
> > + ASSERT_IN_ARRAY(info.perf_event.kprobe.addr, addrs, "kprobe_addr");
>
> we have check for IBT in get_func_ip_test, it might be easier
> to use the same in here as well and do the exact check
Thanks for your information!
will change it.
>
> we wouldn't need the ASSERT_IN_ARRAY then and would be correct
> wrt other archs
>
>
> SNIP
>
> > +static void test_uprobe_fill_link_info(struct test_fill_link_info *skel,
> > + enum bpf_perf_event_type type, ssize_t offset,
> > + bool retprobe)
> > +{
> > + int link_fd, err;
> > +
> > + skel->links.uprobe_run = bpf_program__attach_uprobe(skel->progs.uprobe_run, retprobe,
> > + 0, /* self pid */
> > + UPROBE_FILE, offset);
> > + if (!ASSERT_OK_PTR(skel->links.uprobe_run, "attach_uprobe"))
> > + return;
> > +
> > + link_fd = bpf_link__fd(skel->links.uprobe_run);
> > + if (!ASSERT_GE(link_fd, 0, "link_fd"))
> > + return;
> > +
> > + err = verify_link_info(link_fd, type, 0, offset);
> > + ASSERT_OK(err, "verify_link_info");
> > + bpf_link__detach(skel->links.uprobe_run);
> > +}
> > +
> > +void serial_test_fill_link_info(void)
>
> why does it need to be serial?
Ah, it can run in parallel. will change it.
>
> > +{
> > + struct test_fill_link_info *skel;
> > + ssize_t offset;
> > +
> > + skel = test_fill_link_info__open_and_load();
> > + if (!ASSERT_OK_PTR(skel, "skel_open"))
> > + goto cleanup;
> > +
> > + /* load kallsyms to compare the addr */
> > + if (!ASSERT_OK(load_kallsyms_refresh(), "load_kallsyms_refresh"))
> > + return;
> > + if (test__start_subtest("kprobe_link_info"))
> > + test_kprobe_fill_link_info(skel, BPF_PERF_EVENT_KPROBE, false, false);
> > + if (test__start_subtest("kretprobe_link_info"))
> > + test_kprobe_fill_link_info(skel, BPF_PERF_EVENT_KRETPROBE, true, false);
> > + if (test__start_subtest("fill_invalid_user_buff"))
> > + test_kprobe_fill_link_info(skel, BPF_PERF_EVENT_KPROBE, false, true);
> > + if (test__start_subtest("tracepoint_link_info"))
> > + test_tp_fill_link_info(skel);
> > +
> > + offset = get_uprobe_offset(&uprobe_func);
> > + if (test__start_subtest("uprobe_link_info"))
> > + test_uprobe_fill_link_info(skel, BPF_PERF_EVENT_UPROBE, offset, false);
> > + if (test__start_subtest("uretprobe_link_info"))
> > + test_uprobe_fill_link_info(skel, BPF_PERF_EVENT_URETPROBE, offset, true);
>
> do you plan to add kprobe_multi link test as well?
will add it in the next version.
--
Regards
Yafang
