linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: yebin <yebin@huaweicloud.com>
To: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Cc: naveen@kernel.org, davem@davemloft.net,
	linux-trace-kernel@vger.kernel.org, yebin10@huawei.com
Subject: Re: [PATCH] kprobes: avoid crash when rmmod/insmod modules after ftrace_disabled
Date: Fri, 28 Nov 2025 10:27:42 +0800	[thread overview]
Message-ID: <6929089E.10706@huaweicloud.com> (raw)
In-Reply-To: <20251127131820.152fd7651ece26139778cc25@kernel.org>



On 2025/11/27 12:18, Masami Hiramatsu (Google) wrote:
> On Thu, 27 Nov 2025 12:52:48 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
>
>> Hi,
>>
>> Thanks for reporting!
>>
>>
>> On Tue, 25 Nov 2025 10:05:36 +0800
>> Ye Bin <yebin@huaweicloud.com> wrote:
>>
>>> From: Ye Bin <yebin10@huawei.com>
>>>
>>> There's a issue as follows when rmmod modules after ftrace disabled:
>>
>> You may see something like;
>>
>> Failed to unregister kprobe-ftrace (error -19)
>>
>> or
>>
>> Failed to disarm kprobe-ftrace at <function name> (error -19)
>>
>> right before this BUG, don't you?
>> If you reported with that line, it's more easier to understand.
>>
Yes, there is indeed a warning generated. I might not have expressed it 
clearly enough. The issue below is related to the problem that occurs 
when the second module is unloaded. When the first module was unloaded, 
some nodes were left in the hash list, causing a use-after-free (UAF) 
issue when traversing the hash list.
Therefore, this patch aims to resolve the UAF problem caused by residual 
nodes in the hash list after unloading a module while ftrace is disabled.
>>
>>> BUG: unable to handle page fault for address: fffffbfff805000d
>>> PGD 817fcc067 P4D 817fcc067 PUD 817fc8067 PMD 101555067 PTE 0
>>> Oops: Oops: 0000 [#1] SMP KASAN PTI
>>> CPU: 4 UID: 0 PID: 2012 Comm: rmmod Tainted: G        W  OE
>>> Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
>>> RIP: 0010:kprobes_module_callback+0x89/0x790
>>> RSP: 0018:ffff88812e157d30 EFLAGS: 00010a02
>>> RAX: 1ffffffff805000d RBX: dffffc0000000000 RCX: ffffffff86a8de90
>>> RDX: ffffed1025c2af9b RSI: 0000000000000008 RDI: ffffffffc0280068
>>> RBP: 0000000000000000 R08: 0000000000000001 R09: ffffed1025c2af9a
>>> R10: ffff88812e157cd7 R11: 205d323130325420 R12: 0000000000000002
>>> R13: ffffffffc0290488 R14: 0000000000000002 R15: ffffffffc0280040
>>> FS:  00007fbc450dd740(0000) GS:ffff888420331000(0000) knlGS:0000000000000000
>>> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> CR2: fffffbfff805000d CR3: 000000010f624000 CR4: 00000000000006f0
>>> Call Trace:
>>>   <TASK>
>>>   notifier_call_chain+0xc6/0x280
>>>   blocking_notifier_call_chain+0x60/0x90
>>>   __do_sys_delete_module.constprop.0+0x32a/0x4e0
>>>   do_syscall_64+0x5d/0xfa0
>>>   entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>>
>>> The above issue occurs because the kprobe was not removed from the hash
>>> list after ftrace_disable.
>>> To prevent the system from restarting unexpectedly after ftrace_disable,
>>> in such cases, unregister_kprobe() ensures that the probe is removed from
>>> the hash list, preventing subsequent access to already freed memory.
>>>
>>> Fixes: 6f0f1dd71953 ("kprobes: Cleanup disabling and unregistering path")
>>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>>> ---
>>>   kernel/kprobes.c | 26 ++++++++++++++++++++++++--
>>>   1 file changed, 24 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
>>> index ab8f9fc1f0d1..d735a608b810 100644
>>> --- a/kernel/kprobes.c
>>> +++ b/kernel/kprobes.c
>>> @@ -1731,8 +1731,30 @@ static int __unregister_kprobe_top(struct kprobe *p)
>>>
>>>   	/* Disable kprobe. This will disarm it if needed. */
>>>   	ap = __disable_kprobe(p);
>>> -	if (IS_ERR(ap))
>>> -		return PTR_ERR(ap);
>>> +	if (IS_ERR(ap)) {
>>> +		int ret = PTR_ERR(ap);
>>> +
>>> +		/*
>>> +		 * If ftrace disabled we need to delete kprobe node from
>>> +		 * hlist or aggregation list. If nodes are not removed when
>>> +		 * modules are removed, the already released nodes will
>>> +		 * remain in the linked list. Subsequent access to the
>>> +		 * linked list may then trigger exceptions.
>>> +		 */
>>> +		if (ret != -ENODEV)
>>> +			return ret;
>>> +
>>> +		ap = __get_valid_kprobe(p);
>>> +		if (!ap)
>>> +			return ret;
>>> +
>>> +		if (ap == p)
>>> +			hlist_del_rcu(&ap->hlist);
>>> +		else
>>> +			list_del_rcu(&p->list);
>>
>> Instead of repeating this process, we should ignore
>> -ENODEV error from ftrace directly. BTW, ftrace_disabled is set
>> when ftrace_kill() is called, that means ftrace is no more usable.
>> So I think we can just ignore ftrace operation in
>> __disarm_kprobe_ftrace().
>
> So, what we need is;
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index ab8f9fc1f0d1..17d451553389 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1104,6 +1104,10 @@ static int __disarm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
>   	int ret;
>
>   	lockdep_assert_held(&kprobe_mutex);
> +	if (unlikely(kprobe_ftrace_disabled)) {
> +		/* Now ftrace is disabled forever, disarm is already done. */
> +		return 0;
> +	}
>
>   	if (*cnt == 1) {
>   		ret = unregister_ftrace_function(ops);
>
>


  reply	other threads:[~2025-11-28  2:27 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-25  2:05 [PATCH] kprobes: avoid crash when rmmod/insmod modules after ftrace_disabled Ye Bin
2025-11-27  3:52 ` Masami Hiramatsu
2025-11-27  4:18   ` Masami Hiramatsu
2025-11-28  2:27     ` yebin [this message]
2025-12-02  2:32       ` Masami Hiramatsu
2025-12-03  2:37         ` yebin
2025-12-03  4:51           ` Masami Hiramatsu
2025-12-02 16:43       ` Steven Rostedt
2025-12-03  3:21         ` yebin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6929089E.10706@huaweicloud.com \
    --to=yebin@huaweicloud.com \
    --cc=davem@davemloft.net \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=naveen@kernel.org \
    --cc=yebin10@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).