> 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.
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
