Skip to content

Commit 4540aed

Browse files
borkmannAlexei Starovoitov
authored andcommitted
bpf: Enforce expected_attach_type for tailcall compatibility
Yinhao et al. recently reported: Our fuzzer tool discovered an uninitialized pointer issue in the bpf_prog_test_run_xdp() function within the Linux kernel's BPF subsystem. This leads to a NULL pointer dereference when a BPF program attempts to deference the txq member of struct xdp_buff object. The test initializes two programs of BPF_PROG_TYPE_XDP: progA acts as the entry point for bpf_prog_test_run_xdp() and its expected_attach_type can neither be of be BPF_XDP_DEVMAP nor BPF_XDP_CPUMAP. progA calls into a slot of a tailcall map it owns. progB's expected_attach_type must be BPF_XDP_DEVMAP to pass xdp_is_valid_access() validation. The program returns struct xdp_md's egress_ifindex, and the latter is only allowed to be accessed under mentioned expected_attach_type. progB is then inserted into the tailcall which progA calls. The underlying issue goes beyond XDP though. Another example are programs of type BPF_PROG_TYPE_CGROUP_SOCK_ADDR. sock_addr_is_valid_access() as well as sock_addr_func_proto() have different logic depending on the programs' expected_attach_type. Similarly, a program attached to BPF_CGROUP_INET4_GETPEERNAME should not be allowed doing a tailcall into a program which calls bpf_bind() out of BPF which is only enabled for BPF_CGROUP_INET4_CONNECT. In short, specifying expected_attach_type allows to open up additional functionality or restrictions beyond what the basic bpf_prog_type enables. The use of tailcalls must not violate these constraints. Fix it by enforcing expected_attach_type in __bpf_prog_map_compatible(). Note that we only enforce this for tailcall maps, but not for BPF devmaps or cpumaps: There, the programs are invoked through dev_map_bpf_prog_run*() and cpu_map_bpf_prog_run*() which set up a new environment / context and therefore these situations are not prone to this issue. Fixes: 5e43f89 ("bpf: Check attach type at prog load time") Reported-by: Yinhao Hu <[email protected]> Reported-by: Kaiyan Mei <[email protected]> Reviewed-by: Dongliang Mu <[email protected]> Signed-off-by: Daniel Borkmann <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
1 parent 4b21134 commit 4540aed

File tree

2 files changed

+6
-0
lines changed

2 files changed

+6
-0
lines changed

include/linux/bpf.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@ struct bpf_map_owner {
289289
bool xdp_has_frags;
290290
u64 storage_cookie[MAX_BPF_CGROUP_STORAGE_TYPE];
291291
const struct btf_type *attach_func_proto;
292+
enum bpf_attach_type expected_attach_type;
292293
};
293294

294295
struct bpf_map {

kernel/bpf/core.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2361,6 +2361,7 @@ static bool __bpf_prog_map_compatible(struct bpf_map *map,
23612361
map->owner->type = prog_type;
23622362
map->owner->jited = fp->jited;
23632363
map->owner->xdp_has_frags = aux->xdp_has_frags;
2364+
map->owner->expected_attach_type = fp->expected_attach_type;
23642365
map->owner->attach_func_proto = aux->attach_func_proto;
23652366
for_each_cgroup_storage_type(i) {
23662367
map->owner->storage_cookie[i] =
@@ -2372,6 +2373,10 @@ static bool __bpf_prog_map_compatible(struct bpf_map *map,
23722373
ret = map->owner->type == prog_type &&
23732374
map->owner->jited == fp->jited &&
23742375
map->owner->xdp_has_frags == aux->xdp_has_frags;
2376+
if (ret &&
2377+
map->map_type == BPF_MAP_TYPE_PROG_ARRAY &&
2378+
map->owner->expected_attach_type != fp->expected_attach_type)
2379+
ret = false;
23752380
for_each_cgroup_storage_type(i) {
23762381
if (!ret)
23772382
break;

0 commit comments

Comments
 (0)