On 9/13/23 3:26 PM, Song Liu wrote:
Current code charges modmem for regular trampoline, but not for struct_ops
trampoline. Add bpf_jit_[charge|uncharge]_modmem() to struct_ops so the
trampoline is charged in both cases.
Signed-off-by: Song Liu <song@xxxxxxxxxx>
---
kernel/bpf/bpf_struct_ops.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index fdc3e8705a3c..ea6ca87a2ed9 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -615,7 +615,10 @@ static void __bpf_struct_ops_map_free(struct bpf_map *map)
if (st_map->links)
bpf_struct_ops_map_put_progs(st_map);
bpf_map_area_free(st_map->links);
- bpf_jit_free_exec(st_map->image);
+ if (st_map->image) {
+ bpf_jit_free_exec(st_map->image);
+ bpf_jit_uncharge_modmem(PAGE_SIZE);
+ }
bpf_map_area_free(st_map->uvalue);
bpf_map_area_free(st_map);
}
@@ -657,6 +660,7 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
struct bpf_struct_ops_map *st_map;
const struct btf_type *t, *vt;
struct bpf_map *map;
+ int ret;
st_ops = bpf_struct_ops_find_value(attr->btf_vmlinux_value_type_id);
if (!st_ops)
@@ -681,6 +685,12 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
st_map->st_ops = st_ops;
map = &st_map->map;
+ ret = bpf_jit_charge_modmem(PAGE_SIZE);
+ if (ret) {
+ __bpf_struct_ops_map_free(map);
+ return ERR_PTR(ret);
+ }
This just came to my mind when reading it again.
It will miss a bpf_jit_uncharge_modmem() if the bpf_jit_alloc_exec() at a few
lines below did fail (meaning st_map->image is NULL). It is because the
__bpf_struct_ops_map_free() only uncharge if st_map->image is not NULL.
How above moving the bpf_jit_alloc_exec() to here (immediately after
bpf_jit_charge_modem succeeded). Like,
st_map->image = bpf_jit_alloc_exec(PAGE_SIZE);
if (!st_map->image) {
bpf_jit_uncharge_modmem(PAGE_SIZE);
__bpf_struct_ops_map_free(map);
return ERR_PTR(-ENOMEM);
}
Then there is also no need to test 'if (st_map->image)' in
__bpf_struct_ops_map_free().
+
st_map->uvalue = bpf_map_area_alloc(vt->size, NUMA_NO_NODE);
st_map->links =
bpf_map_area_alloc(btf_type_vlen(t) * sizeof(struct bpf_links *),
@@ -907,4 +917,3 @@ int bpf_struct_ops_link_create(union bpf_attr *attr)
kfree(link);
return err;
}
-
