bpf: Enforce expected_attach_type for tailcall compatibility

[ Upstream commit 4540aed51b12bc13364149bf95f6ecef013197c0 ]

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: 5e43f899b03a ("bpf: Check attach type at prog load time")
Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/r/20250926171201.188490-1-daniel@iogearbox.net
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 5f01845627d495debcb8b591216db1c6110b7687..142a21f019ff890ab361fd133e5dc7ad8f304101 100644 (file)
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -228,6 +228,7 @@ struct bpf_map_owner {
        bool xdp_has_frags;
        u64 storage_cookie[MAX_BPF_CGROUP_STORAGE_TYPE];
        const struct btf_type *attach_func_proto;
+       enum bpf_attach_type expected_attach_type;
 };
 
 struct bpf_map {

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 3136af6559a821253b74934928157160548c1807..6924f86a8a3ff1092eb7e0d05686eb29a005ea99 100644 (file)
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2137,6 +2137,7 @@ bool bpf_prog_map_compatible(struct bpf_map *map,
                map->owner->type  = prog_type;
                map->owner->jited = fp->jited;
                map->owner->xdp_has_frags = aux->xdp_has_frags;
+               map->owner->expected_attach_type = fp->expected_attach_type;
                map->owner->attach_func_proto = aux->attach_func_proto;
                for_each_cgroup_storage_type(i) {
                        map->owner->storage_cookie[i] =
@@ -2148,6 +2149,10 @@ bool bpf_prog_map_compatible(struct bpf_map *map,
                ret = map->owner->type  == prog_type &&
                      map->owner->jited == fp->jited &&
                      map->owner->xdp_has_frags == aux->xdp_has_frags;
+               if (ret &&
+                   map->map_type == BPF_MAP_TYPE_PROG_ARRAY &&
+                   map->owner->expected_attach_type != fp->expected_attach_type)
+                       ret = false;
                for_each_cgroup_storage_type(i) {
                        if (!ret)
                                break;