From 7b5237996a42c3b8a6fe8ccae656047de2831f58 Mon Sep 17 00:00:00 2001 From: David Vernet Date: Wed, 24 Jul 2024 12:14:58 -0500 Subject: [PATCH] libbpf: Don't take direct pointers into BTF data from st_ops In struct bpf_struct_ops, we have take a pointer to a BTF type name, and a struct btf_type. This was presumably done for convenience, but can actually result in subtle and confusing bugs given that BTF data can be invalidated before a program is loaded. For example, in sched_ext, we may sometimes resize a data section after a skeleton has been opened, but before the struct_ops scheduler map has been loaded. This may cause the BTF data to be realloc'd, which can then cause a UAF when loading the program because the struct_ops map has pointers directly into the BTF data. We're already storing the BTF type_id in struct bpf_struct_ops. Because type_id is stable, we can therefore just update the places where we were looking at those pointers to instead do the lookups we need from the type_id. Fixes: 590a00888250 ("bpf: libbpf: Add STRUCT_OPS support") Signed-off-by: David Vernet Signed-off-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20240724171459.281234-1-void@manifault.com Conflict:parts of functions do not exist in v0.8.1 Reference:https://github.com/libbpf/libbpf/commit/7b5237996a42c3b8a6fe8ccae656047de2831f58 --- src/libbpf.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libbpf.c b/src/libbpf.c index 5eb2452..25a91ba 100644 --- a/src/libbpf.c +++ b/src/libbpf.c @@ -327,8 +327,6 @@ struct bpf_program { }; struct bpf_struct_ops { - const char *tname; - const struct btf_type *type; struct bpf_program **progs; __u32 *kern_func_off; /* e.g. struct tcp_congestion_ops in bpf_prog's btf format */ @@ -955,8 +953,8 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map, int err; st_ops = map->st_ops; - type = st_ops->type; - tname = st_ops->tname; + type = btf__type_by_id(btf, st_ops->type_id); + tname = btf__name_by_offset(btf, type->name_off); err = find_struct_ops_kern_types(kern_btf, tname, &kern_type, &kern_type_id, &kern_vtype, &kern_vtype_id, @@ -1175,8 +1173,6 @@ static int bpf_object__init_struct_ops_maps(struct bpf_object *obj) memcpy(st_ops->data, obj->efile.st_ops_data->d_buf + vsi->offset, type->size); - st_ops->tname = tname; - st_ops->type = type; st_ops->type_id = type_id; pr_debug("struct_ops init: struct %s(type_id=%u) %s found at offset %u\n", @@ -9361,6 +9357,7 @@ static struct bpf_map *find_struct_ops_map_by_offset(struct bpf_object *obj, static int bpf_object__collect_st_ops_relos(struct bpf_object *obj, Elf64_Shdr *shdr, Elf_Data *data) { + const struct btf_type *type; const struct btf_member *member; struct bpf_struct_ops *st_ops; struct bpf_program *prog; @@ -9420,13 +9417,14 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj, } insn_idx = sym->st_value / BPF_INSN_SZ; - member = find_member_by_offset(st_ops->type, moff * 8); + type = btf__type_by_id(btf, st_ops->type_id); + member = find_member_by_offset(type, moff * 8); if (!member) { pr_warn("struct_ops reloc %s: cannot find member at moff %u\n", map->name, moff); return -EINVAL; } - member_idx = member - btf_members(st_ops->type); + member_idx = member - btf_members(type); name = btf__name_by_offset(btf, member->name_off); if (!resolve_func_ptr(btf, member->type, NULL)) { @@ -11967,6 +11965,7 @@ static int bpf_link__detach_struct_ops(struct bpf_link *link) struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map) { + const struct btf_type *type; struct bpf_struct_ops *st_ops; struct bpf_link *link; __u32 i, zero = 0; @@ -11980,7 +11979,8 @@ struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map) return libbpf_err_ptr(-EINVAL); st_ops = map->st_ops; - for (i = 0; i < btf_vlen(st_ops->type); i++) { + type = btf__type_by_id(map->obj->btf, st_ops->type_id); + for (i = 0; i < btf_vlen(type); i++) { struct bpf_program *prog = st_ops->progs[i]; void *kern_data; int prog_fd; -- 2.33.0