Skip to content

ftrace,bpf: Use single direct ops for bpf trampolines #10342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 8 commits into
base: bpf-next_base
Choose a base branch
from
Open

ftrace,bpf: Use single direct ops for bpf trampolines #10342

wants to merge 8 commits into from
+547 −51

Conversation

@kernel-patches-daemon-bpf
Copy link

@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot commented Nov 20, 2025

Pull request for series with
subject: ftrace,bpf: Use single direct ops for bpf trampolines
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1026027

@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Nov 20, 2025

Upstream branch: d088da9
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1026027
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Nov 20, 2025

Upstream branch: e0940c6
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1026027
version: 1

@kernel-patches-review-bot
Copy link

kernel-patches-review-bot bot commented Nov 20, 2025 

commit d7f49335d23efdc78d50eba6b26e9483d50b7d3f
Author: Jiri Olsa <jolsa@kernel.org>

ftrace: Add update_ftrace_direct_del function

Adding update_ftrace_direct_del function that removes all entries
(ip -> addr) provided in hash argument to direct ftrace ops and updates
its attachments. This allows gradual removal of direct ftrace entries
using the hash-based API.

> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 9cf4cd56d..c571deeff 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -551,6 +551,7 @@ int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr);
>  int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned long addr);
>
>  int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash);
> +int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash);
>
>  void ftrace_stub_direct_tramp(void);
>
> @@ -583,6 +584,11 @@ static inline int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace
>  	return -ENODEV;
>  }
>
> +static inline int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
> +{
> +	return -ENODEV;
> +}
> +

[ ... ]

> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 850ff55ff..cc730a8fd 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c

[ ... ]

> +/**
> + * hash_sub - substracts @b from @a and returns the result
> + * @a: struct ftrace_hash object
> + * @b: struct ftrace_hash object
> + *
> + * Returns struct ftrace_hash object on success, NULL on error.
> + */
> +static struct ftrace_hash *hash_sub(struct ftrace_hash *a, struct ftrace_hash *b)
> +{
> +	struct ftrace_func_entry *entry, *del;
> +	struct ftrace_hash *sub;
> +	int size, i;
> +
> +	sub = alloc_and_copy_ftrace_hash(a->size_bits, a);
                                           ^^^^^^^^^^^^

Can hash_sub() handle a NULL hash pointer in parameter a?  I notice the
parallel function hash_add() safely handles NULL via hash_count() and
alloc_and_copy_ftrace_hash() which checks ftrace_hash_empty().

However, hash_sub() directly dereferences a->size_bits before passing a
to alloc_and_copy_ftrace_hash().  This will crash if a is NULL.

This matters because update_ftrace_direct_del() calls hash_sub() with
old_filter_hash which can be NULL:

    old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
    ...
    new_filter_hash = hash_sub(old_filter_hash, hash);

The existing update_ftrace_direct_add() has the same defensive check and
passes the potentially-NULL old_filter_hash to hash_add(), which handles
it correctly.

> +	if (!sub)
> +		goto error;
> +
> +	size = 1 << b->size_bits;
> +	for (i = 0; i < size; i++) {
> +		hlist_for_each_entry(entry, &b->buckets[i], hlist) {
> +			del = __ftrace_lookup_ip(sub, entry->ip);
> +			if (WARN_ON_ONCE(!del))
> +				goto error;
> +			remove_hash_entry(sub, del);
> +			kfree(del);
> +		}
> +	}
> +	return sub;
> +
> + error:
> +	free_ftrace_hash(sub);
> +	return NULL;
> +}
> +
> +int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
> +{

[ ... ]

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

In-Reply-To-Subject: ftrace: Add update_ftrace_direct_del function
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19552032346

@kernel-patches-review-bot
Copy link

kernel-patches-review-bot bot commented Nov 20, 2025 

commit 40f5f8d0d847f5e567ac4cb3bc74631895619aa2
Author: Jiri Olsa <jolsa@kernel.org>

ftrace: Add update_ftrace_direct_mod function

Adding update_ftrace_direct_mod function that modifies all entries
(ip -> direct) provided in hash argument to direct ftrace ops and
updates its attachments. This enables batch modification of multiple
ip -> direct entries at once for BPF direct interface users.

> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index c571deeff..4e2c0ed76 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h

[ ... ]

> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index cc730a8fd..5243aefb6 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c

[ ... ]

> @@ -6486,6 +6486,74 @@ int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
>  	return err;
>  }
>
> +int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash *hash, bool do_direct_lock)
> +{
> +	struct ftrace_hash *orig_hash = ops->func_hash->filter_hash;
                                             ^^^^^^^^^^^^^^^^

Can this crash if ops->func_hash is NULL?  In update_ftrace_direct_add()
and update_ftrace_direct_del(), there's a defensive check:

    old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;

But update_ftrace_direct_mod() dereferences ops->func_hash before any
validation checks occur. The check_direct_multi(ops) validation happens
later, but the dereference occurs first at function entry.

> +	struct ftrace_func_entry *entry, *tmp;
> +	static struct ftrace_ops tmp_ops = {
> +		.func		= ftrace_stub,
> +		.flags		= FTRACE_OPS_FL_STUB,
> +	};
> +	unsigned long size, i;
> +	int err;
> +
> +	if (!hash_count(hash))
> +		return -EINVAL;
> +	if (check_direct_multi(ops))
> +		return -EINVAL;

[ ... ]

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

In-Reply-To-Subject: ftrace: Add update_ftrace_direct_mod function
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19552032346

@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Nov 20, 2025

Forwarding comment 3560349965 via email
In-Reply-To: 20251120212402.466524-5-jolsa@kernel.org
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20251120212402.466524-5-jolsa@kernel.org/

@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Nov 20, 2025

Forwarding comment 3560350292 via email
In-Reply-To: 20251120212402.466524-6-jolsa@kernel.org
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20251120212402.466524-6-jolsa@kernel.org/

@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Nov 21, 2025

Upstream branch: 792f258
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1026027
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Nov 22, 2025

Upstream branch: 878ee3c
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1026027
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Nov 22, 2025

Upstream branch: ae24fc8
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1026027
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Nov 22, 2025

Upstream branch: 4dd3a48
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1026027
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Nov 22, 2025

Upstream branch: 8f7cf30
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1026027
version: 1

@olsajiri
ftrace: Make alloc_and_copy_ftrace_hash direct friendly
ffef9b8 
Make alloc_and_copy_ftrace_hash to copy also direct address
for each hash entry.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
@olsajiri
ftrace: Export some of hash related functions
5d2e96c 
We are going to use these functions in following changes.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
@olsajiri
ftrace: Add update_ftrace_direct_add function
ab411a9 
Adding update_ftrace_direct_add function that adds all entries
(ip -> addr) provided in hash argument to direct ftrace ops
and updates its attachments.

The difference to current register_ftrace_direct is
 - hash argument that allows to register multiple ip -> direct
   entries at once
 - we can call update_ftrace_direct_add multiple times on the
   same ftrace_ops object, becase after first registration with
   register_ftrace_function_nolock, it uses ftrace_update_ops to
   update the ftrace_ops object

This change will allow us to have simple ftrace_ops for all bpf
direct interface users in following changes.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
@olsajiri
ftrace: Add update_ftrace_direct_del function
644f0c2 
Adding update_ftrace_direct_del function that removes all entries
(ip -> addr) provided in hash argument to direct ftrace ops and
updates its attachments.

The difference to current unregister_ftrace_direct is
 - hash argument that allows to unregister multiple ip -> direct
   entries at once
 - we can call update_ftrace_direct_del multiple times on the
   same ftrace_ops object, becase we do not need to unregister
   all entries at once, we can do it gradualy with the help of
   ftrace_update_ops function

This change will allow us to have simple ftrace_ops for all bpf
direct interface users in following changes.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Nov 22, 2025

Upstream branch: c427320
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1026027
version: 1

@olsajiri
ftrace: Add update_ftrace_direct_mod function
1645768 
Adding update_ftrace_direct_mod function that modifies all entries
(ip -> direct) provided in hash argument to direct ftrace ops and
updates its attachments.

The difference to current modify_ftrace_direct is:
- hash argument that allows to modify multiple ip -> direct
  entries at once

This change will allow us to have simple ftrace_ops for all bpf
direct interface users in following changes.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
@olsajiri
bpf: Add trampoline ip hash table
b28d6a6 
Following changes need to lookup trampoline based on its ip address,
adding hash table for that.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
@olsajiri
ftrace: Factor ftrace_ops ops_func interface
0365cdd 
We are going to remove "ftrace_ops->private == bpf_trampoline" setup
in following changes.

Adding ip argument to ftrace_ops_func_t callback function, so we can
use it to look up the trampoline.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
@olsajiri
bpf, x86: Use single ftrace_ops for direct calls
332d6b7 
Using single ftrace_ops for direct calls update instead of allocating
ftrace_ops object for each trampoline.

With single ftrace_ops object we can use update_ftrace_direct_* api
that allows multiple ip sites updates on single ftrace_ops object.

Adding HAVE_SINGLE_FTRACE_DIRECT_OPS config option to be enabled on
each arch that supports this.

At the moment we can enable this only on x86 arch, because arm relies
on ftrace_ops object representing just single trampoline image (stored
in ftrace_ops::direct_call). Ach that do not support this will continue
to use *_ftrace_direct api.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bpf-next new V1-ci-pass V1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@olsajiri