tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

libbpf: move global data mmap()'ing into bpf_object__load()

Since BPF skeleton inception libbpf has been doing mmap()'ing of global
data ARRAY maps in bpf_object__load_skeleton() API, which is used by
code generated .skel.h files (i.e., by BPF skeletons only).

This is wrong because if BPF object is loaded through generic
bpf_object__load() API, global data maps won't be re-mmap()'ed after
load step, and memory pointers returned from bpf_map__initial_value()
would be wrong and won't reflect the actual memory shared between BPF
program and user space.

bpf_map__initial_value() return result is rarely used after load, so
this went unnoticed for a really long time, until bpftrace project
attempted to load BPF object through generic bpf_object__load() API and
then used BPF subskeleton instantiated from such bpf_object. It turned
out that .data/.rodata/.bss data updates through such subskeleton was
"blackholed", all because libbpf wouldn't re-mmap() those maps during
bpf_object__load() phase.

Long story short, this step should be done by libbpf regardless of BPF
skeleton usage, right after BPF map is created in the kernel. This patch
moves this functionality into bpf_object__populate_internal_map() to
achieve this. And bpf_object__load_skeleton() is now simple and almost
trivial, only propagating these mmap()'ed pointers into user-supplied
skeleton structs.

We also do trivial adjustments to error reporting inside
bpf_object__populate_internal_map() for consistency with the rest of
libbpf's map-handling code.

Reported-by: Alastair Robertson <ajor@meta.com>
Reported-by: Jonathan Wiepert <jwiepert@meta.com>
Fixes: d66562fba1ce ("libbpf: Add BPF object skeleton support")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20241023043908.3834423-3-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

authored by

Andrii Nakryiko and committed by
Alexei Starovoitov 137978f4 1b2bfc29

+40 -43
+40 -43
tools/lib/bpf/libbpf.c
··· 5122 5122 enum libbpf_map_type map_type = map->libbpf_type; 5123 5123 char *cp, errmsg[STRERR_BUFSIZE]; 5124 5124 int err, zero = 0; 5125 + size_t mmap_sz; 5125 5126 5126 5127 if (obj->gen_loader) { 5127 5128 bpf_gen__map_update_elem(obj->gen_loader, map - obj->maps, ··· 5136 5135 if (err) { 5137 5136 err = -errno; 5138 5137 cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg)); 5139 - pr_warn("Error setting initial map(%s) contents: %s\n", 5140 - map->name, cp); 5138 + pr_warn("map '%s': failed to set initial contents: %s\n", 5139 + bpf_map__name(map), cp); 5141 5140 return err; 5142 5141 } 5143 5142 ··· 5147 5146 if (err) { 5148 5147 err = -errno; 5149 5148 cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg)); 5150 - pr_warn("Error freezing map(%s) as read-only: %s\n", 5151 - map->name, cp); 5149 + pr_warn("map '%s': failed to freeze as read-only: %s\n", 5150 + bpf_map__name(map), cp); 5152 5151 return err; 5153 5152 } 5154 5153 } 5154 + 5155 + /* Remap anonymous mmap()-ed "map initialization image" as 5156 + * a BPF map-backed mmap()-ed memory, but preserving the same 5157 + * memory address. This will cause kernel to change process' 5158 + * page table to point to a different piece of kernel memory, 5159 + * but from userspace point of view memory address (and its 5160 + * contents, being identical at this point) will stay the 5161 + * same. This mapping will be released by bpf_object__close() 5162 + * as per normal clean up procedure. 5163 + */ 5164 + mmap_sz = bpf_map_mmap_sz(map); 5165 + if (map->def.map_flags & BPF_F_MMAPABLE) { 5166 + void *mmaped; 5167 + int prot; 5168 + 5169 + if (map->def.map_flags & BPF_F_RDONLY_PROG) 5170 + prot = PROT_READ; 5171 + else 5172 + prot = PROT_READ | PROT_WRITE; 5173 + mmaped = mmap(map->mmaped, mmap_sz, prot, MAP_SHARED | MAP_FIXED, map->fd, 0); 5174 + if (mmaped == MAP_FAILED) { 5175 + err = -errno; 5176 + pr_warn("map '%s': failed to re-mmap() contents: %d\n", 5177 + bpf_map__name(map), err); 5178 + return err; 5179 + } 5180 + map->mmaped = mmaped; 5181 + } else if (map->mmaped) { 5182 + munmap(map->mmaped, mmap_sz); 5183 + map->mmaped = NULL; 5184 + } 5185 + 5155 5186 return 0; 5156 5187 } 5157 5188 ··· 5500 5467 err = bpf_object__populate_internal_map(obj, map); 5501 5468 if (err < 0) 5502 5469 goto err_out; 5503 - } 5504 - if (map->def.type == BPF_MAP_TYPE_ARENA) { 5470 + } else if (map->def.type == BPF_MAP_TYPE_ARENA) { 5505 5471 map->mmaped = mmap((void *)(long)map->map_extra, 5506 5472 bpf_map_mmap_sz(map), PROT_READ | PROT_WRITE, 5507 5473 map->map_extra ? MAP_SHARED | MAP_FIXED : MAP_SHARED, ··· 13948 13916 for (i = 0; i < s->map_cnt; i++) { 13949 13917 struct bpf_map_skeleton *map_skel = (void *)s->maps + i * s->map_skel_sz; 13950 13918 struct bpf_map *map = *map_skel->map; 13951 - size_t mmap_sz = bpf_map_mmap_sz(map); 13952 - int prot, map_fd = map->fd; 13953 - void **mmaped = map_skel->mmaped; 13954 13919 13955 - if (!mmaped) 13920 + if (!map_skel->mmaped) 13956 13921 continue; 13957 13922 13958 - if (!(map->def.map_flags & BPF_F_MMAPABLE)) { 13959 - *mmaped = NULL; 13960 - continue; 13961 - } 13962 - 13963 - if (map->def.type == BPF_MAP_TYPE_ARENA) { 13964 - *mmaped = map->mmaped; 13965 - continue; 13966 - } 13967 - 13968 - if (map->def.map_flags & BPF_F_RDONLY_PROG) 13969 - prot = PROT_READ; 13970 - else 13971 - prot = PROT_READ | PROT_WRITE; 13972 - 13973 - /* Remap anonymous mmap()-ed "map initialization image" as 13974 - * a BPF map-backed mmap()-ed memory, but preserving the same 13975 - * memory address. This will cause kernel to change process' 13976 - * page table to point to a different piece of kernel memory, 13977 - * but from userspace point of view memory address (and its 13978 - * contents, being identical at this point) will stay the 13979 - * same. This mapping will be released by bpf_object__close() 13980 - * as per normal clean up procedure, so we don't need to worry 13981 - * about it from skeleton's clean up perspective. 13982 - */ 13983 - *mmaped = mmap(map->mmaped, mmap_sz, prot, MAP_SHARED | MAP_FIXED, map_fd, 0); 13984 - if (*mmaped == MAP_FAILED) { 13985 - err = -errno; 13986 - *mmaped = NULL; 13987 - pr_warn("failed to re-mmap() map '%s': %d\n", 13988 - bpf_map__name(map), err); 13989 - return libbpf_err(err); 13990 - } 13923 + *map_skel->mmaped = map->mmaped; 13991 13924 } 13992 13925 13993 13926 return 0;