Song Liu <song@xxxxxxxxxx> 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@xxxxxxxxxx>
> Acked-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
> Tested-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx> # 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@xxxxxxxxxxxx>
