From nobody Mon Jun 15 12:22:32 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EE0193AE196; Fri, 10 Apr 2026 09:35:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775813723; cv=none; b=Mj+493yBKHb9VJDILunXvUsid0c7TUSR/0e2NbK5YrK881Qf4UGZN1jjtc1O/TwpGx6XElIQzlTExOs1u0i6RcSKH7LVi0TFb/5tlv7KnLdC9QLmaT0xz6HD+rgT7wrkKyaURRcAXTp2tLq8pPSCHmRB/INmEb60GcGu2aiq1r8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775813723; c=relaxed/simple; bh=MdXE+yvLBVQbxyHx+/lHgZ8yQWyUyvXtTgoKmqmIuGc=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=gpsGjK0h9LG49PG9RuySFWJuHx2yyOvLnZCHoJ3i7CErm44NM382BLhOrNjmaguyWnYJa8Cm3Rl/sT8l9Fi+FfynnehIHSSAlre1SGOkK48enl4uxl2VsNA0F/cgdGi4SuOzZlnhfZWk/L/D71o74MPH/6Qy0TvRD5XE1/m3Pa8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=sxiMfgwA; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="sxiMfgwA" Received: by smtp.kernel.org (Postfix) with ESMTPSA id AA43EC19421; Fri, 10 Apr 2026 09:35:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1775813722; bh=MdXE+yvLBVQbxyHx+/lHgZ8yQWyUyvXtTgoKmqmIuGc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sxiMfgwA0E3HoPcEFK9Tf1hJK6EGqVPJVeranIvyK2VBOdWJ4EQA03rqkVwkgmSPV S6gL3dUnPBWHIzixQAGc4LyVmfnadhmwXzIezKBsBGzZvogRem9H0yVBb9eG/jAdFX FOa7ZoKxbcIbZXG0FsutN93uzGGPGl7H4Qk/jCJdW83YLKAfTyAi6/8loWv6y6gHKQ bMIG/bgFaS2/mBlmeORCoC0DWG6aA8JqfpcAldl+jumX9Qnq19Rvd3CEQ28nLqEJrq z+GDi0oUE4ayIR2ocXyJx/7kG4KsC0zKeMsImutlhSRA2zYCzjOMZ0so3bhi7/6xHq 7wwGTm2O5SQ1A== From: "Masami Hiramatsu (Google)" To: Steven Rostedt , Masami Hiramatsu Cc: Menglong Dong , Mathieu Desnoyers , jiang.biao@linux.dev, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org Subject: [PATCH v3 1/3] tracing/fprobe: Remove fprobe from hash in failure path Date: Fri, 10 Apr 2026 18:35:20 +0900 Message-ID: <177581371999.617881.16151200432846312505.stgit@mhiramat.tok.corp.google.com> X-Mailer: git-send-email 2.53.0.1213.gd9a14994de-goog In-Reply-To: <177581370903.617881.3002655215679528157.stgit@mhiramat.tok.corp.google.com> References: <177581370903.617881.3002655215679528157.stgit@mhiramat.tok.corp.google.com> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable From: Masami Hiramatsu (Google) When register_fprobe_ips() fails, it tries to remove a list of fprobe_hash_node from fprobe_ip_table, but it missed to remove fprobe itself from fprobe_table. Moreover, when removing the fprobe_hash_node which is added to rhltable once, it must use kfree_rcu() after removing from rhltable. To fix these issues, this reuses unregister_fprobe() internal code to rollback the half-way registered fprobe. Fixes: 4346ba160409 ("fprobe: Rewrite fprobe on function-graph tracer") Signed-off-by: Masami Hiramatsu (Google) --- Changes in v3: - Newly added. --- kernel/trace/fprobe.c | 73 ++++++++++++++++++++++++++-------------------= ---- 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c index 56d145017902..1b5c47685186 100644 --- a/kernel/trace/fprobe.c +++ b/kernel/trace/fprobe.c @@ -4,6 +4,7 @@ */ #define pr_fmt(fmt) "fprobe: " fmt =20 +#include #include #include #include @@ -821,6 +822,8 @@ int register_fprobe(struct fprobe *fp, const char *filt= er, const char *notfilter } EXPORT_SYMBOL_GPL(register_fprobe); =20 +static int unregister_fprobe_nolock(struct fprobe *fp); + /** * register_fprobe_ips() - Register fprobe to ftrace by address. * @fp: A fprobe data structure to be registered. @@ -845,30 +848,29 @@ int register_fprobe_ips(struct fprobe *fp, unsigned l= ong *addrs, int num) =20 mutex_lock(&fprobe_mutex); =20 - hlist_array =3D fp->hlist_array; if (fprobe_is_ftrace(fp)) ret =3D fprobe_ftrace_add_ips(addrs, num); else ret =3D fprobe_graph_add_ips(addrs, num); + if (ret) { + fprobe_fail_cleanup(fp); + return ret; + } =20 if (!ret) { + hlist_array =3D fp->hlist_array; add_fprobe_hash(fp); for (i =3D 0; i < hlist_array->size; i++) { ret =3D insert_fprobe_node(&hlist_array->array[i]); - if (ret) + if (ret) { + hlist_array->size =3D i; + unregister_fprobe_nolock(fp); break; - } - /* fallback on insert error */ - if (ret) { - for (i--; i >=3D 0; i--) - delete_fprobe_node(&hlist_array->array[i]); + } } } mutex_unlock(&fprobe_mutex); =20 - if (ret) - fprobe_fail_cleanup(fp); - return ret; } EXPORT_SYMBOL_GPL(register_fprobe_ips); @@ -911,32 +913,21 @@ bool fprobe_is_registered(struct fprobe *fp) return true; } =20 -/** - * unregister_fprobe() - Unregister fprobe. - * @fp: A fprobe data structure to be unregistered. - * - * Unregister fprobe (and remove ftrace hooks from the function entries). - * - * Return 0 if @fp is unregistered successfully, -errno if not. - */ -int unregister_fprobe(struct fprobe *fp) +static int unregister_fprobe_nolock(struct fprobe *fp) { - struct fprobe_hlist *hlist_array; + struct fprobe_hlist *hlist_array =3D fp->hlist_array; unsigned long *addrs =3D NULL; - int ret =3D 0, i, count; + int i, count; =20 - mutex_lock(&fprobe_mutex); - if (!fp || !is_fprobe_still_exist(fp)) { - ret =3D -EINVAL; - goto out; + if (!hlist_array || !hlist_array->size) { + del_fprobe_hash(fp); + fprobe_fail_cleanup(fp); + return 0; } =20 - hlist_array =3D fp->hlist_array; addrs =3D kcalloc(hlist_array->size, sizeof(unsigned long), GFP_KERNEL); - if (!addrs) { - ret =3D -ENOMEM; /* TODO: Fallback to one-by-one loop */ - goto out; - } + if (!addrs) + return -ENOMEM; /* TODO: Fallback to one-by-one loop */ =20 /* Remove non-synonim ips from table and hash */ count =3D 0; @@ -953,12 +944,26 @@ int unregister_fprobe(struct fprobe *fp) =20 kfree_rcu(hlist_array, rcu); fp->hlist_array =3D NULL; + kfree(addrs); =20 -out: - mutex_unlock(&fprobe_mutex); + return 0; +} =20 - kfree(addrs); - return ret; +/** + * unregister_fprobe() - Unregister fprobe. + * @fp: A fprobe data structure to be unregistered. + * + * Unregister fprobe (and remove ftrace hooks from the function entries). + * + * Return 0 if @fp is unregistered successfully, -errno if not. + */ +int unregister_fprobe(struct fprobe *fp) +{ + guard(mutex)(&fprobe_mutex); + if (!fp || !is_fprobe_still_exist(fp)) + return -EINVAL; + + return unregister_fprobe_nolock(fp); } EXPORT_SYMBOL_GPL(unregister_fprobe); From nobody Mon Jun 15 12:22:32 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DD7383BC67D; Fri, 10 Apr 2026 09:35:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775813730; cv=none; b=HpLTtqJNIT9x6y36xsQPhSSHti6ynKxhGMDyFeYXvQFhUemuQ75BeeEtfeAyq391BZu+Fu4dQJ7uw2vPUgMw0LL+HpGRtlVFBEjV2lHSqLKCd5nCQ1k4qqO3WxdlpGluCfss1vqVSO/fis4IiqZBOsgjxRZNGM4y0YwF3P0kZUY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775813730; c=relaxed/simple; bh=kXDIXToLNoulNFdH94bVGq1GTn3IXpilnT3y0/Hm9Ig=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=en3MRVoC5Plk7xg4h1qSkhftMU0VLlpSF7damupy7L0DAK208SWaX8ZPz0q/w0jc/RV5LX7GzJ7dd9RFDCHdX06v8Jsd+8B/1MZANTVTqssDicccM+o4eumbmii/eptHz2+/2SSxv72s40IJ0BBsWzJMp9pFqc8QkV3np6L0aD8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=rcLukNaL; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="rcLukNaL" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 93AA7C19421; Fri, 10 Apr 2026 09:35:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1775813730; bh=kXDIXToLNoulNFdH94bVGq1GTn3IXpilnT3y0/Hm9Ig=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=rcLukNaLAdq9zZ76Z6NGlEMgsdpgvI9fjkJm/5M00OvhCRxsoxtS4O1LJbpWKXv8D 3zf6GZWvFRWcPCh74X3TjhRRIUFJab/KxLO/PZzZxCNJcMfFnISWdJr+ukmpkrS+7p PBHZLA8Oqoyxwm5A1Zh/orDjlORzyUXyH8kIQl/3DnImfxAFi4YODoFSRRyiJRa3Yu 8bD9g5xk31qTGf9hwWduNMccHqFIHtAiNbt1UiUyLsInc1P0wJIzQ3j2S2EBwbL7a3 KOkJLJi4gylIFlsHjAW2cT/4Wi11MV//tFL6IAfOvLR/SU/4IQKPUHaxLMqrDnAmxB IIFOM55uEQ4Tw== From: "Masami Hiramatsu (Google)" To: Steven Rostedt , Masami Hiramatsu Cc: Menglong Dong , Mathieu Desnoyers , jiang.biao@linux.dev, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org Subject: [PATCH v3 2/3] tracing/fprobe: Avoid kcalloc() in rcu_read_lock section Date: Fri, 10 Apr 2026 18:35:28 +0900 Message-ID: <177581372792.617881.3541285555394646169.stgit@mhiramat.tok.corp.google.com> X-Mailer: git-send-email 2.53.0.1213.gd9a14994de-goog In-Reply-To: <177581370903.617881.3002655215679528157.stgit@mhiramat.tok.corp.google.com> References: <177581370903.617881.3002655215679528157.stgit@mhiramat.tok.corp.google.com> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable From: Masami Hiramatsu (Google) fprobe_remove_node_in_module() is called under RCU read locked, but this invokes kcalloc() if there are more than 8 fprobes installed on the module. Sashiko warns it because kcalloc() can sleep [1]. [1] https://sashiko.dev/#/patchset/177552432201.853249.5125045538812833325= .stgit%40mhiramat.tok.corp.google.com To fix this issue, expand the batch size to 128 and do not expand the fprobe_addr_list, but just cancel walking on fprobe_ip_table, update fgraph/ftrace_ops and retry the loop again. Fixes: 0de4c70d04a4 ("tracing: fprobe: use rhltable for fprobe_ip_table") Cc: stable@vger.kernel.org Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Retry inside rhltable_walk_enter/exit(). - Rename fprobe_set_ips() to fprobe_remove_ips(). - Rename 'retry' label to 'again'. --- kernel/trace/fprobe.c | 75 ++++++++++++++++++++-------------------------= ---- 1 file changed, 30 insertions(+), 45 deletions(-) diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c index 1b5c47685186..46357808cbde 100644 --- a/kernel/trace/fprobe.c +++ b/kernel/trace/fprobe.c @@ -339,11 +339,10 @@ static bool fprobe_is_ftrace(struct fprobe *fp) } =20 #ifdef CONFIG_MODULES -static void fprobe_set_ips(unsigned long *ips, unsigned int cnt, int remov= e, - int reset) +static void fprobe_remove_ips(unsigned long *ips, unsigned int cnt) { - ftrace_set_filter_ips(&fprobe_graph_ops.ops, ips, cnt, remove, reset); - ftrace_set_filter_ips(&fprobe_ftrace_ops, ips, cnt, remove, reset); + ftrace_set_filter_ips(&fprobe_graph_ops.ops, ips, cnt, 1, 0); + ftrace_set_filter_ips(&fprobe_ftrace_ops, ips, cnt, 1, 0); } #endif #else @@ -362,10 +361,9 @@ static bool fprobe_is_ftrace(struct fprobe *fp) } =20 #ifdef CONFIG_MODULES -static void fprobe_set_ips(unsigned long *ips, unsigned int cnt, int remov= e, - int reset) +static void fprobe_set_ips(unsigned long *ips, unsigned int cnt) { - ftrace_set_filter_ips(&fprobe_graph_ops.ops, ips, cnt, remove, reset); + ftrace_set_filter_ips(&fprobe_graph_ops.ops, ips, cnt, 1, 0); } #endif #endif /* !CONFIG_DYNAMIC_FTRACE_WITH_ARGS && !CONFIG_DYNAMIC_FTRACE_WITH_= REGS */ @@ -537,7 +535,7 @@ static void fprobe_graph_remove_ips(unsigned long *addr= s, int num) =20 #ifdef CONFIG_MODULES =20 -#define FPROBE_IPS_BATCH_INIT 8 +#define FPROBE_IPS_BATCH_INIT 128 /* instruction pointer address list */ struct fprobe_addr_list { int index; @@ -545,45 +543,21 @@ struct fprobe_addr_list { unsigned long *addrs; }; =20 -static int fprobe_addr_list_add(struct fprobe_addr_list *alist, unsigned l= ong addr) +static int fprobe_remove_node_in_module(struct module *mod, struct fprobe_= hlist_node *node, + struct fprobe_addr_list *alist) { - unsigned long *addrs; - - /* Previously we failed to expand the list. */ - if (alist->index =3D=3D alist->size) - return -ENOSPC; - - alist->addrs[alist->index++] =3D addr; - if (alist->index < alist->size) + if (!within_module(node->addr, mod)) return 0; =20 - /* Expand the address list */ - addrs =3D kcalloc(alist->size * 2, sizeof(*addrs), GFP_KERNEL); - if (!addrs) - return -ENOMEM; - - memcpy(addrs, alist->addrs, alist->size * sizeof(*addrs)); - alist->size *=3D 2; - kfree(alist->addrs); - alist->addrs =3D addrs; + if (delete_fprobe_node(node)) + return 0; =20 + alist->addrs[alist->index++] =3D node->addr; + if (alist->index =3D=3D alist->size) + return -ENOSPC; return 0; } =20 -static void fprobe_remove_node_in_module(struct module *mod, struct fprobe= _hlist_node *node, - struct fprobe_addr_list *alist) -{ - if (!within_module(node->addr, mod)) - return; - if (delete_fprobe_node(node)) - return; - /* - * If failed to update alist, just continue to update hlist. - * Therefore, at list user handler will not hit anymore. - */ - fprobe_addr_list_add(alist, node->addr); -} - /* Handle module unloading to manage fprobe_ip_table. */ static int fprobe_module_callback(struct notifier_block *nb, unsigned long val, void *data) @@ -592,6 +566,7 @@ static int fprobe_module_callback(struct notifier_block= *nb, struct fprobe_hlist_node *node; struct rhashtable_iter iter; struct module *mod =3D data; + bool retry; =20 if (val !=3D MODULE_STATE_GOING) return NOTIFY_DONE; @@ -603,18 +578,28 @@ static int fprobe_module_callback(struct notifier_blo= ck *nb, =20 mutex_lock(&fprobe_mutex); rhltable_walk_enter(&fprobe_ip_table, &iter); +again: + retry =3D false; + alist.index =3D 0; do { rhashtable_walk_start(&iter); =20 while ((node =3D rhashtable_walk_next(&iter)) && !IS_ERR(node)) - fprobe_remove_node_in_module(mod, node, &alist); + if (fprobe_remove_node_in_module(mod, node, &alist) < 0) { + retry =3D true; + break; + } =20 rhashtable_walk_stop(&iter); - } while (node =3D=3D ERR_PTR(-EAGAIN)); - rhashtable_walk_exit(&iter); + } while (node =3D=3D ERR_PTR(-EAGAIN) && !retry); + /* Remove any ips from hash table(s) */ + if (alist.index > 0) { + fprobe_remove_ips(alist.addrs, alist.index); + if (retry) + goto again; + } =20 - if (alist.index > 0) - fprobe_set_ips(alist.addrs, alist.index, 1, 0); + rhashtable_walk_exit(&iter); mutex_unlock(&fprobe_mutex); =20 kfree(alist.addrs); From nobody Mon Jun 15 12:22:32 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BBB5E3B52E2; Fri, 10 Apr 2026 09:35:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775813738; cv=none; b=b1xcfGOkDI8ujvrCrLnWee5kMhHoL/SWw5e3no1UTZc8AS2xNXB1uvh1EnZh/1I1/pB7c7h2RsApMX0UJdkbkDZGcD5ESvFRQToMKyJrh1cMkueKFk3+BTeTOcy0P28I264bCBYUcQdKDM34SX3nA+onI/Lvug8pf+s/tctDLng= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775813738; c=relaxed/simple; bh=PesYormxSzyPqoir4DFNN8L3FSOH52CALEnHY2eUv04=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=i1/NDRzKFkxJWpZqC2a16/4ZVcAq4CLLjZxp+c0UyrLJr3S1ZgdtQAocU6e5/m3iC0OnlvrYxeaSjh17HiIjQpRF1SOiUoBuR5NR601N7RmqWl2hTdvk/khaliUJl6DhqjMlfB1tLoGQuNR8Rdb+KBSrOc08TQ1aynTbDg5rxGM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=LlNVzkeD; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="LlNVzkeD" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 749A7C19421; Fri, 10 Apr 2026 09:35:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1775813738; bh=PesYormxSzyPqoir4DFNN8L3FSOH52CALEnHY2eUv04=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LlNVzkeDR7AviyyfxAWN17hnJ31bQpEKX6mVWCzCl0/a3fli5lBuIrmMegENDwMLf 5yFnrazJ3l8jxUuurlHWGg6arq7/v2HEel7mUSCskcDBDNotuuE2bdE3YUNa7fX0iX a9ZqwSXAAsfXaDrF0Jtf2wx0MqVRmH4YEhMkxntFJt0tBId1aEu0Yo9ioGH216I3vT T2JXVJKkhBmLPouTOqnqpgC2k3IGlZyMTdGpFQqUkk5sdW/vkayFUb1E7EIkvu3Kjt jaEoV6cJpM5KtXGA6ddbll0LtJh+xuS+4lKCFE/+XzSKqRqRcFDRCQWb6TFRb4hxkZ UGr5JRKNLSSJQ== From: "Masami Hiramatsu (Google)" To: Steven Rostedt , Masami Hiramatsu Cc: Menglong Dong , Mathieu Desnoyers , jiang.biao@linux.dev, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org Subject: [PATCH v3 3/3] tracing/fprobe: Check the same type fprobe on table as the unregistered one Date: Fri, 10 Apr 2026 18:35:36 +0900 Message-ID: <177581373580.617881.355647055040227134.stgit@mhiramat.tok.corp.google.com> X-Mailer: git-send-email 2.53.0.1213.gd9a14994de-goog In-Reply-To: <177581370903.617881.3002655215679528157.stgit@mhiramat.tok.corp.google.com> References: <177581370903.617881.3002655215679528157.stgit@mhiramat.tok.corp.google.com> User-Agent: StGit/0.19 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable From: Masami Hiramatsu (Google) 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: =3D=3D=3D=3D=3D=3D=3D 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/0x= 60) ->ftrace_graph_func+0x0/0x60 subops: {ent:fprobe_fgraph_entry+0x0/0x= 620 ret:fprobe_return+0x0/0x150} vfs_read (1) tramp: 0xffffffffa0001000 (ftrace_graph_func+0x0/0x= 60) ->ftrace_graph_func+0x0/0x60 subops: {ent:fprobe_fgraph_entry+0x0/0x= 620 ret:fprobe_return+0x0/0x150} =3D=3D=3D=3D=3D=3D=3D 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: stable@vger.kernel.org Signed-off-by: Masami Hiramatsu (Google) --- kernel/trace/fprobe.c | 81 +++++++++++++++++++++++++++++++++++++++------= ---- 1 file changed, 65 insertions(+), 16 deletions(-) diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c index 46357808cbde..7f7daf2c49dd 100644 --- a/kernel/trace/fprobe.c +++ b/kernel/trace/fprobe.c @@ -86,11 +86,9 @@ static int insert_fprobe_node(struct fprobe_hlist_node *= node) return rhltable_insert(&fprobe_ip_table, &node->hlist, fprobe_rht_params); } =20 -/* 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; =20 /* Avoid double deleting */ if (READ_ONCE(node->fp) !=3D NULL) { @@ -98,13 +96,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 =3D !!rhltable_lookup(&fprobe_ip_table, &node->addr, - fprobe_rht_params); - rcu_read_unlock(); - - return ret; } =20 /* Check existence of the fprobe */ @@ -338,6 +329,32 @@ static bool fprobe_is_ftrace(struct fprobe *fp) return !fp->exit_handler; } =20 +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 =3D 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 !=3D ip) + break; + fp =3D 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_remove_ips(unsigned long *ips, unsigned int cnt) { @@ -360,6 +377,29 @@ static bool fprobe_is_ftrace(struct fprobe *fp) return false; } =20 +static bool fprobe_exists_on_hash(unsigned long ip, bool ftrace __maybe_un= used) +{ + struct rhlist_head *head, *pos; + struct fprobe_hlist_node *node; + struct fprobe *fp; + + guard(rcu)(); + head =3D 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 !=3D ip) + break; + fp =3D 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) { @@ -546,15 +586,22 @@ struct fprobe_addr_list { static int 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 0; =20 - if (delete_fprobe_node(node)) - return 0; + delete_fprobe_node(node); + /* + * Don't care the type here, because all fprobes on the same + * address must be removed eventually. + */ + if (!rhltable_lookup(&fprobe_ip_table, &node->addr, fprobe_rht_params)) { + alist->addrs[alist->index++] =3D node->addr; + if (alist->index =3D=3D alist->size) + return -ENOSPC; + } =20 - alist->addrs[alist->index++] =3D node->addr; - if (alist->index =3D=3D alist->size) - return -ENOSPC; return 0; } =20 @@ -917,7 +964,9 @@ static int unregister_fprobe_nolock(struct fprobe *fp) /* Remove non-synonim ips from table and hash */ count =3D 0; for (i =3D 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++] =3D hlist_array->array[i].addr; } del_fprobe_hash(fp);