On Wed, Dec 03, 2025 at 08:47:45AM +0000, [email protected] wrote:
> > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> > index 8c034b028..eb60d971e 100644
> > --- a/include/linux/ftrace.h
> > +++ b/include/linux/ftrace.h
>
> [ ... ]
>
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index 84aee9096..1660a8754 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
>
> [ ... ]
>
> > +int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash
> > *hash)
> > +{
> > + struct ftrace_hash *old_direct_functions = NULL, *new_direct_functions
> > = NULL;
> > + struct ftrace_hash *old_filter_hash = NULL, *new_filter_hash = NULL;
> > + struct ftrace_func_entry *entry;
> > + int i, size, err = -EINVAL;
> > + bool reg;
> > +
> > + if (!hash_count(hash))
> > + return -EINVAL;
> > +
> > + mutex_lock(&direct_mutex);
> > +
> > + /* Make sure requested entries are not already registered. */
> > + size = 1 << hash->size_bits;
> > + for (i = 0; i < size; i++) {
> > + hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
> > + if (__ftrace_lookup_ip(direct_functions, entry->ip))
> > + goto out_unlock;
> > + }
> > + }
> > +
> > + old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
> > + old_direct_functions = direct_functions;
> > +
> > + /* If there's nothing in filter_hash we need to register the ops. */
> > + reg = hash_count(old_filter_hash) == 0;
> > + if (reg) {
> > + if (ops->func || ops->trampoline)
> > + goto out_unlock;
> > + if (ops->flags & FTRACE_OPS_FL_ENABLED)
> > + goto out_unlock;
> > + }
> > +
> > + err = -ENOMEM;
> > + new_filter_hash = hash_add(old_filter_hash, hash);
> > + if (!new_filter_hash)
> > + goto out_unlock;
> > +
> > + new_direct_functions = hash_add(old_direct_functions, hash);
> > + if (!new_direct_functions)
> > + goto out_unlock;
>
> Can new_filter_hash leak here? If the second hash_add() fails but the
> first one succeeds, update_ftrace_direct_add() will goto out_unlock
> with new_filter_hash pointing to allocated memory. At out_unlock, only
> old_filter_hash gets freed, not new_filter_hash.
mamma mia, can't get this right apparently.. yea, will fix in next
version, thanks
jirka
>
> register_ftrace_direct() handles this by freeing new_hash on error:
>
> out_unlock:
> mutex_unlock(&direct_mutex);
>
> if (free_hash && free_hash != EMPTY_HASH)
> call_rcu_tasks(&free_hash->rcu, register_ftrace_direct_cb);
>
> if (new_hash)
> free_ftrace_hash(new_hash);
>
> return err;
>
> > +
> > + rcu_assign_pointer(direct_functions, new_direct_functions);
> > +
> > + if (reg) {
> > + ops->func = call_direct_funcs;
> > + ops->flags |= MULTI_FLAGS;
> > + ops->trampoline = FTRACE_REGS_ADDR;
> > + ops->local_hash.filter_hash = new_filter_hash;
> > +
> > + err = register_ftrace_function_nolock(ops);
> > + if (err) {
> > + /* restore old filter on error */
> > + ops->local_hash.filter_hash = old_filter_hash;
> > + old_filter_hash = new_filter_hash;
> > +
> > + /* cleanup for possible another register call */
> > + ops->func = NULL;
> > + ops->trampoline = 0;
> > + }
> > + } else {
> > + err = ftrace_update_ops(ops, new_filter_hash, EMPTY_HASH);
> > + /*
> > + * new_filter_hash is dup-ed, so we need to release it anyway,
> > + * old_filter_hash either stays on error or is released already
> > + */
> > + old_filter_hash = new_filter_hash;
> > + }
> > +
> > + if (err) {
> > + /* reset direct_functions and free the new one */
> > + rcu_assign_pointer(direct_functions, old_direct_functions);
> > + old_direct_functions = new_direct_functions;
> > + }
> > +
> > + out_unlock:
> > + mutex_unlock(&direct_mutex);
> > +
> > + if (old_direct_functions && old_direct_functions != EMPTY_HASH)
> > + call_rcu_tasks(&old_direct_functions->rcu,
> > register_ftrace_direct_cb);
> > + if (old_filter_hash)
> > + free_ftrace_hash(old_filter_hash);
> > +
> > + return err;
> > +}
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19887401362
