* [PATCH bpf-next 1/8] s390/bpf: Let arch_prepare_bpf_trampoline return program size
2023-09-20 5:31 [PATCH bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
@ 2023-09-20 5:31 ` Song Liu
2023-09-20 5:31 ` [PATCH bpf-next 2/8] bpf: Let bpf_prog_pack_free handle any pointer Song Liu
` (6 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Song Liu @ 2023-09-20 5:31 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, kernel-team, Song Liu,
Ilya Leoshkevich
arch_prepare_bpf_trampoline() for s390 currently returns 0 on success. This
is not a problem for regular trampoline. However, struct_ops relies on the
return value to advance "image" pointer:
bpf_struct_ops_map_update_elem() {
...
for_each_member(i, t, member) {
...
err = bpf_struct_ops_prepare_trampoline();
...
image += err;
}
}
When arch_prepare_bpf_trampoline returns 0 on success, all members of the
struct_ops will point to the same trampoline (the last one).
Fix this by returning the program size in arch_prepare_bpf_trampoline (on
success). This is the same behavior as other architectures.
Signed-off-by: Song Liu <song@kernel.org>
Fixes: 528eb2cb87bc ("s390/bpf: Implement arch_prepare_bpf_trampoline()")
Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
Link: https://lore.kernel.org/r/20230919060258.3237176-2-song@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
arch/s390/net/bpf_jit_comp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index eeb42e5cd7d6..e6a643f63ebf 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -2513,7 +2513,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
return -E2BIG;
}
- return ret;
+ return tjit.common.prg;
}
bool bpf_jit_supports_subprog_tailcalls(void)
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH bpf-next 2/8] bpf: Let bpf_prog_pack_free handle any pointer
2023-09-20 5:31 [PATCH bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
2023-09-20 5:31 ` [PATCH bpf-next 1/8] s390/bpf: Let arch_prepare_bpf_trampoline return program size Song Liu
@ 2023-09-20 5:31 ` Song Liu
2023-09-20 5:31 ` [PATCH bpf-next 3/8] bpf: Adjust argument names of arch_prepare_bpf_trampoline() Song Liu
` (5 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Song Liu @ 2023-09-20 5:31 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, Song Liu
Currently, bpf_prog_pack_free only can only free pointer to struct
bpf_binary_header, which is not flexible. Add a size argument to
bpf_prog_pack_free so that it can handle any pointer.
Signed-off-by: Song Liu <song@kernel.org>
---
include/linux/filter.h | 2 +-
kernel/bpf/core.c | 21 ++++++++++-----------
kernel/bpf/dispatcher.c | 5 +----
3 files changed, 12 insertions(+), 16 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 27406aee2d40..eda9efe20026 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1073,7 +1073,7 @@ struct bpf_binary_header *
bpf_jit_binary_pack_hdr(const struct bpf_prog *fp);
void *bpf_prog_pack_alloc(u32 size, bpf_jit_fill_hole_t bpf_fill_ill_insns);
-void bpf_prog_pack_free(struct bpf_binary_header *hdr);
+void bpf_prog_pack_free(void *ptr, u32 size);
static inline bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
{
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 08626b519ce2..fcdf710e6a32 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -928,20 +928,20 @@ void *bpf_prog_pack_alloc(u32 size, bpf_jit_fill_hole_t bpf_fill_ill_insns)
return ptr;
}
-void bpf_prog_pack_free(struct bpf_binary_header *hdr)
+void bpf_prog_pack_free(void *ptr, u32 size)
{
struct bpf_prog_pack *pack = NULL, *tmp;
unsigned int nbits;
unsigned long pos;
mutex_lock(&pack_mutex);
- if (hdr->size > BPF_PROG_PACK_SIZE) {
- bpf_jit_free_exec(hdr);
+ if (size > BPF_PROG_PACK_SIZE) {
+ bpf_jit_free_exec(ptr);
goto out;
}
list_for_each_entry(tmp, &pack_list, list) {
- if ((void *)hdr >= tmp->ptr && (tmp->ptr + BPF_PROG_PACK_SIZE) > (void *)hdr) {
+ if (ptr >= tmp->ptr && (tmp->ptr + BPF_PROG_PACK_SIZE) > ptr) {
pack = tmp;
break;
}
@@ -950,10 +950,10 @@ void bpf_prog_pack_free(struct bpf_binary_header *hdr)
if (WARN_ONCE(!pack, "bpf_prog_pack bug\n"))
goto out;
- nbits = BPF_PROG_SIZE_TO_NBITS(hdr->size);
- pos = ((unsigned long)hdr - (unsigned long)pack->ptr) >> BPF_PROG_CHUNK_SHIFT;
+ nbits = BPF_PROG_SIZE_TO_NBITS(size);
+ pos = ((unsigned long)ptr - (unsigned long)pack->ptr) >> BPF_PROG_CHUNK_SHIFT;
- WARN_ONCE(bpf_arch_text_invalidate(hdr, hdr->size),
+ WARN_ONCE(bpf_arch_text_invalidate(ptr, size),
"bpf_prog_pack bug: missing bpf_arch_text_invalidate?\n");
bitmap_clear(pack->bitmap, pos, nbits);
@@ -1100,8 +1100,7 @@ bpf_jit_binary_pack_alloc(unsigned int proglen, u8 **image_ptr,
*rw_header = kvmalloc(size, GFP_KERNEL);
if (!*rw_header) {
- bpf_arch_text_copy(&ro_header->size, &size, sizeof(size));
- bpf_prog_pack_free(ro_header);
+ bpf_prog_pack_free(ro_header, size);
bpf_jit_uncharge_modmem(size);
return NULL;
}
@@ -1132,7 +1131,7 @@ int bpf_jit_binary_pack_finalize(struct bpf_prog *prog,
kvfree(rw_header);
if (IS_ERR(ptr)) {
- bpf_prog_pack_free(ro_header);
+ bpf_prog_pack_free(ro_header, ro_header->size);
return PTR_ERR(ptr);
}
return 0;
@@ -1153,7 +1152,7 @@ void bpf_jit_binary_pack_free(struct bpf_binary_header *ro_header,
{
u32 size = ro_header->size;
- bpf_prog_pack_free(ro_header);
+ bpf_prog_pack_free(ro_header, size);
kvfree(rw_header);
bpf_jit_uncharge_modmem(size);
}
diff --git a/kernel/bpf/dispatcher.c b/kernel/bpf/dispatcher.c
index fa3e9225aedc..56760fc10e78 100644
--- a/kernel/bpf/dispatcher.c
+++ b/kernel/bpf/dispatcher.c
@@ -150,10 +150,7 @@ void bpf_dispatcher_change_prog(struct bpf_dispatcher *d, struct bpf_prog *from,
goto out;
d->rw_image = bpf_jit_alloc_exec(PAGE_SIZE);
if (!d->rw_image) {
- u32 size = PAGE_SIZE;
-
- bpf_arch_text_copy(d->image, &size, sizeof(size));
- bpf_prog_pack_free((struct bpf_binary_header *)d->image);
+ bpf_prog_pack_free(d->image, PAGE_SIZE);
d->image = NULL;
goto out;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH bpf-next 3/8] bpf: Adjust argument names of arch_prepare_bpf_trampoline()
2023-09-20 5:31 [PATCH bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
2023-09-20 5:31 ` [PATCH bpf-next 1/8] s390/bpf: Let arch_prepare_bpf_trampoline return program size Song Liu
2023-09-20 5:31 ` [PATCH bpf-next 2/8] bpf: Let bpf_prog_pack_free handle any pointer Song Liu
@ 2023-09-20 5:31 ` Song Liu
2023-09-20 5:31 ` [PATCH bpf-next 4/8] bpf: Add helpers for trampoline image management Song Liu
` (4 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Song Liu @ 2023-09-20 5:31 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, Song Liu
We are using "im" for "struct bpf_tramp_image" and "tr" for "struct
bpf_trampoline" in most of the code base. The only exception is the
prototype and fallback version of arch_prepare_bpf_trampoline(). Update
them to match the rest of the code base.
We mix "orig_call" and "func_addr" for the argument in different versions
of arch_prepare_bpf_trampoline(). s/orig_call/func_addr/g so they match.
Signed-off-by: Song Liu <song@kernel.org>
---
arch/arm64/net/bpf_jit_comp.c | 10 +++++-----
include/linux/bpf.h | 4 ++--
kernel/bpf/trampoline.c | 4 ++--
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 7d4af64e3982..d81b886ea4df 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -1828,7 +1828,7 @@ static void restore_args(struct jit_ctx *ctx, int args_off, int nregs)
*
*/
static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
- struct bpf_tramp_links *tlinks, void *orig_call,
+ struct bpf_tramp_links *tlinks, void *func_addr,
int nregs, u32 flags)
{
int i;
@@ -1926,7 +1926,7 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
if (flags & BPF_TRAMP_F_IP_ARG) {
/* save ip address of the traced function */
- emit_addr_mov_i64(A64_R(10), (const u64)orig_call, ctx);
+ emit_addr_mov_i64(A64_R(10), (const u64)func_addr, ctx);
emit(A64_STR64I(A64_R(10), A64_SP, ip_off), ctx);
}
@@ -2029,7 +2029,7 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
void *image_end, const struct btf_func_model *m,
u32 flags, struct bpf_tramp_links *tlinks,
- void *orig_call)
+ void *func_addr)
{
int i, ret;
int nregs = m->nr_args;
@@ -2050,7 +2050,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
if (nregs > 8)
return -ENOTSUPP;
- ret = prepare_trampoline(&ctx, im, tlinks, orig_call, nregs, flags);
+ ret = prepare_trampoline(&ctx, im, tlinks, func_addr, nregs, flags);
if (ret < 0)
return ret;
@@ -2061,7 +2061,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
ctx.idx = 0;
jit_fill_hole(image, (unsigned int)(image_end - image));
- ret = prepare_trampoline(&ctx, im, tlinks, orig_call, nregs, flags);
+ ret = prepare_trampoline(&ctx, im, tlinks, func_addr, nregs, flags);
if (ret > 0 && validate_code(&ctx) < 0)
ret = -EINVAL;
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 30063a760b5a..27a18c0c10ca 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1079,10 +1079,10 @@ struct bpf_tramp_run_ctx;
* fexit = a set of program to run after original function
*/
struct bpf_tramp_image;
-int arch_prepare_bpf_trampoline(struct bpf_tramp_image *tr, void *image, void *image_end,
+int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
const struct btf_func_model *m, u32 flags,
struct bpf_tramp_links *tlinks,
- void *orig_call);
+ void *func_addr);
u64 notrace __bpf_prog_enter_sleepable_recur(struct bpf_prog *prog,
struct bpf_tramp_run_ctx *run_ctx);
void notrace __bpf_prog_exit_sleepable_recur(struct bpf_prog *prog, u64 start,
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index e97aeda3a86b..e114a1c7961e 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -1032,10 +1032,10 @@ bpf_trampoline_exit_t bpf_trampoline_exit(const struct bpf_prog *prog)
}
int __weak
-arch_prepare_bpf_trampoline(struct bpf_tramp_image *tr, void *image, void *image_end,
+arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
const struct btf_func_model *m, u32 flags,
struct bpf_tramp_links *tlinks,
- void *orig_call)
+ void *func_addr)
{
return -ENOTSUPP;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH bpf-next 4/8] bpf: Add helpers for trampoline image management
2023-09-20 5:31 [PATCH bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
` (2 preceding siblings ...)
2023-09-20 5:31 ` [PATCH bpf-next 3/8] bpf: Adjust argument names of arch_prepare_bpf_trampoline() Song Liu
@ 2023-09-20 5:31 ` Song Liu
2023-09-20 16:31 ` Song Liu
2023-09-20 5:31 ` [PATCH bpf-next 5/8] bpf, x86: Adjust arch_prepare_bpf_trampoline return value Song Liu
` (3 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Song Liu @ 2023-09-20 5:31 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, Song Liu
As BPF trampoline of different archs moves from bpf_jit_[alloc|free]_exec()
to bpf_prog_pack_[alloc|free](), we need to use different _alloc, _free for
different archs during the transition. Add the following helpers for this
transition:
void *arch_alloc_bpf_trampoline(int size);
void arch_free_bpf_trampoline(void *image, int size);
void arch_protect_bpf_trampoline(void *image, int size);
void arch_unprotect_bpf_trampoline(void *image, int size);
The fallback version of these helpers require size <= PAGE_SIZE, but they
are only called with size == PAGE_SIZE. They will be called with size <
PAGE_SIZE when arch_bpf_trampoline_size() helper is introduced later.
Signed-off-by: Song Liu <song@kernel.org>
---
include/linux/bpf.h | 5 +++++
kernel/bpf/bpf_struct_ops.c | 12 +++++-----
kernel/bpf/trampoline.c | 44 +++++++++++++++++++++++++++++++------
3 files changed, 47 insertions(+), 14 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 27a18c0c10ca..548f3ef34ba1 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1083,6 +1083,11 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
const struct btf_func_model *m, u32 flags,
struct bpf_tramp_links *tlinks,
void *func_addr);
+void *arch_alloc_bpf_trampoline(int size);
+void arch_free_bpf_trampoline(void *image, int size);
+void arch_protect_bpf_trampoline(void *image, int size);
+void arch_unprotect_bpf_trampoline(void *image, int size);
+
u64 notrace __bpf_prog_enter_sleepable_recur(struct bpf_prog *prog,
struct bpf_tramp_run_ctx *run_ctx);
void notrace __bpf_prog_exit_sleepable_recur(struct bpf_prog *prog, u64 start,
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index db6176fb64dc..e9e95879bce2 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -515,7 +515,7 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
if (err)
goto reset_unlock;
}
- set_memory_rox((long)st_map->image, 1);
+ arch_protect_bpf_trampoline(st_map->image, PAGE_SIZE);
/* Let bpf_link handle registration & unregistration.
*
* Pair with smp_load_acquire() during lookup_elem().
@@ -524,7 +524,7 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
goto unlock;
}
- set_memory_rox((long)st_map->image, 1);
+ arch_protect_bpf_trampoline(st_map->image, PAGE_SIZE);
err = st_ops->reg(kdata);
if (likely(!err)) {
/* This refcnt increment on the map here after
@@ -547,8 +547,7 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
* there was a race in registering the struct_ops (under the same name) to
* a sub-system through different struct_ops's maps.
*/
- set_memory_nx((long)st_map->image, 1);
- set_memory_rw((long)st_map->image, 1);
+ arch_unprotect_bpf_trampoline(st_map->image, PAGE_SIZE);
reset_unlock:
bpf_struct_ops_map_put_progs(st_map);
@@ -616,7 +615,7 @@ static void __bpf_struct_ops_map_free(struct bpf_map *map)
bpf_struct_ops_map_put_progs(st_map);
bpf_map_area_free(st_map->links);
if (st_map->image) {
- bpf_jit_free_exec(st_map->image);
+ arch_free_bpf_trampoline(st_map->image, PAGE_SIZE);
bpf_jit_uncharge_modmem(PAGE_SIZE);
}
bpf_map_area_free(st_map->uvalue);
@@ -691,7 +690,7 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
return ERR_PTR(ret);
}
- st_map->image = bpf_jit_alloc_exec(PAGE_SIZE);
+ st_map->image = arch_alloc_bpf_trampoline(PAGE_SIZE);
if (!st_map->image) {
/* __bpf_struct_ops_map_free() uses st_map->image as flag
* for "charged or not". In this case, we need to unchange
@@ -711,7 +710,6 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
}
mutex_init(&st_map->lock);
- set_vm_flush_reset_perms(st_map->image);
bpf_map_init_from_attr(map, attr);
return map;
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index e114a1c7961e..5509bdf98067 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -254,7 +254,7 @@ bpf_trampoline_get_progs(const struct bpf_trampoline *tr, int *total, bool *ip_a
static void bpf_tramp_image_free(struct bpf_tramp_image *im)
{
bpf_image_ksym_del(&im->ksym);
- bpf_jit_free_exec(im->image);
+ arch_free_bpf_trampoline(im->image, PAGE_SIZE);
bpf_jit_uncharge_modmem(PAGE_SIZE);
percpu_ref_exit(&im->pcref);
kfree_rcu(im, rcu);
@@ -365,10 +365,9 @@ static struct bpf_tramp_image *bpf_tramp_image_alloc(u64 key)
goto out_free_im;
err = -ENOMEM;
- im->image = image = bpf_jit_alloc_exec(PAGE_SIZE);
+ im->image = image = arch_alloc_bpf_trampoline(PAGE_SIZE);
if (!image)
goto out_uncharge;
- set_vm_flush_reset_perms(image);
err = percpu_ref_init(&im->pcref, __bpf_tramp_image_release, 0, GFP_KERNEL);
if (err)
@@ -381,7 +380,7 @@ static struct bpf_tramp_image *bpf_tramp_image_alloc(u64 key)
return im;
out_free_image:
- bpf_jit_free_exec(im->image);
+ arch_free_bpf_trampoline(im->image, PAGE_SIZE);
out_uncharge:
bpf_jit_uncharge_modmem(PAGE_SIZE);
out_free_im:
@@ -444,7 +443,7 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
if (err < 0)
goto out_free;
- set_memory_rox((long)im->image, 1);
+ arch_protect_bpf_trampoline(im->image, PAGE_SIZE);
WARN_ON(tr->cur_image && total == 0);
if (tr->cur_image)
@@ -465,8 +464,7 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
tr->fops->trampoline = 0;
/* reset im->image memory attr for arch_prepare_bpf_trampoline */
- set_memory_nx((long)im->image, 1);
- set_memory_rw((long)im->image, 1);
+ arch_unprotect_bpf_trampoline(im->image, PAGE_SIZE);
goto again;
}
#endif
@@ -1040,6 +1038,38 @@ arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image
return -ENOTSUPP;
}
+void * __weak arch_alloc_bpf_trampoline(int size)
+{
+ void *image;
+
+ WARN_ON_ONCE(size > PAGE_SIZE || size <= 0);
+ image = bpf_jit_alloc_exec(PAGE_SIZE);
+ if (image)
+ set_vm_flush_reset_perms(image);
+ return image;
+}
+
+void __weak arch_free_bpf_trampoline(void *image, int size)
+{
+ /* bpf_jit_free_exec doesn't need "size", but
+ * bpf_prog_pack_free() needs it.
+ */
+ bpf_jit_free_exec(image);
+}
+
+void __weak arch_protect_bpf_trampoline(void *image, int size)
+{
+ WARN_ON_ONCE(size > PAGE_SIZE || size <= 0);
+ set_memory_rox((long)image, 1);
+}
+
+void __weak arch_unprotect_bpf_trampoline(void *image, int size)
+{
+ WARN_ON_ONCE(size > PAGE_SIZE || size <= 0);
+ set_memory_nx((long)image, 1);
+ set_memory_rw((long)image, 1);
+}
+
static int __init init_trampolines(void)
{
int i;
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH bpf-next 4/8] bpf: Add helpers for trampoline image management
2023-09-20 5:31 ` [PATCH bpf-next 4/8] bpf: Add helpers for trampoline image management Song Liu
@ 2023-09-20 16:31 ` Song Liu
0 siblings, 0 replies; 17+ messages in thread
From: Song Liu @ 2023-09-20 16:31 UTC (permalink / raw)
To: Song Liu
Cc: bpf, ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
martin.lau@kernel.org, Kernel Team
> On Sep 19, 2023, at 10:31 PM, Song Liu <song@kernel.org> wrote:
>
> As BPF trampoline of different archs moves from bpf_jit_[alloc|free]_exec()
> to bpf_prog_pack_[alloc|free](), we need to use different _alloc, _free for
> different archs during the transition. Add the following helpers for this
> transition:
>
> void *arch_alloc_bpf_trampoline(int size);
> void arch_free_bpf_trampoline(void *image, int size);
> void arch_protect_bpf_trampoline(void *image, int size);
> void arch_unprotect_bpf_trampoline(void *image, int size);
>
> The fallback version of these helpers require size <= PAGE_SIZE, but they
> are only called with size == PAGE_SIZE. They will be called with size <
> PAGE_SIZE when arch_bpf_trampoline_size() helper is introduced later.
>
> Signed-off-by: Song Liu <song@kernel.org>
> ---
> include/linux/bpf.h | 5 +++++
> kernel/bpf/bpf_struct_ops.c | 12 +++++-----
I just found that we need similar changes in net/bpf/bpf_dummy_struct_ops.c.
Added it to the next version.
Song
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH bpf-next 5/8] bpf, x86: Adjust arch_prepare_bpf_trampoline return value
2023-09-20 5:31 [PATCH bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
` (3 preceding siblings ...)
2023-09-20 5:31 ` [PATCH bpf-next 4/8] bpf: Add helpers for trampoline image management Song Liu
@ 2023-09-20 5:31 ` Song Liu
2023-09-20 5:31 ` [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size() Song Liu
` (2 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Song Liu @ 2023-09-20 5:31 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, Song Liu
x86's implementation of arch_prepare_bpf_trampoline() requires
BPF_INSN_SAFETY buffer space between end of program and image_end. OTOH,
the return value does not include BPF_INSN_SAFETY. This doesn't cause any
real issue at the moment. However, "image" of size retval is not enough for
arch_prepare_bpf_trampoline(). This will cause confusion when we introduce
a new helper arch_bpf_trampoline_size(). To avoid future confusion, adjust
the return value to include BPF_INSN_SAFETY.
Signed-off-by: Song Liu <song@kernel.org>
---
arch/x86/net/bpf_jit_comp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 8c10d9abc239..5f7528cac344 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -2671,7 +2671,7 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
ret = -EFAULT;
goto cleanup;
}
- ret = prog - (u8 *)image;
+ ret = prog - (u8 *)image + BPF_INSN_SAFETY;
cleanup:
kfree(branches);
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size()
2023-09-20 5:31 [PATCH bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
` (4 preceding siblings ...)
2023-09-20 5:31 ` [PATCH bpf-next 5/8] bpf, x86: Adjust arch_prepare_bpf_trampoline return value Song Liu
@ 2023-09-20 5:31 ` Song Liu
2023-09-20 7:39 ` Xu Kuohai
` (3 more replies)
2023-09-20 5:31 ` [PATCH bpf-next 7/8] bpf: Use arch_bpf_trampoline_size for trampoline Song Liu
2023-09-20 5:31 ` [PATCH bpf-next 8/8] x86, bpf: Use bpf_prog_pack for bpf trampoline Song Liu
7 siblings, 4 replies; 17+ messages in thread
From: Song Liu @ 2023-09-20 5:31 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, Song Liu
This helper will be used to calculate the size of the trampoline before
allocating the memory.
Signed-off-by: Song Liu <song@kernel.org>
---
arch/arm64/net/bpf_jit_comp.c | 56 ++++++++++++++++++++++++---------
arch/riscv/net/bpf_jit_comp64.c | 24 +++++++++-----
arch/s390/net/bpf_jit_comp.c | 52 +++++++++++++++++-------------
arch/x86/net/bpf_jit_comp.c | 40 ++++++++++++++++++++---
include/linux/bpf.h | 2 ++
kernel/bpf/trampoline.c | 6 ++++
6 files changed, 131 insertions(+), 49 deletions(-)
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index d81b886ea4df..a6671253b7ed 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -2026,18 +2026,10 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
return ctx->idx;
}
-int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
- void *image_end, const struct btf_func_model *m,
- u32 flags, struct bpf_tramp_links *tlinks,
- void *func_addr)
+static int btf_func_model_nregs(const struct btf_func_model *m)
{
- int i, ret;
int nregs = m->nr_args;
- int max_insns = ((long)image_end - (long)image) / AARCH64_INSN_SIZE;
- struct jit_ctx ctx = {
- .image = NULL,
- .idx = 0,
- };
+ int i;
/* extra registers needed for struct argument */
for (i = 0; i < MAX_BPF_FUNC_ARGS; i++) {
@@ -2046,19 +2038,53 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
nregs += (m->arg_size[i] + 7) / 8 - 1;
}
+ return nregs;
+}
+
+int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
+ struct bpf_tramp_links *tlinks, void *func_addr)
+{
+ struct jit_ctx ctx = {
+ .image = NULL,
+ .idx = 0,
+ };
+ struct bpf_tramp_image im;
+ int nregs, ret;
+
+ nregs = btf_func_model_nregs(m);
/* the first 8 registers are used for arguments */
if (nregs > 8)
return -ENOTSUPP;
- ret = prepare_trampoline(&ctx, im, tlinks, func_addr, nregs, flags);
+ ret = prepare_trampoline(&ctx, &im, tlinks, func_addr, nregs, flags);
if (ret < 0)
return ret;
- if (ret > max_insns)
- return -EFBIG;
+ return ret < 0 ? ret : ret * AARCH64_INSN_SIZE;
+}
- ctx.image = image;
- ctx.idx = 0;
+int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
+ void *image_end, const struct btf_func_model *m,
+ u32 flags, struct bpf_tramp_links *tlinks,
+ void *func_addr)
+{
+ int ret, nregs;
+ struct jit_ctx ctx = {
+ .image = image,
+ .idx = 0,
+ };
+
+ nregs = btf_func_model_nregs(m);
+ /* the first 8 registers are used for arguments */
+ if (nregs > 8)
+ return -ENOTSUPP;
+
+ ret = arch_bpf_trampoline_size(m, flags, tlinks, func_addr);
+ if (ret < 0)
+ return ret;
+
+ if (ret > ((long)image_end - (long)image))
+ return -EFBIG;
jit_fill_hole(image, (unsigned int)(image_end - image));
ret = prepare_trampoline(&ctx, im, tlinks, func_addr, nregs, flags);
diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index ecd3ae6f4116..4de13c4aaad1 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -1024,6 +1024,20 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
return ret;
}
+int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
+ struct bpf_tramp_links *tlinks, void *func_addr)
+{
+ struct bpf_tramp_image im;
+ struct rv_jit_context ctx;
+
+ ctx.ninsns = 0;
+ ctx.insns = NULL;
+ ctx.ro_insns = NULL;
+ ret = __arch_prepare_bpf_trampoline(&im, m, tlinks, func_addr, flags, &ctx);
+
+ return ret < 0 ? ret : ninsns_rvoff(ctx->ninsns);
+}
+
int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
void *image_end, const struct btf_func_model *m,
u32 flags, struct bpf_tramp_links *tlinks,
@@ -1032,14 +1046,8 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
int ret;
struct rv_jit_context ctx;
- ctx.ninsns = 0;
- ctx.insns = NULL;
- ctx.ro_insns = NULL;
- ret = __arch_prepare_bpf_trampoline(im, m, tlinks, func_addr, flags, &ctx);
- if (ret < 0)
- return ret;
-
- if (ninsns_rvoff(ret) > (long)image_end - (long)image)
+ ret = arch_bpf_trampoline_size(im, m, flags, tlinks, func_addr);
+ if (ret > (long)image_end - (long)image)
return -EFBIG;
ctx.ninsns = 0;
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index e6a643f63ebf..8fab4795b497 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -2483,6 +2483,21 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
return 0;
}
+int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
+ struct bpf_tramp_links *tlinks, void *orig_call)
+{
+ struct bpf_tramp_image im;
+ struct bpf_tramp_jit tjit;
+ int ret;
+
+ memset(&tjit, 0, sizeof(tjit));
+
+ ret = __arch_prepare_bpf_trampoline(&im, &tjit, m, flags,
+ tlinks, orig_call);
+
+ return ret < 0 ? ret : tjit.common.prg;
+}
+
int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
void *image_end, const struct btf_func_model *m,
u32 flags, struct bpf_tramp_links *tlinks,
@@ -2490,30 +2505,23 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
{
struct bpf_tramp_jit tjit;
int ret;
- int i;
- for (i = 0; i < 2; i++) {
- if (i == 0) {
- /* Compute offsets, check whether the code fits. */
- memset(&tjit, 0, sizeof(tjit));
- } else {
- /* Generate the code. */
- tjit.common.prg = 0;
- tjit.common.prg_buf = image;
- }
- ret = __arch_prepare_bpf_trampoline(im, &tjit, m, flags,
- tlinks, func_addr);
- if (ret < 0)
- return ret;
- if (tjit.common.prg > (char *)image_end - (char *)image)
- /*
- * Use the same error code as for exceeding
- * BPF_MAX_TRAMP_LINKS.
- */
- return -E2BIG;
- }
+ ret = arch_bpf_trampoline_size(m, flags, tlinks, func_addr);
+ if (ret < 0)
+ return ret;
+
+ if (ret > (char *)image_end - (char *)image)
+ /*
+ * Use the same error code as for exceeding
+ * BPF_MAX_TRAMP_LINKS.
+ */
+ return -E2BIG;
- return tjit.common.prg;
+ tjit.common.prg = 0;
+ tjit.common.prg_buf = image;
+ ret = __arch_prepare_bpf_trampoline(im, &tjit, m, flags,
+ tlinks, func_addr);
+ return ret < 0 ? ret : tjit.common.prg;
}
bool bpf_jit_supports_subprog_tailcalls(void)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 5f7528cac344..eca561621e65 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -2422,10 +2422,10 @@ static int invoke_bpf_mod_ret(const struct btf_func_model *m, u8 **pprog,
* add rsp, 8 // skip eth_type_trans's frame
* ret // return to its caller
*/
-int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
- const struct btf_func_model *m, u32 flags,
- struct bpf_tramp_links *tlinks,
- void *func_addr)
+static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
+ const struct btf_func_model *m, u32 flags,
+ struct bpf_tramp_links *tlinks,
+ void *func_addr)
{
int i, ret, nr_regs = m->nr_args, stack_size = 0;
int regs_off, nregs_off, ip_off, run_ctx_off, arg_stack_off, rbx_off;
@@ -2678,6 +2678,38 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
return ret;
}
+int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
+ const struct btf_func_model *m, u32 flags,
+ struct bpf_tramp_links *tlinks,
+ void *func_addr)
+{
+ return __arch_prepare_bpf_trampoline(im, image, image_end, m, flags, tlinks, func_addr);
+}
+
+int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
+ struct bpf_tramp_links *tlinks, void *func_addr)
+{
+ struct bpf_tramp_image im;
+ void *image;
+ int ret;
+
+ /* Allocate a temporary buffer for __arch_prepare_bpf_trampoline().
+ * This will NOT cause fragmentation in direct map, as we do not
+ * call set_memory_*() on this buffer.
+ */
+ image = bpf_jit_alloc_exec(PAGE_SIZE);
+ if (!image)
+ return -ENOMEM;
+
+ ret = __arch_prepare_bpf_trampoline(&im, image, image + PAGE_SIZE, m, flags,
+ tlinks, func_addr);
+ bpf_jit_free_exec(image);
+ if (ret < 0)
+ return ret;
+
+ return ret;
+}
+
static int emit_bpf_dispatcher(u8 **pprog, int a, int b, s64 *progs, u8 *image, u8 *buf)
{
u8 *jg_reloc, *prog = *pprog;
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 548f3ef34ba1..5bbac549b0a0 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1087,6 +1087,8 @@ void *arch_alloc_bpf_trampoline(int size);
void arch_free_bpf_trampoline(void *image, int size);
void arch_protect_bpf_trampoline(void *image, int size);
void arch_unprotect_bpf_trampoline(void *image, int size);
+int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
+ struct bpf_tramp_links *tlinks, void *func_addr);
u64 notrace __bpf_prog_enter_sleepable_recur(struct bpf_prog *prog,
struct bpf_tramp_run_ctx *run_ctx);
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 5509bdf98067..285c5b7c1ea4 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -1070,6 +1070,12 @@ void __weak arch_unprotect_bpf_trampoline(void *image, int size)
set_memory_rw((long)image, 1);
}
+int __weak arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
+ struct bpf_tramp_links *tlinks, void *func_addr)
+{
+ return -ENOTSUPP;
+}
+
static int __init init_trampolines(void)
{
int i;
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size()
2023-09-20 5:31 ` [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size() Song Liu
@ 2023-09-20 7:39 ` Xu Kuohai
2023-09-20 15:56 ` Song Liu
2023-09-21 0:45 ` Pu Lehui
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Xu Kuohai @ 2023-09-20 7:39 UTC (permalink / raw)
To: Song Liu; +Cc: bpf, ast, daniel, andrii, martin.lau, kernel-team
Hi Song,
On 9/20/2023 1:31 PM, Song Liu wrote:
> This helper will be used to calculate the size of the trampoline before
> allocating the memory.
>
> Signed-off-by: Song Liu <song@kernel.org>
> ---
> arch/arm64/net/bpf_jit_comp.c | 56 ++++++++++++++++++++++++---------
> arch/riscv/net/bpf_jit_comp64.c | 24 +++++++++-----
> arch/s390/net/bpf_jit_comp.c | 52 +++++++++++++++++-------------
> arch/x86/net/bpf_jit_comp.c | 40 ++++++++++++++++++++---
> include/linux/bpf.h | 2 ++
> kernel/bpf/trampoline.c | 6 ++++
> 6 files changed, 131 insertions(+), 49 deletions(-)
>
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index d81b886ea4df..a6671253b7ed 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -2026,18 +2026,10 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
> return ctx->idx;
> }
>
> -int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> - void *image_end, const struct btf_func_model *m,
> - u32 flags, struct bpf_tramp_links *tlinks,
> - void *func_addr)
> +static int btf_func_model_nregs(const struct btf_func_model *m)
> {
> - int i, ret;
> int nregs = m->nr_args;
> - int max_insns = ((long)image_end - (long)image) / AARCH64_INSN_SIZE;
> - struct jit_ctx ctx = {
> - .image = NULL,
> - .idx = 0,
> - };
> + int i;
>
> /* extra registers needed for struct argument */
> for (i = 0; i < MAX_BPF_FUNC_ARGS; i++) {
> @@ -2046,19 +2038,53 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> nregs += (m->arg_size[i] + 7) / 8 - 1;
> }
>
> + return nregs;
> +}
> +
> +int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks, void *func_addr)
> +{
> + struct jit_ctx ctx = {
> + .image = NULL,
> + .idx = 0,
> + };
> + struct bpf_tramp_image im;
> + int nregs, ret;
> +
> + nregs = btf_func_model_nregs(m);
> /* the first 8 registers are used for arguments */
> if (nregs > 8)
> return -ENOTSUPP;
>
> - ret = prepare_trampoline(&ctx, im, tlinks, func_addr, nregs, flags);
> + ret = prepare_trampoline(&ctx, &im, tlinks, func_addr, nregs, flags);
> if (ret < 0)
> return ret;
>
> - if (ret > max_insns)
> - return -EFBIG;
> + return ret < 0 ? ret : ret * AARCH64_INSN_SIZE;
> +}
>
> - ctx.image = image;
> - ctx.idx = 0;
> +int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> + void *image_end, const struct btf_func_model *m,
> + u32 flags, struct bpf_tramp_links *tlinks,
> + void *func_addr)
> +{
> + int ret, nregs;
> + struct jit_ctx ctx = {
> + .image = image,
> + .idx = 0,
> + };
> +
> + nregs = btf_func_model_nregs(m);
> + /* the first 8 registers are used for arguments */
> + if (nregs > 8)
> + return -ENOTSUPP;
> +
> + ret = arch_bpf_trampoline_size(m, flags, tlinks, func_addr);
> + if (ret < 0)
> + return ret;
Since arch_bpf_trampoline_size was already called before the trampoline
image was allocated, it seems this call to arch_bpf_trampoline_size is
unnecessary. If this call can be omitted, we can avoid one less dry run.
> +
> + if (ret > ((long)image_end - (long)image))
> + return -EFBIG;
>
> jit_fill_hole(image, (unsigned int)(image_end - image));
> ret = prepare_trampoline(&ctx, im, tlinks, func_addr, nregs, flags);
[...]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size()
2023-09-20 7:39 ` Xu Kuohai
@ 2023-09-20 15:56 ` Song Liu
0 siblings, 0 replies; 17+ messages in thread
From: Song Liu @ 2023-09-20 15:56 UTC (permalink / raw)
To: Xu Kuohai
Cc: Song Liu, bpf@vger.kernel.org, ast@kernel.org,
daniel@iogearbox.net, andrii@kernel.org, martin.lau@kernel.org,
Kernel Team
Hi Kuohai,
> On Sep 20, 2023, at 12:39 AM, Xu Kuohai <xukuohai@huawei.com> wrote:
>
> Hi Song,
>
> On 9/20/2023 1:31 PM, Song Liu wrote:
>> This helper will be used to calculate the size of the trampoline before
>> allocating the memory.
>> Signed-off-by: Song Liu <song@kernel.org>
>> ---
[...]
>> +
>> + nregs = btf_func_model_nregs(m);
>> + /* the first 8 registers are used for arguments */
>> + if (nregs > 8)
>> + return -ENOTSUPP;
>> +
>> + ret = arch_bpf_trampoline_size(m, flags, tlinks, func_addr);
>> + if (ret < 0)
>> + return ret;
>
> Since arch_bpf_trampoline_size was already called before the trampoline
> image was allocated, it seems this call to arch_bpf_trampoline_size is
> unnecessary. If this call can be omitted, we can avoid one less dry run.
Indeed. This set doesn't call arch_bpf_trampoline_size() from struct_ops.
But we can add that and then remove the _size() call here.
Thanks,
Song
>
>> +
>> + if (ret > ((long)image_end - (long)image))
>> + return -EFBIG;
>> jit_fill_hole(image, (unsigned int)(image_end - image));
>> ret = prepare_trampoline(&ctx, im, tlinks, func_addr, nregs, flags);
>
>
> [...]
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size()
2023-09-20 5:31 ` [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size() Song Liu
2023-09-20 7:39 ` Xu Kuohai
@ 2023-09-21 0:45 ` Pu Lehui
2023-09-21 5:12 ` Song Liu
2023-09-21 14:07 ` Jiri Olsa
2023-09-21 14:24 ` Jiri Olsa
3 siblings, 1 reply; 17+ messages in thread
From: Pu Lehui @ 2023-09-21 0:45 UTC (permalink / raw)
To: Song Liu, bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team
On 2023/9/20 13:31, Song Liu wrote:
> This helper will be used to calculate the size of the trampoline before
> allocating the memory.
>
> Signed-off-by: Song Liu <song@kernel.org>
> ---
> arch/arm64/net/bpf_jit_comp.c | 56 ++++++++++++++++++++++++---------
> arch/riscv/net/bpf_jit_comp64.c | 24 +++++++++-----
> arch/s390/net/bpf_jit_comp.c | 52 +++++++++++++++++-------------
> arch/x86/net/bpf_jit_comp.c | 40 ++++++++++++++++++++---
> include/linux/bpf.h | 2 ++
> kernel/bpf/trampoline.c | 6 ++++
> 6 files changed, 131 insertions(+), 49 deletions(-)
>
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index d81b886ea4df..a6671253b7ed 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -2026,18 +2026,10 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
> return ctx->idx;
> }
>
> -int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> - void *image_end, const struct btf_func_model *m,
> - u32 flags, struct bpf_tramp_links *tlinks,
> - void *func_addr)
> +static int btf_func_model_nregs(const struct btf_func_model *m)
> {
> - int i, ret;
> int nregs = m->nr_args;
> - int max_insns = ((long)image_end - (long)image) / AARCH64_INSN_SIZE;
> - struct jit_ctx ctx = {
> - .image = NULL,
> - .idx = 0,
> - };
> + int i;
>
> /* extra registers needed for struct argument */
> for (i = 0; i < MAX_BPF_FUNC_ARGS; i++) {
> @@ -2046,19 +2038,53 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> nregs += (m->arg_size[i] + 7) / 8 - 1;
> }
>
> + return nregs;
> +}
> +
> +int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks, void *func_addr)
> +{
> + struct jit_ctx ctx = {
> + .image = NULL,
> + .idx = 0,
> + };
> + struct bpf_tramp_image im;
> + int nregs, ret;
> +
> + nregs = btf_func_model_nregs(m);
> /* the first 8 registers are used for arguments */
> if (nregs > 8)
> return -ENOTSUPP;
>
> - ret = prepare_trampoline(&ctx, im, tlinks, func_addr, nregs, flags);
> + ret = prepare_trampoline(&ctx, &im, tlinks, func_addr, nregs, flags);
> if (ret < 0)
> return ret;
>
> - if (ret > max_insns)
> - return -EFBIG;
> + return ret < 0 ? ret : ret * AARCH64_INSN_SIZE;
> +}
>
> - ctx.image = image;
> - ctx.idx = 0;
> +int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> + void *image_end, const struct btf_func_model *m,
> + u32 flags, struct bpf_tramp_links *tlinks,
> + void *func_addr)
> +{
> + int ret, nregs;
> + struct jit_ctx ctx = {
> + .image = image,
> + .idx = 0,
> + };
> +
> + nregs = btf_func_model_nregs(m);
> + /* the first 8 registers are used for arguments */
> + if (nregs > 8)
> + return -ENOTSUPP;
> +
> + ret = arch_bpf_trampoline_size(m, flags, tlinks, func_addr);
> + if (ret < 0)
> + return ret;
> +
> + if (ret > ((long)image_end - (long)image))
> + return -EFBIG;
>
> jit_fill_hole(image, (unsigned int)(image_end - image));
> ret = prepare_trampoline(&ctx, im, tlinks, func_addr, nregs, flags);
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index ecd3ae6f4116..4de13c4aaad1 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -1024,6 +1024,20 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
> return ret;
> }
>
> +int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks, void *func_addr)
> +{
> + struct bpf_tramp_image im;
> + struct rv_jit_context ctx;
> +
> + ctx.ninsns = 0;
> + ctx.insns = NULL;
> + ctx.ro_insns = NULL;
> + ret = __arch_prepare_bpf_trampoline(&im, m, tlinks, func_addr, flags, &ctx);
> +
> + return ret < 0 ? ret : ninsns_rvoff(ctx->ninsns);
> +}
> +
> int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> void *image_end, const struct btf_func_model *m,
> u32 flags, struct bpf_tramp_links *tlinks,
> @@ -1032,14 +1046,8 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> int ret;
> struct rv_jit_context ctx;
>
> - ctx.ninsns = 0;
> - ctx.insns = NULL;
> - ctx.ro_insns = NULL;
> - ret = __arch_prepare_bpf_trampoline(im, m, tlinks, func_addr, flags, &ctx);
> - if (ret < 0)
> - return ret;
> -
> - if (ninsns_rvoff(ret) > (long)image_end - (long)image)
> + ret = arch_bpf_trampoline_size(im, m, flags, tlinks, func_addr);
Hi Song, there is missing check for negative return values.
> + if (ret > (long)image_end - (long)image)
> return -EFBIG;
>
> ctx.ninsns = 0;
> diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
> index e6a643f63ebf..8fab4795b497 100644
> --- a/arch/s390/net/bpf_jit_comp.c
> +++ b/arch/s390/net/bpf_jit_comp.c
> @@ -2483,6 +2483,21 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
> return 0;
> }
>
> +int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks, void *orig_call)
> +{
> + struct bpf_tramp_image im;
> + struct bpf_tramp_jit tjit;
> + int ret;
> +
> + memset(&tjit, 0, sizeof(tjit));
> +
> + ret = __arch_prepare_bpf_trampoline(&im, &tjit, m, flags,
> + tlinks, orig_call);
> +
> + return ret < 0 ? ret : tjit.common.prg;
> +}
> +
> int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> void *image_end, const struct btf_func_model *m,
> u32 flags, struct bpf_tramp_links *tlinks,
> @@ -2490,30 +2505,23 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> {
> struct bpf_tramp_jit tjit;
> int ret;
> - int i;
>
> - for (i = 0; i < 2; i++) {
> - if (i == 0) {
> - /* Compute offsets, check whether the code fits. */
> - memset(&tjit, 0, sizeof(tjit));
> - } else {
> - /* Generate the code. */
> - tjit.common.prg = 0;
> - tjit.common.prg_buf = image;
> - }
> - ret = __arch_prepare_bpf_trampoline(im, &tjit, m, flags,
> - tlinks, func_addr);
> - if (ret < 0)
> - return ret;
> - if (tjit.common.prg > (char *)image_end - (char *)image)
> - /*
> - * Use the same error code as for exceeding
> - * BPF_MAX_TRAMP_LINKS.
> - */
> - return -E2BIG;
> - }
> + ret = arch_bpf_trampoline_size(m, flags, tlinks, func_addr);
> + if (ret < 0)
> + return ret;
> +
> + if (ret > (char *)image_end - (char *)image)
> + /*
> + * Use the same error code as for exceeding
> + * BPF_MAX_TRAMP_LINKS.
> + */
> + return -E2BIG;
>
> - return tjit.common.prg;
> + tjit.common.prg = 0;
> + tjit.common.prg_buf = image;
> + ret = __arch_prepare_bpf_trampoline(im, &tjit, m, flags,
> + tlinks, func_addr);
> + return ret < 0 ? ret : tjit.common.prg;
> }
>
> bool bpf_jit_supports_subprog_tailcalls(void)
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 5f7528cac344..eca561621e65 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -2422,10 +2422,10 @@ static int invoke_bpf_mod_ret(const struct btf_func_model *m, u8 **pprog,
> * add rsp, 8 // skip eth_type_trans's frame
> * ret // return to its caller
> */
> -int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
> - const struct btf_func_model *m, u32 flags,
> - struct bpf_tramp_links *tlinks,
> - void *func_addr)
> +static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
> + const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks,
> + void *func_addr)
> {
> int i, ret, nr_regs = m->nr_args, stack_size = 0;
> int regs_off, nregs_off, ip_off, run_ctx_off, arg_stack_off, rbx_off;
> @@ -2678,6 +2678,38 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> return ret;
> }
>
> +int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
> + const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks,
> + void *func_addr)
> +{
> + return __arch_prepare_bpf_trampoline(im, image, image_end, m, flags, tlinks, func_addr);
> +}
> +
> +int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks, void *func_addr)
> +{
> + struct bpf_tramp_image im;
> + void *image;
> + int ret;
> +
> + /* Allocate a temporary buffer for __arch_prepare_bpf_trampoline().
> + * This will NOT cause fragmentation in direct map, as we do not
> + * call set_memory_*() on this buffer.
> + */
> + image = bpf_jit_alloc_exec(PAGE_SIZE);
> + if (!image)
> + return -ENOMEM;
> +
> + ret = __arch_prepare_bpf_trampoline(&im, image, image + PAGE_SIZE, m, flags,
> + tlinks, func_addr);
> + bpf_jit_free_exec(image);
> + if (ret < 0)
> + return ret;
> +
> + return ret;
> +}
> +
> static int emit_bpf_dispatcher(u8 **pprog, int a, int b, s64 *progs, u8 *image, u8 *buf)
> {
> u8 *jg_reloc, *prog = *pprog;
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 548f3ef34ba1..5bbac549b0a0 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1087,6 +1087,8 @@ void *arch_alloc_bpf_trampoline(int size);
> void arch_free_bpf_trampoline(void *image, int size);
> void arch_protect_bpf_trampoline(void *image, int size);
> void arch_unprotect_bpf_trampoline(void *image, int size);
> +int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks, void *func_addr);
>
> u64 notrace __bpf_prog_enter_sleepable_recur(struct bpf_prog *prog,
> struct bpf_tramp_run_ctx *run_ctx);
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 5509bdf98067..285c5b7c1ea4 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -1070,6 +1070,12 @@ void __weak arch_unprotect_bpf_trampoline(void *image, int size)
> set_memory_rw((long)image, 1);
> }
>
> +int __weak arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks, void *func_addr)
> +{
> + return -ENOTSUPP;
> +}
> +
> static int __init init_trampolines(void)
> {
> int i;
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size()
2023-09-21 0:45 ` Pu Lehui
@ 2023-09-21 5:12 ` Song Liu
0 siblings, 0 replies; 17+ messages in thread
From: Song Liu @ 2023-09-21 5:12 UTC (permalink / raw)
To: Pu Lehui; +Cc: bpf, ast, daniel, andrii, martin.lau, kernel-team
On Wed, Sep 20, 2023 at 5:46 PM Pu Lehui <pulehui@huawei.com> wrote:
>
[...]
> > +int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> > + struct bpf_tramp_links *tlinks, void *func_addr)
> > +{
> > + struct bpf_tramp_image im;
> > + struct rv_jit_context ctx;
> > +
> > + ctx.ninsns = 0;
> > + ctx.insns = NULL;
> > + ctx.ro_insns = NULL;
> > + ret = __arch_prepare_bpf_trampoline(&im, m, tlinks, func_addr, flags, &ctx);
> > +
> > + return ret < 0 ? ret : ninsns_rvoff(ctx->ninsns);
> > +}
> > +
> > int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> > void *image_end, const struct btf_func_model *m,
> > u32 flags, struct bpf_tramp_links *tlinks,
> > @@ -1032,14 +1046,8 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
> > int ret;
> > struct rv_jit_context ctx;
> >
> > - ctx.ninsns = 0;
> > - ctx.insns = NULL;
> > - ctx.ro_insns = NULL;
> > - ret = __arch_prepare_bpf_trampoline(im, m, tlinks, func_addr, flags, &ctx);
> > - if (ret < 0)
> > - return ret;
> > -
> > - if (ninsns_rvoff(ret) > (long)image_end - (long)image)
> > + ret = arch_bpf_trampoline_size(im, m, flags, tlinks, func_addr);
>
> Hi Song, there is missing check for negative return values.
Thanks! I will fix it in the next version.
Song
> > + if (ret > (long)image_end - (long)image)
> > return -EFBIG;
> >
> > ctx.ninsns = 0;
> > diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
> > index e6a643f63ebf..8fab4795b497 100644
> > --- a/arch/s390/net/bpf_jit_comp.c
> > +++ b/arch/s390/net/bpf_jit_comp.c
> > @@ -2483,6 +2483,21 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
> > return 0;
> > }
[...]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size()
2023-09-20 5:31 ` [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size() Song Liu
2023-09-20 7:39 ` Xu Kuohai
2023-09-21 0:45 ` Pu Lehui
@ 2023-09-21 14:07 ` Jiri Olsa
2023-09-21 14:24 ` Jiri Olsa
3 siblings, 0 replies; 17+ messages in thread
From: Jiri Olsa @ 2023-09-21 14:07 UTC (permalink / raw)
To: Song Liu; +Cc: bpf, ast, daniel, andrii, martin.lau, kernel-team
On Tue, Sep 19, 2023 at 10:31:56PM -0700, Song Liu wrote:
SNIP
> +int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks, void *func_addr)
> +{
> + struct bpf_tramp_image im;
> + void *image;
> + int ret;
> +
> + /* Allocate a temporary buffer for __arch_prepare_bpf_trampoline().
> + * This will NOT cause fragmentation in direct map, as we do not
> + * call set_memory_*() on this buffer.
> + */
> + image = bpf_jit_alloc_exec(PAGE_SIZE);
> + if (!image)
> + return -ENOMEM;
> +
> + ret = __arch_prepare_bpf_trampoline(&im, image, image + PAGE_SIZE, m, flags,
> + tlinks, func_addr);
> + bpf_jit_free_exec(image);
> + if (ret < 0)
> + return ret;
> +
> + return ret;
this can be just 'return ret;'
jirka
> +}
> +
> static int emit_bpf_dispatcher(u8 **pprog, int a, int b, s64 *progs, u8 *image, u8 *buf)
> {
> u8 *jg_reloc, *prog = *pprog;
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 548f3ef34ba1..5bbac549b0a0 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1087,6 +1087,8 @@ void *arch_alloc_bpf_trampoline(int size);
> void arch_free_bpf_trampoline(void *image, int size);
> void arch_protect_bpf_trampoline(void *image, int size);
> void arch_unprotect_bpf_trampoline(void *image, int size);
> +int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks, void *func_addr);
>
> u64 notrace __bpf_prog_enter_sleepable_recur(struct bpf_prog *prog,
> struct bpf_tramp_run_ctx *run_ctx);
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 5509bdf98067..285c5b7c1ea4 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -1070,6 +1070,12 @@ void __weak arch_unprotect_bpf_trampoline(void *image, int size)
> set_memory_rw((long)image, 1);
> }
>
> +int __weak arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks, void *func_addr)
> +{
> + return -ENOTSUPP;
> +}
> +
> static int __init init_trampolines(void)
> {
> int i;
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size()
2023-09-20 5:31 ` [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size() Song Liu
` (2 preceding siblings ...)
2023-09-21 14:07 ` Jiri Olsa
@ 2023-09-21 14:24 ` Jiri Olsa
2023-09-21 14:28 ` Jiri Olsa
3 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2023-09-21 14:24 UTC (permalink / raw)
To: Song Liu; +Cc: bpf, ast, daniel, andrii, martin.lau, kernel-team
On Tue, Sep 19, 2023 at 10:31:56PM -0700, Song Liu wrote:
SNIP
> bool bpf_jit_supports_subprog_tailcalls(void)
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 5f7528cac344..eca561621e65 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -2422,10 +2422,10 @@ static int invoke_bpf_mod_ret(const struct btf_func_model *m, u8 **pprog,
> * add rsp, 8 // skip eth_type_trans's frame
> * ret // return to its caller
> */
> -int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
> - const struct btf_func_model *m, u32 flags,
> - struct bpf_tramp_links *tlinks,
> - void *func_addr)
> +static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
> + const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks,
> + void *func_addr)
> {
hum, I dont understand what's the __arch_prepare_bpf_trampoline for,
could we have just the arch_prepare_bpf_trampoline?
jirka
> int i, ret, nr_regs = m->nr_args, stack_size = 0;
> int regs_off, nregs_off, ip_off, run_ctx_off, arg_stack_off, rbx_off;
> @@ -2678,6 +2678,38 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
> return ret;
> }
>
> +int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
> + const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks,
> + void *func_addr)
> +{
> + return __arch_prepare_bpf_trampoline(im, image, image_end, m, flags, tlinks, func_addr);
> +}
> +
> +int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks, void *func_addr)
> +{
> + struct bpf_tramp_image im;
> + void *image;
> + int ret;
> +
> + /* Allocate a temporary buffer for __arch_prepare_bpf_trampoline().
> + * This will NOT cause fragmentation in direct map, as we do not
> + * call set_memory_*() on this buffer.
> + */
> + image = bpf_jit_alloc_exec(PAGE_SIZE);
> + if (!image)
> + return -ENOMEM;
> +
> + ret = __arch_prepare_bpf_trampoline(&im, image, image + PAGE_SIZE, m, flags,
> + tlinks, func_addr);
> + bpf_jit_free_exec(image);
> + if (ret < 0)
> + return ret;
> +
> + return ret;
> +}
> +
> static int emit_bpf_dispatcher(u8 **pprog, int a, int b, s64 *progs, u8 *image, u8 *buf)
> {
> u8 *jg_reloc, *prog = *pprog;
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 548f3ef34ba1..5bbac549b0a0 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1087,6 +1087,8 @@ void *arch_alloc_bpf_trampoline(int size);
> void arch_free_bpf_trampoline(void *image, int size);
> void arch_protect_bpf_trampoline(void *image, int size);
> void arch_unprotect_bpf_trampoline(void *image, int size);
> +int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks, void *func_addr);
>
> u64 notrace __bpf_prog_enter_sleepable_recur(struct bpf_prog *prog,
> struct bpf_tramp_run_ctx *run_ctx);
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 5509bdf98067..285c5b7c1ea4 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -1070,6 +1070,12 @@ void __weak arch_unprotect_bpf_trampoline(void *image, int size)
> set_memory_rw((long)image, 1);
> }
>
> +int __weak arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
> + struct bpf_tramp_links *tlinks, void *func_addr)
> +{
> + return -ENOTSUPP;
> +}
> +
> static int __init init_trampolines(void)
> {
> int i;
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size()
2023-09-21 14:24 ` Jiri Olsa
@ 2023-09-21 14:28 ` Jiri Olsa
0 siblings, 0 replies; 17+ messages in thread
From: Jiri Olsa @ 2023-09-21 14:28 UTC (permalink / raw)
To: Song Liu; +Cc: bpf, ast, daniel, andrii, martin.lau, kernel-team
On Thu, Sep 21, 2023 at 04:25:02PM +0200, Jiri Olsa wrote:
> On Tue, Sep 19, 2023 at 10:31:56PM -0700, Song Liu wrote:
>
> SNIP
>
> > bool bpf_jit_supports_subprog_tailcalls(void)
> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > index 5f7528cac344..eca561621e65 100644
> > --- a/arch/x86/net/bpf_jit_comp.c
> > +++ b/arch/x86/net/bpf_jit_comp.c
> > @@ -2422,10 +2422,10 @@ static int invoke_bpf_mod_ret(const struct btf_func_model *m, u8 **pprog,
> > * add rsp, 8 // skip eth_type_trans's frame
> > * ret // return to its caller
> > */
> > -int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
> > - const struct btf_func_model *m, u32 flags,
> > - struct bpf_tramp_links *tlinks,
> > - void *func_addr)
> > +static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
> > + const struct btf_func_model *m, u32 flags,
> > + struct bpf_tramp_links *tlinks,
> > + void *func_addr)
> > {
>
> hum, I dont understand what's the __arch_prepare_bpf_trampoline for,
> could we have just the arch_prepare_bpf_trampoline?
ok, in the next patch ;-)
jirka
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH bpf-next 7/8] bpf: Use arch_bpf_trampoline_size for trampoline
2023-09-20 5:31 [PATCH bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
` (5 preceding siblings ...)
2023-09-20 5:31 ` [PATCH bpf-next 6/8] bpf: Add arch_bpf_trampoline_size() Song Liu
@ 2023-09-20 5:31 ` Song Liu
2023-09-20 5:31 ` [PATCH bpf-next 8/8] x86, bpf: Use bpf_prog_pack for bpf trampoline Song Liu
7 siblings, 0 replies; 17+ messages in thread
From: Song Liu @ 2023-09-20 5:31 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, Song Liu
Instead of blindly allocating PAGE_SIZE for each trampoline, check the size
of the trampoline with arch_bpf_trampoline_size(). This size is saved in
bpf_tramp_image->size, and used for modmem charge/uncharge. The fallback
arch_alloc_bpf_trampoline() still allocates a whole page because we need to
use set_memory_* to protect the memory.
This change does not cover struct_ops trampoline, which will still use a
whole page for multiple trampolines.
Signed-off-by: Song Liu <song@kernel.org>
---
include/linux/bpf.h | 1 +
kernel/bpf/trampoline.c | 41 +++++++++++++++++++++++++----------------
2 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 5bbac549b0a0..20ce9b536344 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1122,6 +1122,7 @@ enum bpf_tramp_prog_type {
struct bpf_tramp_image {
void *image;
+ int size;
struct bpf_ksym ksym;
struct percpu_ref pcref;
void *ip_after_call;
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 285c5b7c1ea4..da97ec6bdfd5 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -254,8 +254,8 @@ bpf_trampoline_get_progs(const struct bpf_trampoline *tr, int *total, bool *ip_a
static void bpf_tramp_image_free(struct bpf_tramp_image *im)
{
bpf_image_ksym_del(&im->ksym);
- arch_free_bpf_trampoline(im->image, PAGE_SIZE);
- bpf_jit_uncharge_modmem(PAGE_SIZE);
+ arch_free_bpf_trampoline(im->image, im->size);
+ bpf_jit_uncharge_modmem(im->size);
percpu_ref_exit(&im->pcref);
kfree_rcu(im, rcu);
}
@@ -349,7 +349,7 @@ static void bpf_tramp_image_put(struct bpf_tramp_image *im)
call_rcu_tasks_trace(&im->rcu, __bpf_tramp_image_put_rcu_tasks);
}
-static struct bpf_tramp_image *bpf_tramp_image_alloc(u64 key)
+static struct bpf_tramp_image *bpf_tramp_image_alloc(u64 key, int size)
{
struct bpf_tramp_image *im;
struct bpf_ksym *ksym;
@@ -360,12 +360,13 @@ static struct bpf_tramp_image *bpf_tramp_image_alloc(u64 key)
if (!im)
goto out;
- err = bpf_jit_charge_modmem(PAGE_SIZE);
+ err = bpf_jit_charge_modmem(size);
if (err)
goto out_free_im;
+ im->size = size;
err = -ENOMEM;
- im->image = image = arch_alloc_bpf_trampoline(PAGE_SIZE);
+ im->image = image = arch_alloc_bpf_trampoline(size);
if (!image)
goto out_uncharge;
@@ -382,7 +383,7 @@ static struct bpf_tramp_image *bpf_tramp_image_alloc(u64 key)
out_free_image:
arch_free_bpf_trampoline(im->image, PAGE_SIZE);
out_uncharge:
- bpf_jit_uncharge_modmem(PAGE_SIZE);
+ bpf_jit_uncharge_modmem(size);
out_free_im:
kfree(im);
out:
@@ -395,7 +396,7 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
struct bpf_tramp_links *tlinks;
u32 orig_flags = tr->flags;
bool ip_arg = false;
- int err, total;
+ int err, total, size;
tlinks = bpf_trampoline_get_progs(tr, &total, &ip_arg);
if (IS_ERR(tlinks))
@@ -408,12 +409,6 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
goto out;
}
- im = bpf_tramp_image_alloc(tr->key);
- if (IS_ERR(im)) {
- err = PTR_ERR(im);
- goto out;
- }
-
/* clear all bits except SHARE_IPMODIFY and TAIL_CALL_CTX */
tr->flags &= (BPF_TRAMP_F_SHARE_IPMODIFY | BPF_TRAMP_F_TAIL_CALL_CTX);
@@ -437,7 +432,20 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
tr->flags |= BPF_TRAMP_F_ORIG_STACK;
#endif
- err = arch_prepare_bpf_trampoline(im, im->image, im->image + PAGE_SIZE,
+ size = arch_bpf_trampoline_size(&tr->func.model, tr->flags,
+ tlinks, tr->func.addr);
+ if (size < 0) {
+ err = size;
+ goto out;
+ }
+
+ im = bpf_tramp_image_alloc(tr->key, size);
+ if (IS_ERR(im)) {
+ err = PTR_ERR(im);
+ goto out;
+ }
+
+ err = arch_prepare_bpf_trampoline(im, im->image, im->image + size,
&tr->func.model, tr->flags, tlinks,
tr->func.addr);
if (err < 0)
@@ -463,8 +471,9 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
tr->fops->func = NULL;
tr->fops->trampoline = 0;
- /* reset im->image memory attr for arch_prepare_bpf_trampoline */
- arch_unprotect_bpf_trampoline(im->image, PAGE_SIZE);
+ /* free im->image memory and reallocate later */
+ arch_free_bpf_trampoline(im->image, im->size);
+ im->image = NULL;
goto again;
}
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH bpf-next 8/8] x86, bpf: Use bpf_prog_pack for bpf trampoline
2023-09-20 5:31 [PATCH bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
` (6 preceding siblings ...)
2023-09-20 5:31 ` [PATCH bpf-next 7/8] bpf: Use arch_bpf_trampoline_size for trampoline Song Liu
@ 2023-09-20 5:31 ` Song Liu
7 siblings, 0 replies; 17+ messages in thread
From: Song Liu @ 2023-09-20 5:31 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, Song Liu
There are three major changes here:
1. Add arch_[alloc|free]_bpf_trampoline based on bpf_prog_pack;
2. Let arch_prepare_bpf_trampoline handle ROX input image, this requires
arch_prepare_bpf_trampoline allocating a temporary RW buffer;
3. Update __arch_prepare_bpf_trampoline() to handle a RW buffer (rw_image)
and a ROX buffer (image). This part is similar to the image/rw_image
logic in bpf_int_jit_compile().
Signed-off-by: Song Liu <song@kernel.org>
---
arch/x86/net/bpf_jit_comp.c | 95 +++++++++++++++++++++++++++----------
1 file changed, 69 insertions(+), 26 deletions(-)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index eca561621e65..a6a27563cf43 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -2198,7 +2198,8 @@ static void restore_regs(const struct btf_func_model *m, u8 **prog,
static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog,
struct bpf_tramp_link *l, int stack_size,
- int run_ctx_off, bool save_ret)
+ int run_ctx_off, bool save_ret,
+ void *image, void *rw_image)
{
u8 *prog = *pprog;
u8 *jmp_insn;
@@ -2226,7 +2227,7 @@ static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog,
else
EMIT4(0x48, 0x8D, 0x75, -run_ctx_off);
- if (emit_rsb_call(&prog, bpf_trampoline_enter(p), prog))
+ if (emit_rsb_call(&prog, bpf_trampoline_enter(p), image + (prog - (u8 *)rw_image)))
return -EINVAL;
/* remember prog start time returned by __bpf_prog_enter */
emit_mov_reg(&prog, true, BPF_REG_6, BPF_REG_0);
@@ -2250,7 +2251,7 @@ static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog,
(long) p->insnsi >> 32,
(u32) (long) p->insnsi);
/* call JITed bpf program or interpreter */
- if (emit_rsb_call(&prog, p->bpf_func, prog))
+ if (emit_rsb_call(&prog, p->bpf_func, image + (prog - (u8 *)rw_image)))
return -EINVAL;
/*
@@ -2277,7 +2278,7 @@ static int invoke_bpf_prog(const struct btf_func_model *m, u8 **pprog,
EMIT3_off32(0x48, 0x8D, 0x95, -run_ctx_off);
else
EMIT4(0x48, 0x8D, 0x55, -run_ctx_off);
- if (emit_rsb_call(&prog, bpf_trampoline_exit(p), prog))
+ if (emit_rsb_call(&prog, bpf_trampoline_exit(p), image + (prog - (u8 *)rw_image)))
return -EINVAL;
*pprog = prog;
@@ -2312,14 +2313,15 @@ static int emit_cond_near_jump(u8 **pprog, void *func, void *ip, u8 jmp_cond)
static int invoke_bpf(const struct btf_func_model *m, u8 **pprog,
struct bpf_tramp_links *tl, int stack_size,
- int run_ctx_off, bool save_ret)
+ int run_ctx_off, bool save_ret,
+ void *image, void *rw_image)
{
int i;
u8 *prog = *pprog;
for (i = 0; i < tl->nr_links; i++) {
if (invoke_bpf_prog(m, &prog, tl->links[i], stack_size,
- run_ctx_off, save_ret))
+ run_ctx_off, save_ret, image, rw_image))
return -EINVAL;
}
*pprog = prog;
@@ -2328,7 +2330,8 @@ static int invoke_bpf(const struct btf_func_model *m, u8 **pprog,
static int invoke_bpf_mod_ret(const struct btf_func_model *m, u8 **pprog,
struct bpf_tramp_links *tl, int stack_size,
- int run_ctx_off, u8 **branches)
+ int run_ctx_off, u8 **branches,
+ void *image, void *rw_image)
{
u8 *prog = *pprog;
int i;
@@ -2339,7 +2342,8 @@ static int invoke_bpf_mod_ret(const struct btf_func_model *m, u8 **pprog,
emit_mov_imm32(&prog, false, BPF_REG_0, 0);
emit_stx(&prog, BPF_DW, BPF_REG_FP, BPF_REG_0, -8);
for (i = 0; i < tl->nr_links; i++) {
- if (invoke_bpf_prog(m, &prog, tl->links[i], stack_size, run_ctx_off, true))
+ if (invoke_bpf_prog(m, &prog, tl->links[i], stack_size, run_ctx_off, true,
+ image, rw_image))
return -EINVAL;
/* mod_ret prog stored return value into [rbp - 8]. Emit:
@@ -2422,7 +2426,8 @@ static int invoke_bpf_mod_ret(const struct btf_func_model *m, u8 **pprog,
* add rsp, 8 // skip eth_type_trans's frame
* ret // return to its caller
*/
-static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
+static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_image,
+ void *rw_image_end, void *image,
const struct btf_func_model *m, u32 flags,
struct bpf_tramp_links *tlinks,
void *func_addr)
@@ -2521,7 +2526,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image
orig_call += X86_PATCH_SIZE;
}
- prog = image;
+ prog = rw_image;
EMIT_ENDBR();
/*
@@ -2563,7 +2568,8 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image
if (flags & BPF_TRAMP_F_CALL_ORIG) {
/* arg1: mov rdi, im */
emit_mov_imm64(&prog, BPF_REG_1, (long) im >> 32, (u32) (long) im);
- if (emit_rsb_call(&prog, __bpf_tramp_enter, prog)) {
+ if (emit_rsb_call(&prog, __bpf_tramp_enter,
+ image + (prog - (u8 *)rw_image))) {
ret = -EINVAL;
goto cleanup;
}
@@ -2571,7 +2577,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image
if (fentry->nr_links)
if (invoke_bpf(m, &prog, fentry, regs_off, run_ctx_off,
- flags & BPF_TRAMP_F_RET_FENTRY_RET))
+ flags & BPF_TRAMP_F_RET_FENTRY_RET, image, rw_image))
return -EINVAL;
if (fmod_ret->nr_links) {
@@ -2581,7 +2587,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image
return -ENOMEM;
if (invoke_bpf_mod_ret(m, &prog, fmod_ret, regs_off,
- run_ctx_off, branches)) {
+ run_ctx_off, branches, image, rw_image)) {
ret = -EINVAL;
goto cleanup;
}
@@ -2602,14 +2608,14 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image
EMIT2(0xff, 0xd3); /* call *rbx */
} else {
/* call original function */
- if (emit_rsb_call(&prog, orig_call, prog)) {
+ if (emit_rsb_call(&prog, orig_call, image + (prog - (u8 *)rw_image))) {
ret = -EINVAL;
goto cleanup;
}
}
/* remember return value in a stack for bpf prog to access */
emit_stx(&prog, BPF_DW, BPF_REG_FP, BPF_REG_0, -8);
- im->ip_after_call = prog;
+ im->ip_after_call = image + (prog - (u8 *)rw_image);
memcpy(prog, x86_nops[5], X86_PATCH_SIZE);
prog += X86_PATCH_SIZE;
}
@@ -2625,12 +2631,13 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image
* aligned address of do_fexit.
*/
for (i = 0; i < fmod_ret->nr_links; i++)
- emit_cond_near_jump(&branches[i], prog, branches[i],
- X86_JNE);
+ emit_cond_near_jump(&branches[i], image + (prog - (u8 *)rw_image),
+ image + (branches[i] - (u8 *)rw_image), X86_JNE);
}
if (fexit->nr_links)
- if (invoke_bpf(m, &prog, fexit, regs_off, run_ctx_off, false)) {
+ if (invoke_bpf(m, &prog, fexit, regs_off, run_ctx_off,
+ false, image, rw_image)) {
ret = -EINVAL;
goto cleanup;
}
@@ -2643,10 +2650,10 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image
* restored to R0.
*/
if (flags & BPF_TRAMP_F_CALL_ORIG) {
- im->ip_epilogue = prog;
+ im->ip_epilogue = image + (prog - (u8 *)rw_image);
/* arg1: mov rdi, im */
emit_mov_imm64(&prog, BPF_REG_1, (long) im >> 32, (u32) (long) im);
- if (emit_rsb_call(&prog, __bpf_tramp_exit, prog)) {
+ if (emit_rsb_call(&prog, __bpf_tramp_exit, image + (prog - (u8 *)rw_image))) {
ret = -EINVAL;
goto cleanup;
}
@@ -2665,25 +2672,61 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image
if (flags & BPF_TRAMP_F_SKIP_FRAME)
/* skip our return address and return to parent */
EMIT4(0x48, 0x83, 0xC4, 8); /* add rsp, 8 */
- emit_return(&prog, prog);
+ emit_return(&prog, image + (prog - (u8 *)rw_image));
/* Make sure the trampoline generation logic doesn't overflow */
- if (WARN_ON_ONCE(prog > (u8 *)image_end - BPF_INSN_SAFETY)) {
+ if (WARN_ON_ONCE(prog > (u8 *)rw_image_end - BPF_INSN_SAFETY)) {
ret = -EFAULT;
goto cleanup;
}
- ret = prog - (u8 *)image + BPF_INSN_SAFETY;
+ ret = prog - (u8 *)rw_image + BPF_INSN_SAFETY;
cleanup:
kfree(branches);
return ret;
}
+void *arch_alloc_bpf_trampoline(int size)
+{
+ return bpf_prog_pack_alloc(size, jit_fill_hole);
+}
+
+void arch_free_bpf_trampoline(void *image, int size)
+{
+ bpf_prog_pack_free(image, size);
+}
+
+void arch_protect_bpf_trampoline(void *image, int size)
+{
+}
+
+void arch_unprotect_bpf_trampoline(void *image, int size)
+{
+}
+
int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *image_end,
const struct btf_func_model *m, u32 flags,
struct bpf_tramp_links *tlinks,
void *func_addr)
{
- return __arch_prepare_bpf_trampoline(im, image, image_end, m, flags, tlinks, func_addr);
+ void *rw_image, *tmp;
+ int ret;
+ u32 size = image_end - image;
+
+ rw_image = bpf_jit_alloc_exec(size);
+ if (!rw_image)
+ return -ENOMEM;
+
+ ret = __arch_prepare_bpf_trampoline(im, rw_image, rw_image + size, image, m,
+ flags, tlinks, func_addr);
+ if (ret < 0)
+ goto out;
+
+ tmp = bpf_arch_text_copy(image, rw_image, size);
+ if (IS_ERR(tmp))
+ ret = PTR_ERR(tmp);
+out:
+ bpf_jit_free_exec(rw_image);
+ return ret;
}
int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
@@ -2701,8 +2744,8 @@ int arch_bpf_trampoline_size(const struct btf_func_model *m, u32 flags,
if (!image)
return -ENOMEM;
- ret = __arch_prepare_bpf_trampoline(&im, image, image + PAGE_SIZE, m, flags,
- tlinks, func_addr);
+ ret = __arch_prepare_bpf_trampoline(&im, image, image + PAGE_SIZE, image,
+ m, flags, tlinks, func_addr);
bpf_jit_free_exec(image);
if (ret < 0)
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread