[PATCH] libbpf: bounds-check struct_ops member offset before writing shadow pointer

[Naveed Khan <naveed@digiscrypt.com>]

bpf_object__collect_st_ops_relos() replaces each function pointer in a
struct_ops value with a pointer to the corresponding struct bpf_program
in the map's shadow data:

	*((struct bpf_program **)(st_ops->data + moff)) = prog;

st_ops->data is allocated with malloc(type->size), so it is exactly
map->def.value_size bytes long, and moff is the byte offset of the
member being relocated. The only bound placed on moff comes from
find_struct_ops_map_by_offset():

	offset - map->sec_offset < map->def.value_size

so moff can be as large as value_size - 1. The store above is
sizeof(struct bpf_program *) bytes wide, and nothing checks that
moff + sizeof(struct bpf_program *) stays within value_size.

libbpf's BTF sanity check does not validate that a struct member's
offset plus its size fits within the struct's declared size (see
btf_validate_type()), and this relocation runs at bpf_object__open()
time, before the kernel validates the struct_ops type. A crafted object
with a struct_ops struct whose declared size is small (e.g. 1) but which
contains a function-pointer member near the end of (or beyond) that size
therefore makes libbpf write up to sizeof(void *) - 1 bytes past the heap
allocation - a controlled heap out-of-bounds write reachable purely from
opening an untrusted object file.

Reject such relocations by requiring the whole pointer store to fit
within the map value.

Signed-off-by: Naveed Khan <naveed@digiscrypt.com>
---
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 1368752aa1..1aa2a1d098 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10529,6 +10529,17 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj,
 			return -EINVAL;
 		}
 
+		/* the shadow pointer stored below is sizeof(struct bpf_program *)
+		 * bytes wide, so the whole write must fit within st_ops->data,
+		 * which is only map->def.value_size bytes long. A malformed BTF
+		 * can place a member near the end of the value and overflow it.
+		 */
+		if (moff + sizeof(struct bpf_program *) > map->def.value_size) {
+			pr_warn("struct_ops reloc %s: member %s at moff %u overflows map value size %u\n",
+				map->name, name, moff, map->def.value_size);
+			return -EINVAL;
+		}
+
 		prog = find_prog_by_sec_insn(obj, shdr_idx, insn_idx);
 		if (!prog) {
 			pr_warn("struct_ops reloc %s: cannot find prog at shdr_idx %u to relocate func ptr %s\n",
-- 
2.52.0