Skip to content
This repository was archived by the owner on Nov 8, 2023. It is now read-only.

Commit 5d5e3b4

Browse files
Xu Kuohaigregkh
authored andcommitted
bpf: Prevent tail call between progs attached to different hooks
[ Upstream commit 28ead3e ] bpf progs can be attached to kernel functions, and the attached functions can take different parameters or return different return values. If prog attached to one kernel function tail calls prog attached to another kernel function, the ctx access or return value verification could be bypassed. For example, if prog1 is attached to func1 which takes only 1 parameter and prog2 is attached to func2 which takes two parameters. Since verifier assumes the bpf ctx passed to prog2 is constructed based on func2's prototype, verifier allows prog2 to access the second parameter from the bpf ctx passed to it. The problem is that verifier does not prevent prog1 from passing its bpf ctx to prog2 via tail call. In this case, the bpf ctx passed to prog2 is constructed from func1 instead of func2, that is, the assumption for ctx access verification is bypassed. Another example, if BPF LSM prog1 is attached to hook file_alloc_security, and BPF LSM prog2 is attached to hook bpf_lsm_audit_rule_known. Verifier knows the return value rules for these two hooks, e.g. it is legal for bpf_lsm_audit_rule_known to return positive number 1, and it is illegal for file_alloc_security to return positive number. So verifier allows prog2 to return positive number 1, but does not allow prog1 to return positive number. The problem is that verifier does not prevent prog1 from calling prog2 via tail call. In this case, prog2's return value 1 will be used as the return value for prog1's hook file_alloc_security. That is, the return value rule is bypassed. This patch adds restriction for tail call to prevent such bypasses. Signed-off-by: Xu Kuohai <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]> Signed-off-by: Andrii Nakryiko <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent 96b1280 commit 5d5e3b4

File tree

2 files changed

+19
-3
lines changed

2 files changed

+19
-3
lines changed

include/linux/bpf.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,7 @@ struct bpf_map {
288288
* same prog type, JITed flag and xdp_has_frags flag.
289289
*/
290290
struct {
291+
const struct btf_type *attach_func_proto;
291292
spinlock_t lock;
292293
enum bpf_prog_type type;
293294
bool jited;

kernel/bpf/core.c

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2259,6 +2259,7 @@ bool bpf_prog_map_compatible(struct bpf_map *map,
22592259
{
22602260
enum bpf_prog_type prog_type = resolve_prog_type(fp);
22612261
bool ret;
2262+
struct bpf_prog_aux *aux = fp->aux;
22622263

22632264
if (fp->kprobe_override)
22642265
return false;
@@ -2268,7 +2269,7 @@ bool bpf_prog_map_compatible(struct bpf_map *map,
22682269
* in the case of devmap and cpumap). Until device checks
22692270
* are implemented, prohibit adding dev-bound programs to program maps.
22702271
*/
2271-
if (bpf_prog_is_dev_bound(fp->aux))
2272+
if (bpf_prog_is_dev_bound(aux))
22722273
return false;
22732274

22742275
spin_lock(&map->owner.lock);
@@ -2278,12 +2279,26 @@ bool bpf_prog_map_compatible(struct bpf_map *map,
22782279
*/
22792280
map->owner.type = prog_type;
22802281
map->owner.jited = fp->jited;
2281-
map->owner.xdp_has_frags = fp->aux->xdp_has_frags;
2282+
map->owner.xdp_has_frags = aux->xdp_has_frags;
2283+
map->owner.attach_func_proto = aux->attach_func_proto;
22822284
ret = true;
22832285
} else {
22842286
ret = map->owner.type == prog_type &&
22852287
map->owner.jited == fp->jited &&
2286-
map->owner.xdp_has_frags == fp->aux->xdp_has_frags;
2288+
map->owner.xdp_has_frags == aux->xdp_has_frags;
2289+
if (ret &&
2290+
map->owner.attach_func_proto != aux->attach_func_proto) {
2291+
switch (prog_type) {
2292+
case BPF_PROG_TYPE_TRACING:
2293+
case BPF_PROG_TYPE_LSM:
2294+
case BPF_PROG_TYPE_EXT:
2295+
case BPF_PROG_TYPE_STRUCT_OPS:
2296+
ret = false;
2297+
break;
2298+
default:
2299+
break;
2300+
}
2301+
}
22872302
}
22882303
spin_unlock(&map->owner.lock);
22892304

0 commit comments

Comments
 (0)