> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -416,6 +416,7 @@ void free_ftrace_hash(struct ftrace_hash *hash);
> struct ftrace_func_entry *add_ftrace_hash_entry_direct(struct ftrace_hash
> *hash,
> unsigned long ip,
> unsigned long direct);
> unsigned long ftrace_hash_count(struct ftrace_hash *hash);
> +void ftrace_hash_remove(struct ftrace_hash *hash);
>
> /* The hash used to know what functions callbacks trace */
> struct ftrace_ops_hash {
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -1249,6 +1249,25 @@ remove_hash_entry(struct ftrace_hash *hash,
> hash->count--;
> }
>
> +void ftrace_hash_remove(struct ftrace_hash *hash)
> +{
> + struct hlist_head *hhd;
> + struct hlist_node *tn;
> + struct ftrace_func_entry *entry;
> + int size = 1 << hash->size_bits;
^^^^^^^^^^^^^^^^
Can this dereference a NULL pointer? The code computes the size using
hash->size_bits before checking if hash is NULL:
int size = 1 << hash->size_bits;
...
if (!hash || !hash->count)
return;
If a caller passes NULL, the function will crash on the size computation.
Looking at how this is used in the later commit e77bc5ff90dd
('bpf: Add bpf_trampoline_multi_attach/detach functions'),
init_tracing_multi_data() in kernel/bpf/trampoline.c allocates three hashes:
data->reg = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
data->unreg = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
data->modify = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
if (!data->reg || !data->unreg || !data->modify) {
clear_tracing_multi_data(data);
return -ENOMEM;
}
On allocation failure, clear_tracing_multi_data() calls
remove_tracing_multi_data(), which calls ftrace_hash_remove() on data->reg,
data->unreg, and data->modify. Any of these might be NULL if the
corresponding alloc_ftrace_hash() failed.
Would it be safer to move the size calculation below the NULL check?
> + int i;
> +
> + if (!hash || !hash->count)
> + return;
> +
> + for (i = 0; i < size; i++) {
> + hhd = &hash->buckets[i];
> + hlist_for_each_entry_safe(entry, tn, hhd, hlist)
> + remove_hash_entry(hash, entry);
> + }
> + FTRACE_WARN_ON(hash->count);
> +}
> +
> static void ftrace_hash_clear(struct ftrace_hash *hash)
> {
> struct hlist_head *hhd;
---
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/24598000047
