Commit 8b8d9a3
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]>1 parent 77743ac commit 8b8d9a3
2 files changed
+6
-0
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
290 | 290 | | |
291 | 291 | | |
292 | 292 | | |
| 293 | + | |
293 | 294 | | |
294 | 295 | | |
295 | 296 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2361 | 2361 | | |
2362 | 2362 | | |
2363 | 2363 | | |
| 2364 | + | |
2364 | 2365 | | |
2365 | 2366 | | |
2366 | 2367 | | |
| |||
2372 | 2373 | | |
2373 | 2374 | | |
2374 | 2375 | | |
| 2376 | + | |
| 2377 | + | |
| 2378 | + | |
| 2379 | + | |
2375 | 2380 | | |
2376 | 2381 | | |
2377 | 2382 | | |
| |||
0 commit comments