Hmm, I also found another problem when probing module with fprobe.
/sys/kernel/tracing # insmod xt_LOG.ko /sys/kernel/tracing # echo 'f:test log_tg*' > dynamic_events /sys/kernel/tracing # echo 1 > events/fprobes/test/enable /sys/kernel/tracing # cat enabled_functions log_tg [xt_LOG] (1) tramp: 0xffffffffa0004000 (fprobe_ftrace_entry+0x0/0x490) ->fprobe_ftrace_entry+0x0/0x490 log_tg_check [xt_LOG] (1) tramp: 0xffffffffa0004000 (fprobe_ftrace_entry+0x0/0x490) ->fprobe_ftrace_entry+0x0/0x490 log_tg_destroy [xt_LOG] (1) tramp: 0xffffffffa0004000 (fprobe_ftrace_entry+0x0/0x490) ->fprobe_ftrace_entry+0x0/0x490 /sys/kernel/tracing # wc -l enabled_functions 3 enabled_functions /sys/kernel/tracing # rmmod xt_LOG /sys/kernel/tracing # wc -l enabled_functions 34085 enabled_functions It seems to reverse the selected functions if the hash is empty... Thanks, On Tue, 7 Apr 2026 19:24:12 +0900 "Masami Hiramatsu (Google)" <[email protected]> wrote: > From: Masami Hiramatsu (Google) <[email protected]> > > Commit 2c67dc457bc6 ("tracing: fprobe: optimization for entry only case") > introduced a different ftrace_ops for entry-only fprobes. > > However, when unregistering an fprobe, the kernel only checks if another > fprobe exists at the same address, without checking which type of fprobe > it is. > If different fprobes are registered at the same address, the same address > will be registered in both fgraph_ops and ftrace_ops, but only one of > them will be deleted when unregistering. (the one removed first will not > be deleted from the ops). > > This results in junk entries remaining in either fgraph_ops or ftrace_ops. > For example: > ======= > cd /sys/kernel/tracing > > # 'Add entry and exit events on the same place' > echo 'f:event1 vfs_read' >> dynamic_events > echo 'f:event2 vfs_read%return' >> dynamic_events > > # 'Enable both of them' > echo 1 > events/fprobes/enable > cat enabled_functions > vfs_read (2) ->arch_ftrace_ops_list_func+0x0/0x210 > > # 'Disable and remove exit event' > echo 0 > events/fprobes/event2/enable > echo -:event2 >> dynamic_events > > # 'Disable and remove all events' > echo 0 > events/fprobes/enable > echo > dynamic_events > > # 'Add another event' > echo 'f:event3 vfs_open%return' > dynamic_events > cat dynamic_events > f:fprobes/event3 vfs_open%return > > echo 1 > events/fprobes/enable > cat enabled_functions > vfs_open (1) tramp: 0xffffffffa0001000 > (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 subops: > {ent:fprobe_fgraph_entry+0x0/0x620 ret:fprobe_return+0x0/0x150} > vfs_read (1) tramp: 0xffffffffa0001000 > (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60 subops: > {ent:fprobe_fgraph_entry+0x0/0x620 ret:fprobe_return+0x0/0x150} > ======= > > As you can see, an entry for the vfs_read remains. > > To fix this issue, when unregistering, the kernel should also check if > there is the same type of fprobes still exist at the same address, and > if not, delete its entry from either fgraph_ops or ftrace_ops. > > Fixes: 2c67dc457bc6 ("tracing: fprobe: optimization for entry only case") > Cc: [email protected] > Signed-off-by: Masami Hiramatsu (Google) <[email protected]> > --- > kernel/trace/fprobe.c | 77 > +++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 62 insertions(+), 15 deletions(-) > > diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c > index dcadf1d23b8a..7f75e6e4462c 100644 > --- a/kernel/trace/fprobe.c > +++ b/kernel/trace/fprobe.c > @@ -85,11 +85,9 @@ static int insert_fprobe_node(struct fprobe_hlist_node > *node) > return rhltable_insert(&fprobe_ip_table, &node->hlist, > fprobe_rht_params); > } > > -/* Return true if there are synonims */ > -static bool delete_fprobe_node(struct fprobe_hlist_node *node) > +static void delete_fprobe_node(struct fprobe_hlist_node *node) > { > lockdep_assert_held(&fprobe_mutex); > - bool ret; > > /* Avoid double deleting */ > if (READ_ONCE(node->fp) != NULL) { > @@ -97,13 +95,6 @@ static bool delete_fprobe_node(struct fprobe_hlist_node > *node) > rhltable_remove(&fprobe_ip_table, &node->hlist, > fprobe_rht_params); > } > - > - rcu_read_lock(); > - ret = !!rhltable_lookup(&fprobe_ip_table, &node->addr, > - fprobe_rht_params); > - rcu_read_unlock(); > - > - return ret; > } > > /* Check existence of the fprobe */ > @@ -337,6 +328,32 @@ static bool fprobe_is_ftrace(struct fprobe *fp) > return !fp->exit_handler; > } > > +static bool fprobe_exists_on_hash(unsigned long ip, bool ftrace) > +{ > + struct rhlist_head *head, *pos; > + struct fprobe_hlist_node *node; > + struct fprobe *fp; > + > + guard(rcu)(); > + head = rhltable_lookup(&fprobe_ip_table, &ip, > + fprobe_rht_params); > + if (!head) > + return false; > + /* We have to check the same type on the list. */ > + rhl_for_each_entry_rcu(node, pos, head, hlist) { > + if (node->addr != ip) > + break; > + fp = READ_ONCE(node->fp); > + if (likely(fp)) { > + if ((!ftrace && fp->exit_handler) || > + (ftrace && !fp->exit_handler)) > + return true; > + } > + } > + > + return false; > +} > + > #ifdef CONFIG_MODULES > static void fprobe_set_ips(unsigned long *ips, unsigned int cnt, int remove, > int reset) > @@ -360,6 +377,29 @@ static bool fprobe_is_ftrace(struct fprobe *fp) > return false; > } > > +static bool fprobe_exists_on_hash(unsigned long ip, bool ftrace > __maybe_unused) > +{ > + struct rhlist_head *head, *pos; > + struct fprobe_hlist_node *node; > + struct fprobe *fp; > + > + guard(rcu)(); > + head = rhltable_lookup(&fprobe_ip_table, &ip, > + fprobe_rht_params); > + if (!head) > + return false; > + /* We only need to check fp is there. */ > + rhl_for_each_entry_rcu(node, pos, head, hlist) { > + if (node->addr != ip) > + break; > + fp = READ_ONCE(node->fp); > + if (likely(fp)) > + return true; > + } > + > + return false; > +} > + > #ifdef CONFIG_MODULES > static void fprobe_set_ips(unsigned long *ips, unsigned int cnt, int remove, > int reset) > @@ -574,15 +614,20 @@ static int fprobe_addr_list_add(struct fprobe_addr_list > *alist, unsigned long ad > static void fprobe_remove_node_in_module(struct module *mod, struct > fprobe_hlist_node *node, > struct fprobe_addr_list *alist) > { > + lockdep_assert_in_rcu_read_lock(); > + > if (!within_module(node->addr, mod)) > return; > - if (delete_fprobe_node(node)) > - return; > + > + delete_fprobe_node(node); > /* > - * If failed to update alist, just continue to update hlist. > + * Ignore failure of updating alist, but continue to update hlist. > * Therefore, at list user handler will not hit anymore. > + * And don't care the type here, because all fprobes on the same > + * address must be removed eventually. > */ > - fprobe_addr_list_add(alist, node->addr); > + if (!rhltable_lookup(&fprobe_ip_table, &node->addr, fprobe_rht_params)) > + fprobe_addr_list_add(alist, node->addr); > } > > /* Handle module unloading to manage fprobe_ip_table. */ > @@ -943,7 +988,9 @@ int unregister_fprobe(struct fprobe *fp) > /* Remove non-synonim ips from table and hash */ > count = 0; > for (i = 0; i < hlist_array->size; i++) { > - if (!delete_fprobe_node(&hlist_array->array[i])) > + delete_fprobe_node(&hlist_array->array[i]); > + if (!fprobe_exists_on_hash(hlist_array->array[i].addr, > + fprobe_is_ftrace(fp))) > addrs[count++] = hlist_array->array[i].addr; > } > del_fprobe_hash(fp); > -- Masami Hiramatsu (Google) <[email protected]>
