Skip to content

L
linux
Commit a4a8b2ee authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files
Options

Merge branch 'bpf: Fix cookie values for kprobe multi' 

Jiri Olsa says:

====================

hi,
there's bug in kprobe_multi link that makes cookies misplaced when
using symbols to attach. The reason is that we sort symbols by name
but not adjacent cookie values. Current test did not find it because
bpf_fentry_test* are already sorted by name.

v3 changes:
  - fixed kprobe_multi bench test to filter out invalid entries
    from available_filter_functions

v2 changes:
  - rebased on top of bpf/master
  - checking if cookies are defined later in swap function [Andrii]
  - added acks

thanks,
jirka
====================
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 5e0b0a4c 73006702
Hide whitespace changes
Inline Side-by-side
Showing with 110 additions and 68 deletions
kernel/trace/bpf_trace.c
View file @ a4a8b2ee
......@@ -2423,7 +2423,7 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long entry_ip,
kprobe_multi_link_prog_run(link, entry_ip, regs);
}
static int symbols_cmp(const void *a, const void *b)
static int symbols_cmp_r(const void *a, const void *b, const void *priv)
{
const char **str_a = (const char **) a;
const char **str_b = (const char **) b;
......@@ -2431,6 +2431,28 @@ static int symbols_cmp(const void *a, const void *b)
return strcmp(*str_a, *str_b);
}
struct multi_symbols_sort {
const char **funcs;
u64 *cookies;
};
static void symbols_swap_r(void *a, void *b, int size, const void *priv)
{
const struct multi_symbols_sort *data = priv;
const char **name_a = a, **name_b = b;
swap(*name_a, *name_b);
/* If defined, swap also related cookies. */
if (data->cookies) {
u64 *cookie_a, *cookie_b;
cookie_a = data->cookies + (name_a - data->funcs);
cookie_b = data->cookies + (name_b - data->funcs);
swap(*cookie_a, *cookie_b);
}
}
int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
{
struct bpf_kprobe_multi_link *link = NULL;
......@@ -2468,38 +2490,46 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
if (!addrs)
return -ENOMEM;
ucookies = u64_to_user_ptr(attr->link_create.kprobe_multi.cookies);
if (ucookies) {
cookies = kvmalloc_array(cnt, sizeof(*addrs), GFP_KERNEL);
if (!cookies) {
err = -ENOMEM;
goto error;
}
if (copy_from_user(cookies, ucookies, size)) {
err = -EFAULT;
goto error;
}
}
if (uaddrs) {
if (copy_from_user(addrs, uaddrs, size)) {
err = -EFAULT;
goto error;
}
} else {
struct multi_symbols_sort data = {
.cookies = cookies,
};
struct user_syms us;
err = copy_user_syms(&us, usyms, cnt);
if (err)
goto error;
sort(us.syms, cnt, sizeof(*us.syms), symbols_cmp, NULL);
if (cookies)
data.funcs = us.syms;
sort_r(us.syms, cnt, sizeof(*us.syms), symbols_cmp_r,
symbols_swap_r, &data);
err = ftrace_lookup_symbols(us.syms, cnt, addrs);
free_user_syms(&us);
if (err)
goto error;
}
ucookies = u64_to_user_ptr(attr->link_create.kprobe_multi.cookies);
if (ucookies) {
cookies = kvmalloc_array(cnt, sizeof(*addrs), GFP_KERNEL);
if (!cookies) {
err = -ENOMEM;
goto error;
}
if (copy_from_user(cookies, ucookies, size)) {
err = -EFAULT;
goto error;
}
}
link = kzalloc(sizeof(*link), GFP_KERNEL);
if (!link) {
err = -ENOMEM;
......
kernel/trace/ftrace.c
View file @ a4a8b2ee
......@@ -8029,15 +8029,23 @@ static int kallsyms_callback(void *data, const char *name,
struct module *mod, unsigned long addr)
{
struct kallsyms_data *args = data;
const char **sym;
int idx;
if (!bsearch(&name, args->syms, args->cnt, sizeof(*args->syms), symbols_cmp))
sym = bsearch(&name, args->syms, args->cnt, sizeof(*args->syms), symbols_cmp);
if (!sym)
return 0;
idx = sym - args->syms;
if (args->addrs[idx])
return 0;
addr = ftrace_location(addr);
if (!addr)
return 0;
args->addrs[args->found++] = addr;
args->addrs[idx] = addr;
args->found++;
return args->found == args->cnt ? 1 : 0;
}
......@@ -8062,6 +8070,7 @@ int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *a
struct kallsyms_data args;
int err;
memset(addrs, 0, sizeof(*addrs) * cnt);
args.addrs = addrs;
args.syms = sorted_syms;
args.cnt = cnt;
......
tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
View file @ a4a8b2ee
......@@ -121,24 +121,24 @@ static void kprobe_multi_link_api_subtest(void)
})
GET_ADDR("bpf_fentry_test1", addrs[0]);
GET_ADDR("bpf_fentry_test2", addrs[1]);
GET_ADDR("bpf_fentry_test3", addrs[2]);
GET_ADDR("bpf_fentry_test4", addrs[3]);
GET_ADDR("bpf_fentry_test5", addrs[4]);
GET_ADDR("bpf_fentry_test6", addrs[5]);
GET_ADDR("bpf_fentry_test7", addrs[6]);
GET_ADDR("bpf_fentry_test3", addrs[1]);
GET_ADDR("bpf_fentry_test4", addrs[2]);
GET_ADDR("bpf_fentry_test5", addrs[3]);
GET_ADDR("bpf_fentry_test6", addrs[4]);
GET_ADDR("bpf_fentry_test7", addrs[5]);
GET_ADDR("bpf_fentry_test2", addrs[6]);
GET_ADDR("bpf_fentry_test8", addrs[7]);
#undef GET_ADDR
cookies[0] = 1;
cookies[1] = 2;
cookies[2] = 3;
cookies[3] = 4;
cookies[4] = 5;
cookies[5] = 6;
cookies[6] = 7;
cookies[7] = 8;
cookies[0] = 1; /* bpf_fentry_test1 */
cookies[1] = 2; /* bpf_fentry_test3 */
cookies[2] = 3; /* bpf_fentry_test4 */
cookies[3] = 4; /* bpf_fentry_test5 */
cookies[4] = 5; /* bpf_fentry_test6 */
cookies[5] = 6; /* bpf_fentry_test7 */
cookies[6] = 7; /* bpf_fentry_test2 */
cookies[7] = 8; /* bpf_fentry_test8 */
opts.kprobe_multi.addrs = (const unsigned long *) &addrs;
opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
......@@ -149,14 +149,14 @@ static void kprobe_multi_link_api_subtest(void)
if (!ASSERT_GE(link1_fd, 0, "link1_fd"))
goto cleanup;
cookies[0] = 8;
cookies[1] = 7;
cookies[2] = 6;
cookies[3] = 5;
cookies[4] = 4;
cookies[5] = 3;
cookies[6] = 2;
cookies[7] = 1;
cookies[0] = 8; /* bpf_fentry_test1 */
cookies[1] = 7; /* bpf_fentry_test3 */
cookies[2] = 6; /* bpf_fentry_test4 */
cookies[3] = 5; /* bpf_fentry_test5 */
cookies[4] = 4; /* bpf_fentry_test6 */
cookies[5] = 3; /* bpf_fentry_test7 */
cookies[6] = 2; /* bpf_fentry_test2 */
cookies[7] = 1; /* bpf_fentry_test8 */
opts.kprobe_multi.flags = BPF_F_KPROBE_MULTI_RETURN;
prog_fd = bpf_program__fd(skel->progs.test_kretprobe);
......@@ -181,12 +181,12 @@ static void kprobe_multi_attach_api_subtest(void)
struct kprobe_multi *skel = NULL;
const char *syms[8] = {
"bpf_fentry_test1",
"bpf_fentry_test2",
"bpf_fentry_test3",
"bpf_fentry_test4",
"bpf_fentry_test5",
"bpf_fentry_test6",
"bpf_fentry_test7",
"bpf_fentry_test2",
"bpf_fentry_test8",
};
__u64 cookies[8];
......@@ -198,14 +198,14 @@ static void kprobe_multi_attach_api_subtest(void)
skel->bss->pid = getpid();
skel->bss->test_cookie = true;
cookies[0] = 1;
cookies[1] = 2;
cookies[2] = 3;
cookies[3] = 4;
cookies[4] = 5;
cookies[5] = 6;
cookies[6] = 7;
cookies[7] = 8;
cookies[0] = 1; /* bpf_fentry_test1 */
cookies[1] = 2; /* bpf_fentry_test3 */
cookies[2] = 3; /* bpf_fentry_test4 */
cookies[3] = 4; /* bpf_fentry_test5 */
cookies[4] = 5; /* bpf_fentry_test6 */
cookies[5] = 6; /* bpf_fentry_test7 */
cookies[6] = 7; /* bpf_fentry_test2 */
cookies[7] = 8; /* bpf_fentry_test8 */
opts.syms = syms;
opts.cnt = ARRAY_SIZE(syms);
......@@ -216,14 +216,14 @@ static void kprobe_multi_attach_api_subtest(void)
if (!ASSERT_OK_PTR(link1, "bpf_program__attach_kprobe_multi_opts"))
goto cleanup;
cookies[0] = 8;
cookies[1] = 7;
cookies[2] = 6;
cookies[3] = 5;
cookies[4] = 4;
cookies[5] = 3;
cookies[6] = 2;
cookies[7] = 1;
cookies[0] = 8; /* bpf_fentry_test1 */
cookies[1] = 7; /* bpf_fentry_test3 */
cookies[2] = 6; /* bpf_fentry_test4 */
cookies[3] = 5; /* bpf_fentry_test5 */
cookies[4] = 4; /* bpf_fentry_test6 */
cookies[5] = 3; /* bpf_fentry_test7 */
cookies[6] = 2; /* bpf_fentry_test2 */
cookies[7] = 1; /* bpf_fentry_test8 */
opts.retprobe = true;
......
tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
View file @ a4a8b2ee
......@@ -364,6 +364,9 @@ static int get_syms(char ***symsp, size_t *cntp)
continue;
if (!strncmp(name, "rcu_", 4))
continue;
if (!strncmp(name, "__ftrace_invalid_address__",
sizeof("__ftrace_invalid_address__") - 1))
continue;
err = hashmap__add(map, name, NULL);
if (err) {
free(name);
......
tools/testing/selftests/bpf/progs/kprobe_multi.c
View file @ a4a8b2ee
......@@ -54,21 +54,21 @@ static void kprobe_multi_check(void *ctx, bool is_return)
if (is_return) {
SET(kretprobe_test1_result, &bpf_fentry_test1, 8);
SET(kretprobe_test2_result, &bpf_fentry_test2, 7);
SET(kretprobe_test3_result, &bpf_fentry_test3, 6);
SET(kretprobe_test4_result, &bpf_fentry_test4, 5);
SET(kretprobe_test5_result, &bpf_fentry_test5, 4);
SET(kretprobe_test6_result, &bpf_fentry_test6, 3);
SET(kretprobe_test7_result, &bpf_fentry_test7, 2);
SET(kretprobe_test2_result, &bpf_fentry_test2, 2);
SET(kretprobe_test3_result, &bpf_fentry_test3, 7);
SET(kretprobe_test4_result, &bpf_fentry_test4, 6);
SET(kretprobe_test5_result, &bpf_fentry_test5, 5);
SET(kretprobe_test6_result, &bpf_fentry_test6, 4);
SET(kretprobe_test7_result, &bpf_fentry_test7, 3);
SET(kretprobe_test8_result, &bpf_fentry_test8, 1);
} else {
SET(kprobe_test1_result, &bpf_fentry_test1, 1);
SET(kprobe_test2_result, &bpf_fentry_test2, 2);
SET(kprobe_test3_result, &bpf_fentry_test3, 3);
SET(kprobe_test4_result, &bpf_fentry_test4, 4);
SET(kprobe_test5_result, &bpf_fentry_test5, 5);
SET(kprobe_test6_result, &bpf_fentry_test6, 6);
SET(kprobe_test7_result, &bpf_fentry_test7, 7);
SET(kprobe_test2_result, &bpf_fentry_test2, 7);
SET(kprobe_test3_result, &bpf_fentry_test3, 2);
SET(kprobe_test4_result, &bpf_fentry_test4, 3);
SET(kprobe_test5_result, &bpf_fentry_test5, 4);
SET(kprobe_test6_result, &bpf_fentry_test6, 5);
SET(kprobe_test7_result, &bpf_fentry_test7, 6);
SET(kprobe_test8_result, &bpf_fentry_test8, 8);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7