Commit 04a94133 authored by David Vernet's avatar David Vernet Committed by Andrii Nakryiko

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: 590a0088 ("bpf: libbpf: Add STRUCT_OPS support")
Signed-off-by: default avatarDavid Vernet <void@manifault.com>
Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/bpf/20240724171459.281234-1-void@manifault.com
parent 8efffab4
...@@ -496,8 +496,6 @@ struct bpf_program { ...@@ -496,8 +496,6 @@ struct bpf_program {
}; };
struct bpf_struct_ops { struct bpf_struct_ops {
const char *tname;
const struct btf_type *type;
struct bpf_program **progs; struct bpf_program **progs;
__u32 *kern_func_off; __u32 *kern_func_off;
/* e.g. struct tcp_congestion_ops in bpf_prog's btf format */ /* e.g. struct tcp_congestion_ops in bpf_prog's btf format */
...@@ -1083,11 +1081,14 @@ static int bpf_object_adjust_struct_ops_autoload(struct bpf_object *obj) ...@@ -1083,11 +1081,14 @@ static int bpf_object_adjust_struct_ops_autoload(struct bpf_object *obj)
continue; continue;
for (j = 0; j < obj->nr_maps; ++j) { for (j = 0; j < obj->nr_maps; ++j) {
const struct btf_type *type;
map = &obj->maps[j]; map = &obj->maps[j];
if (!bpf_map__is_struct_ops(map)) if (!bpf_map__is_struct_ops(map))
continue; continue;
vlen = btf_vlen(map->st_ops->type); type = btf__type_by_id(obj->btf, map->st_ops->type_id);
vlen = btf_vlen(type);
for (k = 0; k < vlen; ++k) { for (k = 0; k < vlen; ++k) {
slot_prog = map->st_ops->progs[k]; slot_prog = map->st_ops->progs[k];
if (prog != slot_prog) if (prog != slot_prog)
...@@ -1121,8 +1122,8 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map) ...@@ -1121,8 +1122,8 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map)
int err; int err;
st_ops = map->st_ops; st_ops = map->st_ops;
type = st_ops->type; type = btf__type_by_id(btf, st_ops->type_id);
tname = st_ops->tname; tname = btf__name_by_offset(btf, type->name_off);
err = find_struct_ops_kern_types(obj, tname, &mod_btf, err = find_struct_ops_kern_types(obj, tname, &mod_btf,
&kern_type, &kern_type_id, &kern_type, &kern_type_id,
&kern_vtype, &kern_vtype_id, &kern_vtype, &kern_vtype_id,
...@@ -1423,8 +1424,6 @@ static int init_struct_ops_maps(struct bpf_object *obj, const char *sec_name, ...@@ -1423,8 +1424,6 @@ static int init_struct_ops_maps(struct bpf_object *obj, const char *sec_name,
memcpy(st_ops->data, memcpy(st_ops->data,
data->d_buf + vsi->offset, data->d_buf + vsi->offset,
type->size); type->size);
st_ops->tname = tname;
st_ops->type = type;
st_ops->type_id = type_id; st_ops->type_id = type_id;
pr_debug("struct_ops init: struct %s(type_id=%u) %s found at offset %u\n", pr_debug("struct_ops init: struct %s(type_id=%u) %s found at offset %u\n",
...@@ -8445,11 +8444,13 @@ static int bpf_object__resolve_externs(struct bpf_object *obj, ...@@ -8445,11 +8444,13 @@ static int bpf_object__resolve_externs(struct bpf_object *obj,
static void bpf_map_prepare_vdata(const struct bpf_map *map) static void bpf_map_prepare_vdata(const struct bpf_map *map)
{ {
const struct btf_type *type;
struct bpf_struct_ops *st_ops; struct bpf_struct_ops *st_ops;
__u32 i; __u32 i;
st_ops = map->st_ops; 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]; struct bpf_program *prog = st_ops->progs[i];
void *kern_data; void *kern_data;
int prog_fd; int prog_fd;
...@@ -9712,6 +9713,7 @@ static struct bpf_map *find_struct_ops_map_by_offset(struct bpf_object *obj, ...@@ -9712,6 +9713,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, static int bpf_object__collect_st_ops_relos(struct bpf_object *obj,
Elf64_Shdr *shdr, Elf_Data *data) Elf64_Shdr *shdr, Elf_Data *data)
{ {
const struct btf_type *type;
const struct btf_member *member; const struct btf_member *member;
struct bpf_struct_ops *st_ops; struct bpf_struct_ops *st_ops;
struct bpf_program *prog; struct bpf_program *prog;
...@@ -9771,13 +9773,14 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj, ...@@ -9771,13 +9773,14 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj,
} }
insn_idx = sym->st_value / BPF_INSN_SZ; 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) { if (!member) {
pr_warn("struct_ops reloc %s: cannot find member at moff %u\n", pr_warn("struct_ops reloc %s: cannot find member at moff %u\n",
map->name, moff); map->name, moff);
return -EINVAL; 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); name = btf__name_by_offset(btf, member->name_off);
if (!resolve_func_ptr(btf, member->type, NULL)) { if (!resolve_func_ptr(btf, member->type, NULL)) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment