[PATCH 0/2] fix UAF when lookup kallsym after ftrace disabled

[PATCH 0/2] fix UAF when lookup kallsym after ftrace disabled

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

Ye Bin (2):
  ftrace: fix UAF when lookup kallsym after ftrace disabled
  ftrace: don't allocate  ftrace module map

 kernel/trace/ftrace.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH 1/2] ftrace: fix UAF when lookup kallsym after ftrace disabled

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

There's issue as follows:
BUG: unable to handle page fault for address: ffffffffc05d0218
PGD 1bd66f067 P4D 1bd66f067 PUD 1bd671067 PMD 101808067 PTE 0
Oops: Oops: 0000 [#1] SMP KASAN PTI
Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
RIP: 0010:sized_strscpy+0x81/0x2f0
RSP: 0018:ffff88812d76fa08 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffffffffc0601010 RCX: dffffc0000000000
RDX: 0000000000000038 RSI: dffffc0000000000 RDI: ffff88812608da2d
RBP: 8080808080808080 R08: ffff88812608da2d R09: ffff88812608da68
R10: ffff88812608d82d R11: ffff88812608d810 R12: 0000000000000038
R13: ffff88812608da2d R14: ffffffffc05d0218 R15: fefefefefefefeff
FS:  00007fef552de740(0000) GS:ffff8884251c7000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffc05d0218 CR3: 00000001146f0000 CR4: 00000000000006f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 ftrace_mod_get_kallsym+0x1ac/0x590
 update_iter_mod+0x239/0x5b0
 s_next+0x5b/0xa0
 seq_read_iter+0x8c9/0x1070
 seq_read+0x249/0x3b0
 proc_reg_read+0x1b0/0x280
 vfs_read+0x17f/0x920
 ksys_read+0xf3/0x1c0
 do_syscall_64+0x5f/0x2e0
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

Above issue may happens as follow:
(1) Add kprobe trace point;
(2) insmod test.ko;
(3) Trigger ftrace disabled;
(4) rmmod test.ko;
(5) cat /proc/kallsyms; --> Will trigger UAF as test.ko already removed;
ftrace_mod_get_kallsym()
...
strscpy(module_name, mod_map->mod->name, MODULE_NAME_LEN);
...

As ftrace_release_mod() judge 'ftrace_disabled' is true will return, and
'mod_map' will remaining in ftrace_mod_maps. 'mod_map' has no chance to
release. Therefore, this also causes residual resources to accumulate.
To solve above issue, unconditionally clean up'mod_map'.

Fixes: aba4b5c22cba ("ftrace: Save module init functions kallsyms symbols for tracing")
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 kernel/trace/ftrace.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index a3d4dfad0cbc..ff5d9d73a4a7 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -7438,9 +7438,6 @@ void ftrace_release_mod(struct module *mod)
 
 	mutex_lock(&ftrace_lock);
 
-	if (ftrace_disabled)
-		goto out_unlock;
-
 	list_for_each_entry_safe(mod_map, n, &ftrace_mod_maps, list) {
 		if (mod_map->mod == mod) {
 			list_del_rcu(&mod_map->list);
-- 
2.34.1

[PATCH 2/2] ftrace: don't allocate ftrace module map

[Ye Bin]

From: Ye Bin <yebin10@huawei.com>

If the ftrace is disabled, it is meaningless to allocate  module map.
So add check in ftrace_free_mem().

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 kernel/trace/ftrace.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index ff5d9d73a4a7..56adf45de92e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -7824,6 +7824,11 @@ void ftrace_free_mem(struct module *mod, void *start_ptr, void *end_ptr)
 
 	mutex_lock(&ftrace_lock);
 
+	if (ftrace_disabled || (mod && !mod->num_ftrace_callsites)) {
+		mutex_unlock(&ftrace_lock);
+		return;
+	}
+
 	/*
 	 * If we are freeing module init memory, then check if
 	 * any tracer is active. If so, we need to save a mapping of
-- 
2.34.1

Re: [PATCH 1/2] ftrace: fix UAF when lookup kallsym after ftrace disabled

[Steven Rostedt]

On Fri, 23 May 2025 16:39:44 +0800
Ye Bin <yebin@huaweicloud.com> wrote:

> Above issue may happens as follow:
> (1) Add kprobe trace point;
> (2) insmod test.ko;
> (3) Trigger ftrace disabled;

This is the bug. How was ftrace_disabled triggered? That should never
happen. Was test.ko buggy?

> (4) rmmod test.ko;
> (5) cat /proc/kallsyms; --> Will trigger UAF as test.ko already removed;
> ftrace_mod_get_kallsym()
> ...
> strscpy(module_name, mod_map->mod->name, MODULE_NAME_LEN);
> ...
> 
> As ftrace_release_mod() judge 'ftrace_disabled' is true will return, and
> 'mod_map' will remaining in ftrace_mod_maps. 'mod_map' has no chance to
> release. Therefore, this also causes residual resources to accumulate.
> To solve above issue, unconditionally clean up'mod_map'.
> 
> Fixes: aba4b5c22cba ("ftrace: Save module init functions kallsyms symbols for tracing")

This is *not* a fix. ftrace_disabled gets set when a bug is triggered. If
this prevents ftrace_disabled from getting set, then it would be a fix. But
if something else happens when ftrace_disabled is set, it just fixes a
symptom and not the bug itself.


> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  kernel/trace/ftrace.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index a3d4dfad0cbc..ff5d9d73a4a7 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -7438,9 +7438,6 @@ void ftrace_release_mod(struct module *mod)
>  
>  	mutex_lock(&ftrace_lock);
>  
> -	if (ftrace_disabled)
> -		goto out_unlock;
> -

Here you delete the check, and the next patch you have:

+	if (ftrace_disabled || (mod && !mod->num_ftrace_callsites)) {
+		mutex_unlock(&ftrace_lock);
+		return;
+	}
+

Why the two patches where the second patch just adds back the check and
then adds some more stuff around it. This should be a single patch.

Also, why not just keep the goto unlock, that has:

 out_unlock:
	mutex_unlock(&ftrace_lock);

	/* Need to synchronize with ftrace_location_range() */
	if (tmp_page)
		synchronize_rcu();
	for (pg = tmp_page; pg; pg = tmp_page) {

		/* Needs to be called outside of ftrace_lock */
		clear_mod_from_hashes(pg);

		if (pg->records) {
			free_pages((unsigned long)pg->records, pg->order);
			ftrace_number_of_pages -= 1 << pg->order;
		}
		tmp_page = pg->next;
		kfree(pg);
		ftrace_number_of_groups--;
	}
}

And tmp_page is set to NULL before that jump, so the if and for loop will
both be nops.

Why all this extra churn?

-- Steve


>  	list_for_each_entry_safe(mod_map, n, &ftrace_mod_maps, list) {
>  		if (mod_map->mod == mod) {
>  			list_del_rcu(&mod_map->list);

Re: [PATCH 2/2] ftrace: don't allocate ftrace module map

[kernel test robot]

Hi Ye,

kernel test robot noticed the following build errors:

[auto build test ERROR on trace/for-next]
[also build test ERROR on linus/master v6.15-rc7 next-20250523]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ye-Bin/ftrace-fix-UAF-when-lookup-kallsym-after-ftrace-disabled/20250523-164234
base:   https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace for-next
patch link:    https://lore.kernel.org/r/20250523083945.3390587-3-yebin%40huaweicloud.com
patch subject: [PATCH 2/2] ftrace: don't allocate  ftrace module map
config: s390-randconfig-001-20250524 (https://download.01.org/0day-ci/archive/20250524/202505240845.ACz1UFaT-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250524/202505240845.ACz1UFaT-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505240845.ACz1UFaT-lkp@intel.com/

All errors (new ones prefixed by >>):

>> kernel/trace/ftrace.c:7825:37: error: incomplete definition of type 'struct module'
    7825 |         if (ftrace_disabled || (mod && !mod->num_ftrace_callsites)) {
         |                                         ~~~^
   include/linux/printk.h:400:8: note: forward declaration of 'struct module'
     400 | struct module;
         |        ^
   1 error generated.


vim +7825 kernel/trace/ftrace.c

  7806	
  7807	void ftrace_free_mem(struct module *mod, void *start_ptr, void *end_ptr)
  7808	{
  7809		unsigned long start = (unsigned long)(start_ptr);
  7810		unsigned long end = (unsigned long)(end_ptr);
  7811		struct ftrace_page **last_pg = &ftrace_pages_start;
  7812		struct ftrace_page *tmp_page = NULL;
  7813		struct ftrace_page *pg;
  7814		struct dyn_ftrace *rec;
  7815		struct dyn_ftrace key;
  7816		struct ftrace_mod_map *mod_map = NULL;
  7817		struct ftrace_init_func *func, *func_next;
  7818		LIST_HEAD(clear_hash);
  7819	
  7820		key.ip = start;
  7821		key.flags = end;	/* overload flags, as it is unsigned long */
  7822	
  7823		mutex_lock(&ftrace_lock);
  7824	
> 7825		if (ftrace_disabled || (mod && !mod->num_ftrace_callsites)) {
  7826			mutex_unlock(&ftrace_lock);
  7827			return;
  7828		}
  7829	
  7830		/*
  7831		 * If we are freeing module init memory, then check if
  7832		 * any tracer is active. If so, we need to save a mapping of
  7833		 * the module functions being freed with the address.
  7834		 */
  7835		if (mod && ftrace_ops_list != &ftrace_list_end)
  7836			mod_map = allocate_ftrace_mod_map(mod, start, end);
  7837	
  7838		for (pg = ftrace_pages_start; pg; last_pg = &pg->next, pg = *last_pg) {
  7839			if (end < pg->records[0].ip ||
  7840			    start >= (pg->records[pg->index - 1].ip + MCOUNT_INSN_SIZE))
  7841				continue;
  7842	 again:
  7843			rec = bsearch(&key, pg->records, pg->index,
  7844				      sizeof(struct dyn_ftrace),
  7845				      ftrace_cmp_recs);
  7846			if (!rec)
  7847				continue;
  7848	
  7849			/* rec will be cleared from hashes after ftrace_lock unlock */
  7850			add_to_clear_hash_list(&clear_hash, rec);
  7851	
  7852			if (mod_map)
  7853				save_ftrace_mod_rec(mod_map, rec);
  7854	
  7855			pg->index--;
  7856			ftrace_update_tot_cnt--;
  7857			if (!pg->index) {
  7858				*last_pg = pg->next;
  7859				pg->next = tmp_page;
  7860				tmp_page = pg;
  7861				pg = container_of(last_pg, struct ftrace_page, next);
  7862				if (!(*last_pg))
  7863					ftrace_pages = pg;
  7864				continue;
  7865			}
  7866			memmove(rec, rec + 1,
  7867				(pg->index - (rec - pg->records)) * sizeof(*rec));
  7868			/* More than one function may be in this block */
  7869			goto again;
  7870		}
  7871		mutex_unlock(&ftrace_lock);
  7872	
  7873		list_for_each_entry_safe(func, func_next, &clear_hash, list) {
  7874			clear_func_from_hashes(func);
  7875			kfree(func);
  7876		}
  7877		/* Need to synchronize with ftrace_location_range() */
  7878		if (tmp_page) {
  7879			synchronize_rcu();
  7880			ftrace_free_pages(tmp_page);
  7881		}
  7882	}
  7883	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/2] ftrace: fix UAF when lookup kallsym after ftrace disabled

[yebin]

On 2025/5/24 1:54, Steven Rostedt wrote:
> On Fri, 23 May 2025 16:39:44 +0800
> Ye Bin <yebin@huaweicloud.com> wrote:
>
>> Above issue may happens as follow:
>> (1) Add kprobe trace point;
>> (2) insmod test.ko;
>> (3) Trigger ftrace disabled;
>
> This is the bug. How was ftrace_disabled triggered? That should never
> happen. Was test.ko buggy?
>
Yes. The following warning is reported during concurrent registration 
between register_kprobe() and live patch, causing ftrace_disabled.

WARNING: CPU: 56 PID: 2769 at kernel/trace/ftrace.c:2612 
ftrace_modify_all_code+0x116/0x140
>> (4) rmmod test.ko;
>> (5) cat /proc/kallsyms; --> Will trigger UAF as test.ko already removed;
>> ftrace_mod_get_kallsym()
>> ...
>> strscpy(module_name, mod_map->mod->name, MODULE_NAME_LEN);
>> ...
>>
>> As ftrace_release_mod() judge 'ftrace_disabled' is true will return, and
>> 'mod_map' will remaining in ftrace_mod_maps. 'mod_map' has no chance to
>> release. Therefore, this also causes residual resources to accumulate.
>> To solve above issue, unconditionally clean up'mod_map'.
>>
>> Fixes: aba4b5c22cba ("ftrace: Save module init functions kallsyms symbols for tracing")
>
> This is *not* a fix. ftrace_disabled gets set when a bug is triggered. If
> this prevents ftrace_disabled from getting set, then it would be a fix. But
> if something else happens when ftrace_disabled is set, it just fixes a
> symptom and not the bug itself.
>
There are multiple causes for triggering ftrace_disabled. I agree that 
aba4b5c22cba is not faulty. However, the incorporation of this patch 
will cause problems due to triggering ftrace_disabled. The generation of 
ftrace_disabled is beyond our control. This is related to the user. What 
we can do is even if there are no additional derivative problems.
>
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>> ---
>>   kernel/trace/ftrace.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index a3d4dfad0cbc..ff5d9d73a4a7 100644
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -7438,9 +7438,6 @@ void ftrace_release_mod(struct module *mod)
>>
>>   	mutex_lock(&ftrace_lock);
>>
>> -	if (ftrace_disabled)
>> -		goto out_unlock;
>> -
>
> Here you delete the check, and the next patch you have:
>
> +	if (ftrace_disabled || (mod && !mod->num_ftrace_callsites)) {
> +		mutex_unlock(&ftrace_lock);
> +		return;
> +	}
> +
>
The second patch I added judgment when initializing 'mod_map' in 
ftrace_free_mem(). The first patch removes the judgment when 
ftrace_release_mod() releases'mod_map'. The logic modified by the two 
patches is isolated.
> Why the two patches where the second patch just adds back the check and
> then adds some more stuff around it. This should be a single patch.
>
> Also, why not just keep the goto unlock, that has:
>
The ftrace_free_mem() function itself looks a little strange. It is easy 
to misunderstand that it is a release function, but it is actually an 
initialization function. My two patches did not modify the same function.
>   out_unlock:
> 	mutex_unlock(&ftrace_lock);
>
> 	/* Need to synchronize with ftrace_location_range() */
> 	if (tmp_page)
> 		synchronize_rcu();
> 	for (pg = tmp_page; pg; pg = tmp_page) {
>
> 		/* Needs to be called outside of ftrace_lock */
> 		clear_mod_from_hashes(pg);
>
> 		if (pg->records) {
> 			free_pages((unsigned long)pg->records, pg->order);
> 			ftrace_number_of_pages -= 1 << pg->order;
> 		}
> 		tmp_page = pg->next;
> 		kfree(pg);
> 		ftrace_number_of_groups--;
> 	}
> }
>
> And tmp_page is set to NULL before that jump, so the if and for loop will
> both be nops.
>
> Why all this extra churn?
>
> -- Steve
>
>
>>   	list_for_each_entry_safe(mod_map, n, &ftrace_mod_maps, list) {
>>   		if (mod_map->mod == mod) {
>>   			list_del_rcu(&mod_map->list);
>

Re: [PATCH 1/2] ftrace: fix UAF when lookup kallsym after ftrace disabled

[Steven Rostedt]

On Mon, 26 May 2025 09:33:37 +0800
yebin <yebin@huaweicloud.com> wrote:

> On 2025/5/24 1:54, Steven Rostedt wrote:
> > On Fri, 23 May 2025 16:39:44 +0800
> > Ye Bin <yebin@huaweicloud.com> wrote:
> >  
> >> Above issue may happens as follow:
> >> (1) Add kprobe trace point;
> >> (2) insmod test.ko;
> >> (3) Trigger ftrace disabled;  
> >
> > This is the bug. How was ftrace_disabled triggered? That should never
> > happen. Was test.ko buggy?
> >  
> Yes. The following warning is reported during concurrent registration 
> between register_kprobe() and live patch, causing ftrace_disabled.
> 
> WARNING: CPU: 56 PID: 2769 at kernel/trace/ftrace.c:2612 
> ftrace_modify_all_code+0x116/0x140

OK, so it is a buggy module.

> >> (4) rmmod test.ko;
> >> (5) cat /proc/kallsyms; --> Will trigger UAF as test.ko already removed;
> >> ftrace_mod_get_kallsym()
> >> ...
> >> strscpy(module_name, mod_map->mod->name, MODULE_NAME_LEN);
> >> ...
> >>
> >> As ftrace_release_mod() judge 'ftrace_disabled' is true will return, and
> >> 'mod_map' will remaining in ftrace_mod_maps. 'mod_map' has no chance to
> >> release. Therefore, this also causes residual resources to accumulate.
> >> To solve above issue, unconditionally clean up'mod_map'.
> >>
> >> Fixes: aba4b5c22cba ("ftrace: Save module init functions kallsyms symbols for tracing")  
> >
> > This is *not* a fix. ftrace_disabled gets set when a bug is triggered. If
> > this prevents ftrace_disabled from getting set, then it would be a fix. But
> > if something else happens when ftrace_disabled is set, it just fixes a
> > symptom and not the bug itself.
> >  
> There are multiple causes for triggering ftrace_disabled. I agree that 

Yes, just like there's multiple causes for BUG_ON() ;-)

The ftrace_disable is used to help keep the system from being totally
corrupted. When it triggers, the best thing to do is a reboot.

> aba4b5c22cba is not faulty. However, the incorporation of this patch 
> will cause problems due to triggering ftrace_disabled. The generation of 
> ftrace_disabled is beyond our control. This is related to the user. What 
> we can do is even if there are no additional derivative problems.

Well, when a user inserts a module, then they become a kernel developer too ;-)

> >  
> >> Signed-off-by: Ye Bin <yebin10@huawei.com>
> >> ---
> >>   kernel/trace/ftrace.c | 3 ---
> >>   1 file changed, 3 deletions(-)
> >>
> >> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> >> index a3d4dfad0cbc..ff5d9d73a4a7 100644
> >> --- a/kernel/trace/ftrace.c
> >> +++ b/kernel/trace/ftrace.c
> >> @@ -7438,9 +7438,6 @@ void ftrace_release_mod(struct module *mod)
> >>
> >>   	mutex_lock(&ftrace_lock);
> >>
> >> -	if (ftrace_disabled)
> >> -		goto out_unlock;
> >> -  
> >
> > Here you delete the check, and the next patch you have:
> >
> > +	if (ftrace_disabled || (mod && !mod->num_ftrace_callsites)) {
> > +		mutex_unlock(&ftrace_lock);
> > +		return;
> > +	}
> > +
> >  
> The second patch I added judgment when initializing 'mod_map' in 
> ftrace_free_mem(). The first patch removes the judgment when 
> ftrace_release_mod() releases'mod_map'. The logic modified by the two 
> patches is isolated.

Actually I think both patches are buggy.

When ftrace_disabled is set, we don't know the state of the code and we do
not want to do *any* more text modification. That's what ftrace_disable
means. Something went wrong with text modification and any more changes can
cause a bigger problem.

We don't add "exceptions".

If you are worried about unloading modules when ftrace_disable is set, what
is a much safer solution is to up the module count of all modules that have
any ftrace callsites active, and prevent those modules from being removed.

Again, the only solution to a ftrace_disable being set is a full reboot.

-- Steve

Re: [PATCH 1/2] ftrace: fix UAF when lookup kallsym after ftrace disabled

[yebin]

On 2025/5/27 21:41, Steven Rostedt wrote:
> On Mon, 26 May 2025 09:33:37 +0800
> yebin <yebin@huaweicloud.com> wrote:
>
>> On 2025/5/24 1:54, Steven Rostedt wrote:
>>> On Fri, 23 May 2025 16:39:44 +0800
>>> Ye Bin <yebin@huaweicloud.com> wrote:
>>>
>>>> Above issue may happens as follow:
>>>> (1) Add kprobe trace point;
>>>> (2) insmod test.ko;
>>>> (3) Trigger ftrace disabled;
>>>
>>> This is the bug. How was ftrace_disabled triggered? That should never
>>> happen. Was test.ko buggy?
>>>
>> Yes. The following warning is reported during concurrent registration
>> between register_kprobe() and live patch, causing ftrace_disabled.
>>
>> WARNING: CPU: 56 PID: 2769 at kernel/trace/ftrace.c:2612
>> ftrace_modify_all_code+0x116/0x140
>
> OK, so it is a buggy module.
>
>>>> (4) rmmod test.ko;
>>>> (5) cat /proc/kallsyms; --> Will trigger UAF as test.ko already removed;
>>>> ftrace_mod_get_kallsym()
>>>> ...
>>>> strscpy(module_name, mod_map->mod->name, MODULE_NAME_LEN);
>>>> ...
>>>>
>>>> As ftrace_release_mod() judge 'ftrace_disabled' is true will return, and
>>>> 'mod_map' will remaining in ftrace_mod_maps. 'mod_map' has no chance to
>>>> release. Therefore, this also causes residual resources to accumulate.
>>>> To solve above issue, unconditionally clean up'mod_map'.
>>>>
>>>> Fixes: aba4b5c22cba ("ftrace: Save module init functions kallsyms symbols for tracing")
>>>
>>> This is *not* a fix. ftrace_disabled gets set when a bug is triggered. If
>>> this prevents ftrace_disabled from getting set, then it would be a fix. But
>>> if something else happens when ftrace_disabled is set, it just fixes a
>>> symptom and not the bug itself.
>>>
>> There are multiple causes for triggering ftrace_disabled. I agree that
>
> Yes, just like there's multiple causes for BUG_ON() ;-)
>
> The ftrace_disable is used to help keep the system from being totally
> corrupted. When it triggers, the best thing to do is a reboot.
>
>> aba4b5c22cba is not faulty. However, the incorporation of this patch
>> will cause problems due to triggering ftrace_disabled. The generation of
>> ftrace_disabled is beyond our control. This is related to the user. What
>> we can do is even if there are no additional derivative problems.
>
> Well, when a user inserts a module, then they become a kernel developer too ;-)
>
>>>
>>>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>>>> ---
>>>>    kernel/trace/ftrace.c | 3 ---
>>>>    1 file changed, 3 deletions(-)
>>>>
>>>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>>>> index a3d4dfad0cbc..ff5d9d73a4a7 100644
>>>> --- a/kernel/trace/ftrace.c
>>>> +++ b/kernel/trace/ftrace.c
>>>> @@ -7438,9 +7438,6 @@ void ftrace_release_mod(struct module *mod)
>>>>
>>>>    	mutex_lock(&ftrace_lock);
>>>>
>>>> -	if (ftrace_disabled)
>>>> -		goto out_unlock;
>>>> -
>>>
>>> Here you delete the check, and the next patch you have:
>>>
>>> +	if (ftrace_disabled || (mod && !mod->num_ftrace_callsites)) {
>>> +		mutex_unlock(&ftrace_lock);
>>> +		return;
>>> +	}
>>> +
>>>
>> The second patch I added judgment when initializing 'mod_map' in
>> ftrace_free_mem(). The first patch removes the judgment when
>> ftrace_release_mod() releases'mod_map'. The logic modified by the two
>> patches is isolated.
>
> Actually I think both patches are buggy.
>
> When ftrace_disabled is set, we don't know the state of the code and we do
> not want to do *any* more text modification. That's what ftrace_disable
> means. Something went wrong with text modification and any more changes can
> cause a bigger problem.
>
This problem can be solved by releasing the 'mod_map' resource when the 
module is unloaded. Freeing up these resources is just an address that 
cannot be translated into symbols, and there are no worse consequences.

> We don't add "exceptions".
>
> If you are worried about unloading modules when ftrace_disable is set, what
> is a much safer solution is to up the module count of all modules that have
> any ftrace callsites active, and prevent those modules from being removed.
>
I don't think it's necessary to introduce logic to restrict module 
unloading here, which doesn't bring benefits but increases the cost of 
interpretation for maintainers.

> Again, the only solution to a ftrace_disable being set is a full reboot.
>
We can't ask users to know such specialized details of the 
implementation, which are unclear even to developers unfamiliar with the 
ftrace module. Users can accept planned reboot system recovery, but 
should not accept casual operations and the system crashes.All we can do 
is do a good job of protection, give users more tolerance.Perhaps a 
system that is dead but won't lie down is also a very undesirable 
situation.However, ftrace is used to collect information and locate 
faults. Even if it does not work, it does not affect services.In the 
production environment, the most afraid of using ftrace suddenly crashes 
the system.Therefore, the robustness of the tool itself is very important.
> -- Steve
>

I reworked the two patches, and the changes to the existing process 
should be minimal. I don't know if I can get your approval. If you 
agree, I'll post another V3 version.

PATCH[1/2]:
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 51580e54677f..b3436d86e470 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -7438,9 +7438,10 @@ void ftrace_release_mod(struct module *mod)

         mutex_lock(&ftrace_lock);

-       if (ftrace_disabled)
-               goto out_unlock;
-
+       /*
+        * To avoid the UAF problem after the module is unloaded, the
+        * 'mod_map' resource needs to be released unconditionally.
+        */
         list_for_each_entry_safe(mod_map, n, &ftrace_mod_maps, list) {
                 if (mod_map->mod == mod) {
                         list_del_rcu(&mod_map->list);
@@ -7451,6 +7452,9 @@ void ftrace_release_mod(struct module *mod)
                 }
         }

+       if (ftrace_disabled)
+               goto out_unlock;
+
         /*
          * Each module has its own ftrace_pages, remove
          * them from the list.

PATCH[2/2]:
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index a3d4dfad0cbc..51580e54677f 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -7629,6 +7629,9 @@ allocate_ftrace_mod_map(struct module *mod,
  {
         struct ftrace_mod_map *mod_map;

+       if (ftrace_disabled)
+               return NULL;
+
         mod_map = kmalloc(sizeof(*mod_map), GFP_KERNEL);
         if (!mod_map)
                 return NULL;

Re: [PATCH 1/2] ftrace: fix UAF when lookup kallsym after ftrace disabled

[Steven Rostedt]

On Wed, 28 May 2025 21:22:37 +0800
yebin <yebin@huaweicloud.com> wrote:

> This problem can be solved by releasing the 'mod_map' resource when the 
> module is unloaded. Freeing up these resources is just an address that 
> cannot be translated into symbols, and there are no worse consequences.

OK, I'm fine with releasing the mod_map resource without doing the text
modifications.

> 
> > Again, the only solution to a ftrace_disable being set is a full reboot.
> >  
> We can't ask users to know such specialized details of the 
> implementation, which are unclear even to developers unfamiliar with the 
> ftrace module. Users can accept planned reboot system recovery, but 
> should not accept casual operations and the system crashes.All we can do 
> is do a good job of protection, give users more tolerance.Perhaps a 
> system that is dead but won't lie down is also a very undesirable 
> situation.However, ftrace is used to collect information and locate 
> faults. Even if it does not work, it does not affect services.In the 
> production environment, the most afraid of using ftrace suddenly crashes 
> the system.Therefore, the robustness of the tool itself is very important.

Preventing ftrace from crashing the system is the reason ftrace_disabled is
set and stops it from doing any more damage.

If you are worried about users not knowing that a reboot is necessary, we
could always add the "Dazed and confused, but trying to continue" message
that could also recommend a reboot.

> > -- Steve
> >  
> 
> I reworked the two patches, and the changes to the existing process 
> should be minimal. I don't know if I can get your approval. If you 
> agree, I'll post another V3 version.
> 
> PATCH[1/2]:
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 51580e54677f..b3436d86e470 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -7438,9 +7438,10 @@ void ftrace_release_mod(struct module *mod)
> 
>          mutex_lock(&ftrace_lock);
> 
> -       if (ftrace_disabled)
> -               goto out_unlock;
> -
> +       /*
> +        * To avoid the UAF problem after the module is unloaded, the
> +        * 'mod_map' resource needs to be released unconditionally.
> +        */
>          list_for_each_entry_safe(mod_map, n, &ftrace_mod_maps, list) {
>                  if (mod_map->mod == mod) {
>                          list_del_rcu(&mod_map->list);
> @@ -7451,6 +7452,9 @@ void ftrace_release_mod(struct module *mod)
>                  }
>          }
> 
> +       if (ftrace_disabled)
> +               goto out_unlock;
> +
>          /*
>           * Each module has its own ftrace_pages, remove
>           * them from the list.

Yes, this is more appropriate.

Thanks,

-- Steve

> 
> PATCH[2/2]:
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index a3d4dfad0cbc..51580e54677f 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -7629,6 +7629,9 @@ allocate_ftrace_mod_map(struct module *mod,
>   {
>          struct ftrace_mod_map *mod_map;
> 
> +       if (ftrace_disabled)
> +               return NULL;
> +
>          mod_map = kmalloc(sizeof(*mod_map), GFP_KERNEL);
>          if (!mod_map)
>                  return NULL;