* [PATCH v3 bpf-next 1/8] s390/bpf: Let arch_prepare_bpf_trampoline return program size
2023-09-26 19:00 [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
@ 2023-09-26 19:00 ` Song Liu
2023-09-26 19:00 ` [PATCH v3 bpf-next 2/8] bpf: Let bpf_prog_pack_free handle any pointer Song Liu
` (7 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2023-09-26 19:00 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, iii, bjorn,
Song Liu
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 9ed0a13865ca..fd1936a63878 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -2652,7 +2652,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] 14+ messages in thread* [PATCH v3 bpf-next 2/8] bpf: Let bpf_prog_pack_free handle any pointer
2023-09-26 19:00 [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
2023-09-26 19:00 ` [PATCH v3 bpf-next 1/8] s390/bpf: Let arch_prepare_bpf_trampoline return program size Song Liu
@ 2023-09-26 19:00 ` Song Liu
2023-09-27 18:29 ` Björn Töpel
2023-09-26 19:00 ` [PATCH v3 bpf-next 3/8] bpf: Adjust argument names of arch_prepare_bpf_trampoline() Song Liu
` (6 subsequent siblings)
8 siblings, 1 reply; 14+ messages in thread
From: Song Liu @ 2023-09-26 19:00 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, iii, bjorn,
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>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Tested-by: Ilya Leoshkevich <iii@linux.ibm.com> # on s390x
---
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] 14+ messages in thread* Re: [PATCH v3 bpf-next 2/8] bpf: Let bpf_prog_pack_free handle any pointer
2023-09-26 19:00 ` [PATCH v3 bpf-next 2/8] bpf: Let bpf_prog_pack_free handle any pointer Song Liu
@ 2023-09-27 18:29 ` Björn Töpel
0 siblings, 0 replies; 14+ messages in thread
From: Björn Töpel @ 2023-09-27 18:29 UTC (permalink / raw)
To: Song Liu, bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, iii, Song Liu
Song Liu <song@kernel.org> writes:
> 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>
> Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
> Tested-by: Ilya Leoshkevich <iii@linux.ibm.com> # on s390x
> ---
> 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);
Nice to get rid of that ugliness!
Reviewed-by: Björn Töpel <bjorn@rivosinc.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 bpf-next 3/8] bpf: Adjust argument names of arch_prepare_bpf_trampoline()
2023-09-26 19:00 [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
2023-09-26 19:00 ` [PATCH v3 bpf-next 1/8] s390/bpf: Let arch_prepare_bpf_trampoline return program size Song Liu
2023-09-26 19:00 ` [PATCH v3 bpf-next 2/8] bpf: Let bpf_prog_pack_free handle any pointer Song Liu
@ 2023-09-26 19:00 ` Song Liu
2023-09-26 19:00 ` [PATCH v3 bpf-next 4/8] bpf: Add helpers for trampoline image management Song Liu
` (5 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2023-09-26 19:00 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, iii, bjorn,
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>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Tested-by: Ilya Leoshkevich <iii@linux.ibm.com> # on s390x
---
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 a82efd34b741..f90339c26c4e 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] 14+ messages in thread* [PATCH v3 bpf-next 4/8] bpf: Add helpers for trampoline image management
2023-09-26 19:00 [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
` (2 preceding siblings ...)
2023-09-26 19:00 ` [PATCH v3 bpf-next 3/8] bpf: Adjust argument names of arch_prepare_bpf_trampoline() Song Liu
@ 2023-09-26 19:00 ` Song Liu
2023-09-26 19:00 ` [PATCH v3 bpf-next 5/8] bpf, x86: Adjust arch_prepare_bpf_trampoline return value Song Liu
` (4 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2023-09-26 19:00 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, iii, bjorn,
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>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Tested-by: Ilya Leoshkevich <iii@linux.ibm.com> # on s390x
---
include/linux/bpf.h | 5 ++++
kernel/bpf/bpf_struct_ops.c | 12 ++++------
kernel/bpf/trampoline.c | 44 ++++++++++++++++++++++++++++------
net/bpf/bpf_dummy_struct_ops.c | 7 +++---
4 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index f90339c26c4e..b28852351959 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;
diff --git a/net/bpf/bpf_dummy_struct_ops.c b/net/bpf/bpf_dummy_struct_ops.c
index 5918d1b32e19..2748f9d77b18 100644
--- a/net/bpf/bpf_dummy_struct_ops.c
+++ b/net/bpf/bpf_dummy_struct_ops.c
@@ -101,12 +101,11 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,
goto out;
}
- image = bpf_jit_alloc_exec(PAGE_SIZE);
+ image = arch_alloc_bpf_trampoline(PAGE_SIZE);
if (!image) {
err = -ENOMEM;
goto out;
}
- set_vm_flush_reset_perms(image);
link = kzalloc(sizeof(*link), GFP_USER);
if (!link) {
@@ -124,7 +123,7 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,
if (err < 0)
goto out;
- set_memory_rox((long)image, 1);
+ arch_protect_bpf_trampoline(image, PAGE_SIZE);
prog_ret = dummy_ops_call_op(image, args);
err = dummy_ops_copy_args(args);
@@ -134,7 +133,7 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,
err = -EFAULT;
out:
kfree(args);
- bpf_jit_free_exec(image);
+ arch_free_bpf_trampoline(image, PAGE_SIZE);
if (link)
bpf_link_put(&link->link);
kfree(tlinks);
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 bpf-next 5/8] bpf, x86: Adjust arch_prepare_bpf_trampoline return value
2023-09-26 19:00 [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
` (3 preceding siblings ...)
2023-09-26 19:00 ` [PATCH v3 bpf-next 4/8] bpf: Add helpers for trampoline image management Song Liu
@ 2023-09-26 19:00 ` Song Liu
2023-09-26 19:00 ` [PATCH v3 bpf-next 6/8] bpf: Add arch_bpf_trampoline_size() Song Liu
` (3 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2023-09-26 19:00 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, iii, bjorn,
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>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
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] 14+ messages in thread* [PATCH v3 bpf-next 6/8] bpf: Add arch_bpf_trampoline_size()
2023-09-26 19:00 [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
` (4 preceding siblings ...)
2023-09-26 19:00 ` [PATCH v3 bpf-next 5/8] bpf, x86: Adjust arch_prepare_bpf_trampoline return value Song Liu
@ 2023-09-26 19:00 ` Song Liu
2023-09-26 19:00 ` [PATCH v3 bpf-next 7/8] bpf: Use arch_bpf_trampoline_size Song Liu
` (2 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2023-09-26 19:00 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, iii, bjorn,
Song Liu
This helper will be used to calculate the size of the trampoline before
allocating the memory.
arch_prepare_bpf_trampoline() for arm64 and riscv64 can use
arch_bpf_trampoline_size() to check the trampoline fits in the image.
OTOH, arch_prepare_bpf_trampoline() for s390 has to call the JIT process
twice, so it cannot use arch_bpf_trampoline_size().
Signed-off-by: Song Liu <song@kernel.org>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Tested-by: Ilya Leoshkevich <iii@linux.ibm.com> # on s390x
---
arch/arm64/net/bpf_jit_comp.c | 56 ++++++++++++++++++++++++---------
arch/riscv/net/bpf_jit_comp64.c | 22 ++++++++++---
arch/s390/net/bpf_jit_comp.c | 56 ++++++++++++++++++++-------------
arch/x86/net/bpf_jit_comp.c | 37 +++++++++++++++++++---
include/linux/bpf.h | 2 ++
kernel/bpf/trampoline.c | 6 ++++
6 files changed, 133 insertions(+), 46 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..2e5299943816 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -1024,6 +1024,21 @@ 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;
+ int ret;
+
+ 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 +1047,11 @@ 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);
+ ret = arch_bpf_trampoline_size(im, m, flags, tlinks, func_addr);
if (ret < 0)
return ret;
- if (ninsns_rvoff(ret) > (long)image_end - (long)image)
+ 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 fd1936a63878..aa83cff3458e 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -2622,6 +2622,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,
@@ -2629,30 +2644,27 @@ 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;
- }
+ /* Compute offsets, check whether the code fits. */
+ memset(&tjit, 0, sizeof(tjit));
+ 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;
+
+ tjit.common.prg = 0;
+ tjit.common.prg_buf = image;
+ ret = __arch_prepare_bpf_trampoline(im, &tjit, m, flags,
+ tlinks, func_addr);
- return tjit.common.prg;
+ 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..561530ef2cdb 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,35 @@ 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);
+ 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 b28852351959..0160e92e30f9 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] 14+ messages in thread* [PATCH v3 bpf-next 7/8] bpf: Use arch_bpf_trampoline_size
2023-09-26 19:00 [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
` (5 preceding siblings ...)
2023-09-26 19:00 ` [PATCH v3 bpf-next 6/8] bpf: Add arch_bpf_trampoline_size() Song Liu
@ 2023-09-26 19:00 ` Song Liu
2023-09-26 19:00 ` [PATCH v3 bpf-next 8/8] x86, bpf: Use bpf_prog_pack for bpf trampoline Song Liu
2023-10-04 15:40 ` [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack patchwork-bot+netdevbpf
8 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2023-09-26 19:00 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, iii, bjorn,
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.
struct_ops trampoline still uses a whole page for multiple trampolines.
With this size check at caller (regular trampoline and struct_ops
trampoline), remove arch_bpf_trampoline_size() from
arch_prepare_bpf_trampoline() in archs.
Signed-off-by: Song Liu <song@kernel.org>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Tested-by: Ilya Leoshkevich <iii@linux.ibm.com> # on s390x
---
arch/arm64/net/bpf_jit_comp.c | 7 -----
arch/riscv/net/bpf_jit_comp64.c | 7 -----
include/linux/bpf.h | 1 +
kernel/bpf/bpf_struct_ops.c | 7 +++++
kernel/bpf/trampoline.c | 49 +++++++++++++++++++++------------
5 files changed, 39 insertions(+), 32 deletions(-)
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index a6671253b7ed..8955da5c47cf 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -2079,13 +2079,6 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
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 2e5299943816..1d838f1f2db4 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -1047,13 +1047,6 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image,
int ret;
struct rv_jit_context ctx;
- ret = arch_bpf_trampoline_size(im, m, flags, tlinks, func_addr);
- if (ret < 0)
- return ret;
-
- if (ret > (long)image_end - (long)image)
- return -EFBIG;
-
ctx.ninsns = 0;
/*
* The bpf_int_jit_compile() uses a RW buffer (ctx.insns) to write the
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 0160e92e30f9..61169422a295 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/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index e9e95879bce2..4d53c53fc5aa 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -355,6 +355,7 @@ int bpf_struct_ops_prepare_trampoline(struct bpf_tramp_links *tlinks,
void *image, void *image_end)
{
u32 flags;
+ int size;
tlinks[BPF_TRAMP_FENTRY].links[0] = link;
tlinks[BPF_TRAMP_FENTRY].nr_links = 1;
@@ -362,6 +363,12 @@ int bpf_struct_ops_prepare_trampoline(struct bpf_tramp_links *tlinks,
* and it must be used alone.
*/
flags = model->ret_size > 0 ? BPF_TRAMP_F_RET_FENTRY_RET : 0;
+
+ size = arch_bpf_trampoline_size(model, flags, tlinks, NULL);
+ if (size < 0)
+ return size;
+ if (size > (unsigned long)image_end - (unsigned long)image)
+ return -E2BIG;
return arch_prepare_bpf_trampoline(NULL, image, image_end,
model, flags, tlinks, NULL);
}
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 285c5b7c1ea4..7c0535edab3f 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;
@@ -380,9 +381,9 @@ static struct bpf_tramp_image *bpf_tramp_image_alloc(u64 key)
return im;
out_free_image:
- arch_free_bpf_trampoline(im->image, PAGE_SIZE);
+ arch_free_bpf_trampoline(im->image, im->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,13 +432,31 @@ 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;
+ }
+
+ if (size > PAGE_SIZE) {
+ err = -E2BIG;
+ 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)
goto out_free;
- arch_protect_bpf_trampoline(im->image, PAGE_SIZE);
+ arch_protect_bpf_trampoline(im->image, im->size);
WARN_ON(tr->cur_image && total == 0);
if (tr->cur_image)
@@ -463,8 +476,8 @@ 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 memory and reallocate later */
+ bpf_tramp_image_free(im);
goto again;
}
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 bpf-next 8/8] x86, bpf: Use bpf_prog_pack for bpf trampoline
2023-09-26 19:00 [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
` (6 preceding siblings ...)
2023-09-26 19:00 ` [PATCH v3 bpf-next 7/8] bpf: Use arch_bpf_trampoline_size Song Liu
@ 2023-09-26 19:00 ` Song Liu
2023-09-27 13:16 ` Jiri Olsa
2023-10-04 15:40 ` [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack patchwork-bot+netdevbpf
8 siblings, 1 reply; 14+ messages in thread
From: Song Liu @ 2023-09-26 19:00 UTC (permalink / raw)
To: bpf; +Cc: ast, daniel, andrii, martin.lau, kernel-team, iii, bjorn,
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>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
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 561530ef2cdb..52e1e3e57848 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);
return ret;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 bpf-next 8/8] x86, bpf: Use bpf_prog_pack for bpf trampoline
2023-09-26 19:00 ` [PATCH v3 bpf-next 8/8] x86, bpf: Use bpf_prog_pack for bpf trampoline Song Liu
@ 2023-09-27 13:16 ` Jiri Olsa
2023-09-27 15:47 ` Song Liu
0 siblings, 1 reply; 14+ messages in thread
From: Jiri Olsa @ 2023-09-27 13:16 UTC (permalink / raw)
To: Song Liu; +Cc: bpf, ast, daniel, andrii, martin.lau, kernel-team, iii, bjorn
On Tue, Sep 26, 2023 at 12:00:20PM -0700, Song Liu wrote:
SNIP
> @@ -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)
> +{
> +}
seems bit confusing having empty non weak functions to overload
the weak versions IIUC
would maybe some other way fit better than weak functions in here?
like having arch specific macro to use bpf_prog_pack_alloc for
trampoline allocation
feel free to disregard if you have already investigated this ;-)
jirka
> +
> 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);
> return ret;
> }
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 bpf-next 8/8] x86, bpf: Use bpf_prog_pack for bpf trampoline
2023-09-27 13:16 ` Jiri Olsa
@ 2023-09-27 15:47 ` Song Liu
0 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2023-09-27 15:47 UTC (permalink / raw)
To: Jiri Olsa
Cc: Song Liu, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, martin.lau@kernel.org, Kernel Team,
iii@linux.ibm.com, bjorn@kernel.org
> On Sep 27, 2023, at 6:16 AM, Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Tue, Sep 26, 2023 at 12:00:20PM -0700, Song Liu wrote:
>
> SNIP
>
>> @@ -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)
>> +{
>> +}
>
> seems bit confusing having empty non weak functions to overload
> the weak versions IIUC
>
> would maybe some other way fit better than weak functions in here?
> like having arch specific macro to use bpf_prog_pack_alloc for
> trampoline allocation
We can also have a few flags that arch code set at init time.
Then we can use these flags in trampoline.c. But I don't think
that's cleaner than current version.
With more archs adopting bpf_prog_pack, I think we will be able
to remove these helpers soon.
Thanks,
Song
>
> feel free to disregard if you have already investigated this ;-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack
2023-09-26 19:00 [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack Song Liu
` (7 preceding siblings ...)
2023-09-26 19:00 ` [PATCH v3 bpf-next 8/8] x86, bpf: Use bpf_prog_pack for bpf trampoline Song Liu
@ 2023-10-04 15:40 ` patchwork-bot+netdevbpf
2023-10-04 15:50 ` Alexei Starovoitov
8 siblings, 1 reply; 14+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-10-04 15:40 UTC (permalink / raw)
To: Song Liu; +Cc: bpf, ast, daniel, andrii, martin.lau, kernel-team, iii, bjorn
Hello:
This series was applied to netdev/net.git (main)
by Alexei Starovoitov <ast@kernel.org>:
On Tue, 26 Sep 2023 12:00:12 -0700 you wrote:
> This set enables allocating bpf trampoline from bpf_prog_pack on x86. The
> majority of this work, however, is the refactoring of trampoline code.
> This is needed because we need to handle 4 archs and 2 users (trampoline
> and struct_ops).
>
> 1/8 is a dependency that is already applied to bpf tree.
> 2/8 through 7/8 refactors trampoline code. A few helpers are added.
> 8/8 finally let bpf trampoline on x86 use bpf_prog_pack.
>
> [...]
Here is the summary with links:
- [v3,bpf-next,1/8] s390/bpf: Let arch_prepare_bpf_trampoline return program size
https://git.kernel.org/netdev/net/c/cf094baa3e0f
- [v3,bpf-next,2/8] bpf: Let bpf_prog_pack_free handle any pointer
(no matching commit)
- [v3,bpf-next,3/8] bpf: Adjust argument names of arch_prepare_bpf_trampoline()
(no matching commit)
- [v3,bpf-next,4/8] bpf: Add helpers for trampoline image management
(no matching commit)
- [v3,bpf-next,5/8] bpf, x86: Adjust arch_prepare_bpf_trampoline return value
(no matching commit)
- [v3,bpf-next,6/8] bpf: Add arch_bpf_trampoline_size()
(no matching commit)
- [v3,bpf-next,7/8] bpf: Use arch_bpf_trampoline_size
(no matching commit)
- [v3,bpf-next,8/8] x86, bpf: Use bpf_prog_pack for bpf trampoline
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack
2023-10-04 15:40 ` [PATCH v3 bpf-next 0/8] Allocate bpf trampoline on bpf_prog_pack patchwork-bot+netdevbpf
@ 2023-10-04 15:50 ` Alexei Starovoitov
0 siblings, 0 replies; 14+ messages in thread
From: Alexei Starovoitov @ 2023-10-04 15:50 UTC (permalink / raw)
To: patchwork-bot+netdevbpf
Cc: Song Liu, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Kernel Team, Ilya Leoshkevich,
Björn Töpel, Jakub Kicinski, David S. Miller
On Wed, Oct 4, 2023 at 8:40 AM <patchwork-bot+netdevbpf@kernel.org> wrote:
>
> Hello:
>
> This series was applied to netdev/net.git (main)
> by Alexei Starovoitov <ast@kernel.org>:
What is going on?
I didn't touch this commit.
> On Tue, 26 Sep 2023 12:00:12 -0700 you wrote:
> > This set enables allocating bpf trampoline from bpf_prog_pack on x86. The
> > majority of this work, however, is the refactoring of trampoline code.
> > This is needed because we need to handle 4 archs and 2 users (trampoline
> > and struct_ops).
> >
> > 1/8 is a dependency that is already applied to bpf tree.
> > 2/8 through 7/8 refactors trampoline code. A few helpers are added.
> > 8/8 finally let bpf trampoline on x86 use bpf_prog_pack.
> >
> > [...]
>
> Here is the summary with links:
> - [v3,bpf-next,1/8] s390/bpf: Let arch_prepare_bpf_trampoline return program size
> https://git.kernel.org/netdev/net/c/cf094baa3e0f
> - [v3,bpf-next,2/8] bpf: Let bpf_prog_pack_free handle any pointer
> (no matching commit)
> - [v3,bpf-next,3/8] bpf: Adjust argument names of arch_prepare_bpf_trampoline()
> (no matching commit)
> - [v3,bpf-next,4/8] bpf: Add helpers for trampoline image management
> (no matching commit)
> - [v3,bpf-next,5/8] bpf, x86: Adjust arch_prepare_bpf_trampoline return value
> (no matching commit)
> - [v3,bpf-next,6/8] bpf: Add arch_bpf_trampoline_size()
> (no matching commit)
> - [v3,bpf-next,7/8] bpf: Use arch_bpf_trampoline_size
> (no matching commit)
> - [v3,bpf-next,8/8] x86, bpf: Use bpf_prog_pack for bpf trampoline
> (no matching commit)
>
> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread