On Mon, Feb 13, 2023 at 04:59:58PM +0100, Viktor Malik wrote:
SNIP
> @@ -248,8 +223,6 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
> ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, NULL, new_addr);
> }
>
> - if (ret)
> - bpf_trampoline_module_put(tr);
> return ret;
> }
>
> @@ -719,8 +692,11 @@ int bpf_trampoline_link_cgroup_shim(struct bpf_prog *prog,
>
> bpf_lsm_find_cgroup_shim(prog, &bpf_func);
> tr = bpf_trampoline_get(key, &tgt_info);
> - if (!tr)
> + if (!tr) {
> + if (tgt_info.tgt_mod)
> + module_put(tgt_info.tgt_mod);
> return -ENOMEM;
> + }
>
> mutex_lock(&tr->mutex);
>
> @@ -800,6 +776,14 @@ struct bpf_trampoline *bpf_trampoline_get(u64 key,
> return NULL;
>
> mutex_lock(&tr->mutex);
> + if (tgt_info->tgt_mod) {
> + if (tr->mod)
> + /* we already have the module reference, release tgt_info reference */
> + module_put(tgt_info->tgt_mod);
> + else
> + /* take ownership of the module reference */
> + tr->mod = tgt_info->tgt_mod;
this seems tricky, should we take and save module reference in bpf_prog
struct and release it when the program goes out? IIUC the module for
which the program was verified for should stay as long as the program
is loaded
jirka
> + }
> if (tr->func.addr)
> goto out;
>
> @@ -819,6 +803,10 @@ void bpf_trampoline_put(struct bpf_trampoline *tr)
> mutex_lock(&trampoline_mutex);
> if (!refcount_dec_and_test(&tr->refcnt))
> goto out;
> + if (tr->mod) {
> + module_put(tr->mod);
> + tr->mod = NULL;
> + }
> WARN_ON_ONCE(mutex_is_locked(&tr->mutex));
>
> for (i = 0; i < BPF_TRAMP_MAX; i++)
SNIP
