On Fri, Jun 15, 2018 at 02:30:47AM +0200, Daniel Borkmann wrote:
> While testing I found that when hitting error path in bpf_prog_load()
> where we jump to free_used_maps and prog contained BPF to BPF calls
> that were JITed earlier, then we never clean up the bpf_prog_kallsyms_add()
> done under jit_subprogs(). Add proper API to make BPF kallsyms deletion
> more clear and fix that.
>
> Fixes: 1c2a088a6626 ("bpf: x64: add JIT support for multi-function programs")
> Signed-off-by: Daniel Borkmann <dan...@iogearbox.net>
Acked-by: Martin KaFai Lau <ka...@fb.com>
> ---
> include/linux/filter.h | 3 +++
> kernel/bpf/core.c | 14 ++++++++++++++
> kernel/bpf/syscall.c | 8 ++------
> 3 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 45fc0f5..297c56f 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -961,6 +961,9 @@ static inline void bpf_prog_kallsyms_del(struct bpf_prog
> *fp)
> }
> #endif /* CONFIG_BPF_JIT */
>
> +void bpf_prog_kallsyms_del_subprogs(struct bpf_prog *fp);
> +void bpf_prog_kallsyms_del_all(struct bpf_prog *fp);
> +
> #define BPF_ANC BIT(15)
>
> static inline bool bpf_needs_clear_a(const struct sock_filter *first)
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 9f14937..1061968 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -350,6 +350,20 @@ struct bpf_prog *bpf_patch_insn_single(struct bpf_prog
> *prog, u32 off,
> return prog_adj;
> }
>
> +void bpf_prog_kallsyms_del_subprogs(struct bpf_prog *fp)
> +{
> + int i;
> +
> + for (i = 0; i < fp->aux->func_cnt; i++)
> + bpf_prog_kallsyms_del(fp->aux->func[i]);
> +}
> +
> +void bpf_prog_kallsyms_del_all(struct bpf_prog *fp)
> +{
> + bpf_prog_kallsyms_del_subprogs(fp);
> + bpf_prog_kallsyms_del(fp);
> +}
> +
> #ifdef CONFIG_BPF_JIT
> /* All BPF JIT sysctl knobs here. */
> int bpf_jit_enable __read_mostly = IS_BUILTIN(CONFIG_BPF_JIT_ALWAYS_ON);
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 0fa2062..0f62692 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1034,14 +1034,9 @@ static void __bpf_prog_put_rcu(struct rcu_head *rcu)
> static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock)
> {
> if (atomic_dec_and_test(&prog->aux->refcnt)) {
> - int i;
> -
> /* bpf_prog_free_id() must be called first */
> bpf_prog_free_id(prog, do_idr_lock);
> -
> - for (i = 0; i < prog->aux->func_cnt; i++)
> - bpf_prog_kallsyms_del(prog->aux->func[i]);
> - bpf_prog_kallsyms_del(prog);
> + bpf_prog_kallsyms_del_all(prog);
>
> call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu);
> }
> @@ -1384,6 +1379,7 @@ static int bpf_prog_load(union bpf_attr *attr)
> return err;
>
> free_used_maps:
> + bpf_prog_kallsyms_del_subprogs(prog);
> free_used_maps(prog->aux);
> free_prog:
> bpf_prog_uncharge_memlock(prog);
> --
> 2.9.5
>
