| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 2 Jun 2024 11:40:32 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org, Mark
Rutland <mark.rutland@....com>, Mathieu Desnoyers
<mathieu.desnoyers@...icios.com>, Andrew Morton
<akpm@...ux-foundation.org>, Alexei Starovoitov
<alexei.starovoitov@...il.com>, Florent Revest <revest@...omium.org>,
Martin KaFai Lau <martin.lau@...ux.dev>, bpf <bpf@...r.kernel.org>, Sven
Schnelle <svens@...ux.ibm.com>, Alexei Starovoitov <ast@...nel.org>, Jiri
Olsa <jolsa@...nel.org>, Arnaldo Carvalho de Melo <acme@...nel.org>, Daniel
Borkmann <daniel@...earbox.net>, Alan Maguire <alan.maguire@...cle.com>,
Peter Zijlstra <peterz@...radead.org>, Thomas Gleixner
<tglx@...utronix.de>, Guo Ren <guoren@...nel.org>
Subject: Re: [PATCH 10/20] function_graph: Have the instances use their own
ftrace_ops for filtering
On Fri, 31 May 2024 18:49:10 -0400
Steven Rostedt <rostedt@...dmis.org> wrote:
> On Fri, 31 May 2024 23:50:23 +0900
> Masami Hiramatsu (Google) <mhiramat@...nel.org> wrote:
>
> > So is it similar to the fprobe/kprobe, use shared signle ftrace_ops,
> > but keep each fgraph has own hash table?
>
> Sort of.
>
> I created helper functions in ftrace that lets you have a "manager
> ftrace_ops" that will be used to assign to ftrace (with the function
> that will demultiplex), and then you can have "subops" that can be
> assigned that is per user. Here's a glimpse of the code:
>
> /**
> * ftrace_startup_subops - enable tracing for subops of an ops
> * @ops: Manager ops (used to pick all the functions of its subops)
> * @subops: A new ops to add to @ops
> * @command: Extra commands to use to enable tracing
> *
> * The @ops is a manager @ops that has the filter that includes all the functions
> * that its list of subops are tracing. Adding a new @subops will add the
> * functions of @subops to @ops.
> */
> int ftrace_startup_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command)
> {
> struct ftrace_hash *filter_hash;
> struct ftrace_hash *notrace_hash;
> struct ftrace_hash *save_filter_hash;
> struct ftrace_hash *save_notrace_hash;
> int size_bits;
> int ret;
>
> if (unlikely(ftrace_disabled))
> return -ENODEV;
>
> ftrace_ops_init(ops);
> ftrace_ops_init(subops);
>
> /* Make everything canonical (Just in case!) */
> if (!ops->func_hash->filter_hash)
> ops->func_hash->filter_hash = EMPTY_HASH;
> if (!ops->func_hash->notrace_hash)
> ops->func_hash->notrace_hash = EMPTY_HASH;
> if (!subops->func_hash->filter_hash)
> subops->func_hash->filter_hash = EMPTY_HASH;
> if (!subops->func_hash->notrace_hash)
> subops->func_hash->notrace_hash = EMPTY_HASH;
>
> /* For the first subops to ops just enable it normally */
> if (list_empty(&ops->subop_list)) {
May above ftrace_ops_init() clear this list up always?
> /* Just use the subops hashes */
> filter_hash = copy_hash(subops->func_hash->filter_hash);
> notrace_hash = copy_hash(subops->func_hash->notrace_hash);
> if (!filter_hash || !notrace_hash) {
> free_ftrace_hash(filter_hash);
> free_ftrace_hash(notrace_hash);
> return -ENOMEM;
> }
>
> save_filter_hash = ops->func_hash->filter_hash;
> save_notrace_hash = ops->func_hash->notrace_hash;
>
> ops->func_hash->filter_hash = filter_hash;
> ops->func_hash->notrace_hash = notrace_hash;
> list_add(&subops->list, &ops->subop_list);
> ret = ftrace_startup(ops, command);
> if (ret < 0) {
> list_del(&subops->list);
> ops->func_hash->filter_hash = save_filter_hash;
> ops->func_hash->notrace_hash = save_notrace_hash;
> free_ftrace_hash(filter_hash);
> free_ftrace_hash(notrace_hash);
> } else {
> free_ftrace_hash(save_filter_hash);
> free_ftrace_hash(save_notrace_hash);
> subops->flags |= FTRACE_OPS_FL_ENABLED;
> }
> return ret;
> }
>
> /*
> * Here there's already something attached. Here are the rules:
> * o If either filter_hash is empty then the final stays empty
> * o Otherwise, the final is a superset of both hashes
> * o If either notrace_hash is empty then the final stays empty
> * o Otherwise, the final is an intersection between the hashes
Yeah, filter_hash |= subops_filter_hash and notrace_hash &= subops_notrace_hash.
The complicated point is filter's EMPTY_HASH means FULLSET_HASH. :)
> */
> if (ops->func_hash->filter_hash == EMPTY_HASH ||
> subops->func_hash->filter_hash == EMPTY_HASH) {
> filter_hash = EMPTY_HASH;
> } else {
> size_bits = max(ops->func_hash->filter_hash->size_bits,
> subops->func_hash->filter_hash->size_bits);
Don't we need to expand the size_bits? In the worst case, both hash does not
share any entry, then it should be expanded.
> filter_hash = alloc_and_copy_ftrace_hash(size_bits, ops->func_hash->filter_hash);
> if (!filter_hash)
> return -ENOMEM;
> ret = append_hash(&filter_hash, subops->func_hash->filter_hash);
> if (ret < 0) {
> free_ftrace_hash(filter_hash);
> return ret;
> }
> }
>
> if (ops->func_hash->notrace_hash == EMPTY_HASH ||
> subops->func_hash->notrace_hash == EMPTY_HASH) {
> notrace_hash = EMPTY_HASH;
> } else {
> size_bits = max(ops->func_hash->filter_hash->size_bits,
> subops->func_hash->filter_hash->size_bits);
> notrace_hash = alloc_ftrace_hash(size_bits);
> if (!notrace_hash) {
> free_ftrace_hash(filter_hash);
> return -ENOMEM;
> }
>
> ret = intersect_hash(¬race_hash, ops->func_hash->filter_hash,
> subops->func_hash->filter_hash);
> if (ret < 0) {
> free_ftrace_hash(filter_hash);
> free_ftrace_hash(notrace_hash);
> return ret;
> }
> }
>
> list_add(&subops->list, &ops->subop_list);
>
> ret = ftrace_update_ops(ops, filter_hash, notrace_hash);
> free_ftrace_hash(filter_hash);
> free_ftrace_hash(notrace_hash);
> if (ret < 0)
> list_del(&subops->list);
> return ret;
> }
>
> /**
> * ftrace_shutdown_subops - Remove a subops from a manager ops
> * @ops: A manager ops to remove @subops from
> * @subops: The subops to remove from @ops
> * @command: Any extra command flags to add to modifying the text
> *
> * Removes the functions being traced by the @subops from @ops. Note, it
> * will not affect functions that are being traced by other subops that
> * still exist in @ops.
> *
> * If the last subops is removed from @ops, then @ops is shutdown normally.
> */
> int ftrace_shutdown_subops(struct ftrace_ops *ops, struct ftrace_ops *subops, int command)
> {
> struct ftrace_hash *filter_hash;
> struct ftrace_hash *notrace_hash;
> int ret;
>
> if (unlikely(ftrace_disabled))
> return -ENODEV;
>
> list_del(&subops->list);
>
> if (list_empty(&ops->subop_list)) {
> /* Last one, just disable the current ops */
>
> ret = ftrace_shutdown(ops, command);
> if (ret < 0) {
> list_add(&subops->list, &ops->subop_list);
> return ret;
> }
>
> subops->flags &= ~FTRACE_OPS_FL_ENABLED;
>
> free_ftrace_hash(ops->func_hash->filter_hash);
> free_ftrace_hash(ops->func_hash->notrace_hash);
> ops->func_hash->filter_hash = EMPTY_HASH;
> ops->func_hash->notrace_hash = EMPTY_HASH;
>
> return 0;
> }
>
> /* Rebuild the hashes without subops */
> filter_hash = append_hashes(ops);
> notrace_hash = intersect_hashes(ops);
> if (!filter_hash || !notrace_hash) {
> free_ftrace_hash(filter_hash);
> free_ftrace_hash(notrace_hash);
> list_add(&subops->list, &ops->subop_list);
> return -ENOMEM;
> }
>
> ret = ftrace_update_ops(ops, filter_hash, notrace_hash);
> if (ret < 0)
> list_add(&subops->list, &ops->subop_list);
> free_ftrace_hash(filter_hash);
> free_ftrace_hash(notrace_hash);
> return ret;
> }
OK, so if the list_is_singlar(ops->subop_list), ftrace_graph_enter_ops() is
called and if not, ftrace_graph_enter() is called, right?
Thank you,
>
>
> >
> > > This removes the need to touch the architecture code. It can also be
> > > used by fprobes to handle the attachments to functions for several
> > > different sets of callbacks.
> > >
> > > I'll send out patches soon.
> >
> > OK, I'll wait for that.
>
> I'm just cleaning it up. I'll post it tomorrow (your today).
>
> -- Steve
--
Masami Hiramatsu (Google) <mhiramat@...nel.org>
Powered by blists - more mailing lists