Skip to content

Commit 3d84d67

Browse files
committed
Merge branch 'bpf-fix-tracing-of-kfuncs-with-implicit-args'
Ihor Solodrai says: ==================== bpf: Fix tracing of kfuncs with implicit args Tejun reported an issue where a BPF program tracing a kfunc with KF_IMPLICIT_ARGS can crash the kernel [1]. This is caused by a bug in bpf_check_attach_target(): the btf_func_model for such a kfunc is computed from a wrong BTF prototype. For more details see the commit message of patch #1. The second patch adds a selftest that can catch this situation. The fix is a candidate for 7.1 backport. [1] sched-ext/scx#3687 (comment) --- v2->v3: * Replace btf_kfunc_accumulated_flags() with btf_kfunc_check_flag() following a discussion with Eduard. Inlining the hook walk is a worse option than a helper, because BTF_KFUNC_HOOK_MAX and co are internal to btf.c and exposing them is uglier. * remove reduntant btf_is_func check (Jiri) * formatting nit (Eduard) v2: https://lore.kernel.org/bpf/20260710192940.3020280-1-ihor.solodrai@linux.dev/ v1->v2: * Take a module reference in btf_attach_func_proto() around the btf_kfunc_accumulated_flags() call (sashiko) v1: https://lore.kernel.org/bpf/20260710005902.2234832-1-ihor.solodrai@linux.dev/ --- ==================== Link: https://patch.msgid.link/20260713235223.1639022-1-ihor.solodrai@linux.dev Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
2 parents 4967bd5 + 0af15f3 commit 3d84d67

5 files changed

Lines changed: 196 additions & 13 deletions

File tree

include/linux/btf.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,7 @@ const char *btf_str_by_offset(const struct btf *btf, u32 offset);
578578
struct btf *btf_parse_vmlinux(void);
579579
struct btf *bpf_prog_get_target_btf(const struct bpf_prog *prog);
580580
u32 *btf_kfunc_flags(const struct btf *btf, u32 kfunc_btf_id, const struct bpf_prog *prog);
581+
int btf_kfunc_check_flag(const struct btf *btf, u32 kfunc_btf_id, u32 flag);
581582
bool btf_kfunc_is_allowed(const struct btf *btf, u32 kfunc_btf_id, const struct bpf_prog *prog);
582583
u32 *btf_kfunc_is_modify_return(const struct btf *btf, u32 kfunc_btf_id,
583584
const struct bpf_prog *prog);

kernel/bpf/btf.c

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9114,6 +9114,35 @@ u32 *btf_kfunc_flags(const struct btf *btf, u32 kfunc_btf_id, const struct bpf_p
91149114
return btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id);
91159115
}
91169116

9117+
/*
9118+
* Check a single KF_* @flag on a kfunc across all of its hook sets.
9119+
* Returns:
9120+
* * 1 if @flag is set
9121+
* * 0 if @flag is not set
9122+
* * -EINVAL if @flag is set inconsistently across the sets
9123+
* * -ENOENT if kfunc_btf_id is not a registered kfunc
9124+
*/
9125+
int btf_kfunc_check_flag(const struct btf *btf, u32 kfunc_btf_id, u32 flag)
9126+
{
9127+
enum btf_kfunc_hook hook;
9128+
int res = -ENOENT;
9129+
bool is_set;
9130+
u32 *flags;
9131+
9132+
for (hook = 0; hook < BTF_KFUNC_HOOK_MAX; hook++) {
9133+
flags = btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id);
9134+
if (!flags)
9135+
continue;
9136+
is_set = *flags & flag;
9137+
if (res < 0)
9138+
res = is_set;
9139+
else if (res != is_set)
9140+
return -EINVAL;
9141+
}
9142+
9143+
return res;
9144+
}
9145+
91179146
u32 *btf_kfunc_is_modify_return(const struct btf *btf, u32 kfunc_btf_id,
91189147
const struct bpf_prog *prog)
91199148
{

kernel/bpf/verifier.c

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2584,24 +2584,25 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env, s16 offset)
25842584

25852585
#define KF_IMPL_SUFFIX "_impl"
25862586

2587-
static const struct btf_type *find_kfunc_impl_proto(struct bpf_verifier_env *env,
2587+
static const struct btf_type *find_kfunc_impl_proto(struct bpf_verifier_log *log,
25882588
struct btf *btf,
25892589
const char *func_name)
25902590
{
2591-
char *buf = env->tmp_str_buf;
25922591
const struct btf_type *func;
2592+
char buf[KSYM_NAME_LEN];
25932593
s32 impl_id;
25942594
int len;
25952595

2596-
len = snprintf(buf, TMP_STR_BUF_LEN, "%s%s", func_name, KF_IMPL_SUFFIX);
2597-
if (len < 0 || len >= TMP_STR_BUF_LEN) {
2598-
verbose(env, "function name %s%s is too long\n", func_name, KF_IMPL_SUFFIX);
2596+
len = snprintf(buf, sizeof(buf), "%s%s", func_name, KF_IMPL_SUFFIX);
2597+
if (len < 0 || len >= sizeof(buf)) {
2598+
bpf_log(log, "function name %s%s is too long\n",
2599+
func_name, KF_IMPL_SUFFIX);
25992600
return NULL;
26002601
}
26012602

26022603
impl_id = btf_find_by_name_kind(btf, buf, BTF_KIND_FUNC);
26032604
if (impl_id <= 0) {
2604-
verbose(env, "cannot find function %s in BTF\n", buf);
2605+
bpf_log(log, "cannot find function %s in BTF\n", buf);
26052606
return NULL;
26062607
}
26072608

@@ -2653,7 +2654,7 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env,
26532654
* can be found through the counterpart _impl kfunc.
26542655
*/
26552656
if (kfunc_flags && (*kfunc_flags & KF_IMPLICIT_ARGS))
2656-
func_proto = find_kfunc_impl_proto(env, btf, func_name);
2657+
func_proto = find_kfunc_impl_proto(&env->log, btf, func_name);
26572658
else
26582659
func_proto = btf_type_by_id(btf, func->type);
26592660

@@ -18880,6 +18881,47 @@ static int btf_id_allow_sleepable(u32 btf_id, unsigned long addr, const struct b
1888018881
return -EINVAL;
1888118882
}
1888218883

18884+
/*
18885+
* Resolve the prototype describing a trace target's real ABI. A
18886+
* KF_IMPLICIT_ARGS kfunc has its injected args stripped from the public
18887+
* prototype, so use the _impl prototype; other targets use their own.
18888+
*/
18889+
static const struct btf_type *
18890+
btf_attach_func_proto(struct bpf_verifier_log *log, struct btf *btf, u32 func_id)
18891+
{
18892+
const struct btf_type *func;
18893+
struct module *mod = NULL;
18894+
const char *name;
18895+
int implicit;
18896+
18897+
func = btf_type_by_id(btf, func_id);
18898+
if (!func || !btf_type_is_func(func))
18899+
return NULL;
18900+
name = btf_name_by_offset(btf, func->name_off);
18901+
18902+
/*
18903+
* btf_kfunc_check_flag() reads kfunc_set_tab, which for a module is
18904+
* stable only once it is live; hold a module ref across the read to
18905+
* exclude a concurrent module load.
18906+
*/
18907+
if (btf_is_module(btf)) {
18908+
mod = btf_try_get_module(btf);
18909+
if (!mod)
18910+
return NULL;
18911+
}
18912+
implicit = btf_kfunc_check_flag(btf, func_id, KF_IMPLICIT_ARGS);
18913+
module_put(mod);
18914+
18915+
if (implicit == -EINVAL) {
18916+
bpf_log(log, "kfunc %s has inconsistent KF_IMPLICIT_ARGS\n", name);
18917+
return NULL;
18918+
}
18919+
if (implicit > 0)
18920+
return find_kfunc_impl_proto(log, btf, name);
18921+
18922+
return btf_type_by_id(btf, func->type);
18923+
}
18924+
1888318925
int bpf_check_attach_target(struct bpf_verifier_log *log,
1888418926
const struct bpf_prog *prog,
1888518927
const struct bpf_prog *tgt_prog,
@@ -19128,8 +19170,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
1912819170
if (prog_extension &&
1912919171
btf_check_type_match(log, prog, btf, t))
1913019172
return -EINVAL;
19131-
t = btf_type_by_id(btf, t->type);
19132-
if (!btf_type_is_func_proto(t))
19173+
t = btf_attach_func_proto(log, btf, btf_id);
19174+
if (!t || !btf_type_is_func_proto(t))
1913319175
return -EINVAL;
1913419176

1913519177
if ((prog->aux->saved_dst_prog_type || prog->aux->saved_dst_attach_type) &&
@@ -19412,10 +19454,8 @@ int bpf_check_attach_btf_id_multi(struct btf *btf, struct bpf_prog *prog, u32 bt
1941219454
tname = btf_name_by_offset(btf, t->name_off);
1941319455
if (!tname)
1941419456
return -EINVAL;
19415-
if (!btf_type_is_func(t))
19416-
return -EINVAL;
19417-
t = btf_type_by_id(btf, t->type);
19418-
if (!btf_type_is_func_proto(t))
19457+
t = btf_attach_func_proto(NULL, btf, btf_id);
19458+
if (!t || !btf_type_is_func_proto(t))
1941919459
return -EINVAL;
1942019460
err = btf_distill_func_proto(NULL, btf, t, tname, &tgt_info->fmodel);
1942119461
if (err < 0)
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
3+
4+
#include <test_progs.h>
5+
#include "kfunc_implicit_args_tracing.skel.h"
6+
7+
void test_kfunc_implicit_args_tracing(void)
8+
{
9+
struct kfunc_implicit_args_tracing *skel;
10+
LIBBPF_OPTS(bpf_test_run_opts, topts);
11+
int err, fd;
12+
13+
skel = kfunc_implicit_args_tracing__open_and_load();
14+
if (!ASSERT_OK_PTR(skel, "open_and_load"))
15+
return;
16+
17+
err = kfunc_implicit_args_tracing__attach(skel);
18+
if (!ASSERT_OK(err, "attach"))
19+
goto cleanup;
20+
21+
fd = bpf_program__fd(skel->progs.trigger_implicit_arg);
22+
err = bpf_prog_test_run_opts(fd, &topts);
23+
if (!ASSERT_OK(err, "test_run"))
24+
goto cleanup;
25+
26+
ASSERT_EQ(topts.retval, 5, "kfunc_retval");
27+
ASSERT_EQ(skel->bss->fentry_arg_cnt, 2, "fentry_arg_cnt");
28+
ASSERT_NEQ(skel->bss->fentry_aux_arg, 0, "fentry_aux_arg");
29+
ASSERT_EQ(skel->bss->fentry_result, 1, "fentry_result");
30+
ASSERT_EQ(skel->bss->fexit_arg_cnt, 2, "fexit_arg_cnt");
31+
ASSERT_NEQ(skel->bss->fexit_aux_arg, 0, "fexit_aux_arg");
32+
ASSERT_EQ(skel->bss->fexit_result, 1, "fexit_result");
33+
34+
cleanup:
35+
kfunc_implicit_args_tracing__destroy(skel);
36+
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
3+
4+
#include <vmlinux.h>
5+
#include <bpf/bpf_helpers.h>
6+
#include <bpf/bpf_tracing.h>
7+
#include <errno.h>
8+
9+
extern int bpf_kfunc_implicit_arg(int a) __weak __ksym;
10+
11+
char _license[] SEC("license") = "GPL";
12+
13+
/* Shared arg checks; reports arg count and aux, returns 1 on success. */
14+
static __always_inline __u64
15+
check_implicit_args(void *ctx, __u64 *arg_cnt, __u64 *aux_arg)
16+
{
17+
__u64 a = 0, aux = 0, z = 0;
18+
__u64 result;
19+
__s64 err;
20+
21+
*arg_cnt = bpf_get_func_arg_cnt(ctx);
22+
result = *arg_cnt == 2;
23+
24+
err = bpf_get_func_arg(ctx, 0, &a);
25+
result &= err == 0 && (int)a == 5;
26+
27+
err = bpf_get_func_arg(ctx, 1, &aux);
28+
*aux_arg = aux;
29+
result &= err == 0 && aux != 0;
30+
31+
err = bpf_get_func_arg(ctx, 2, &z);
32+
result &= err == -EINVAL;
33+
34+
return result;
35+
}
36+
37+
__u64 fentry_result;
38+
__u64 fentry_arg_cnt;
39+
__u64 fentry_aux_arg;
40+
41+
SEC("fentry/bpf_kfunc_implicit_arg")
42+
int BPF_PROG(trace_implicit_arg_fentry)
43+
{
44+
__u64 ret = 0;
45+
__s64 err;
46+
47+
fentry_result = check_implicit_args(ctx, &fentry_arg_cnt, &fentry_aux_arg);
48+
49+
err = bpf_get_func_ret(ctx, &ret);
50+
fentry_result &= err == -EOPNOTSUPP;
51+
52+
return 0;
53+
}
54+
55+
__u64 fexit_result;
56+
__u64 fexit_arg_cnt;
57+
__u64 fexit_aux_arg;
58+
59+
SEC("fexit/bpf_kfunc_implicit_arg")
60+
int BPF_PROG(trace_implicit_arg_fexit)
61+
{
62+
__u64 ret = 0;
63+
__s64 err;
64+
65+
fexit_result = check_implicit_args(ctx, &fexit_arg_cnt, &fexit_aux_arg);
66+
67+
err = bpf_get_func_ret(ctx, &ret);
68+
fexit_result &= err == 0 && ret == 5;
69+
70+
return 0;
71+
}
72+
73+
SEC("syscall")
74+
int trigger_implicit_arg(void *ctx)
75+
{
76+
return bpf_kfunc_implicit_arg(5);
77+
}

0 commit comments

Comments
 (0)