Re: [PATCH] kprobes: avoid crash when rmmod/insmod modules after ftrace_disabled

[yebin <yebin@huaweicloud.com>]

On 2025/12/2 10:32, Masami Hiramatsu (Google) wrote:
> Cc: Steve,
>
> Since this is caused by ftrace_kill.
>
> On Fri, 28 Nov 2025 10:27:42 +0800
> yebin <yebin@huaweicloud.com> wrote:
>
>>
>>
>> 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.
>
> Yes, but I think your patch is redundant. Can you test the code
> which I suggested at the last?
>
Yes, indeed, your solution is simpler and doesn't require writing 
repetitive code.

> BTW, can you explain why that ftrace_disabled was kicked?
> That usually means ftrace hits some bug.
>
Yes, there is a conflict between two modules causing ftrace_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);
>>>
>
> This one, it should fix simply.
>
I tested it and it can solve the problem. This is indeed a good solution 
that reuses the original code.
Do you want me to send a new patch version according to your proposal?
> Thank you,
>
>>>
>>
>
>