mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] profiling: prevent stale prof_cpu_mask access on init failure
@ 2026-06-21 19:23 Tristan Madani
  2026-06-21 22:49 ` Tetsuo Handa
  2026-06-22  0:00 ` [PATCH v2] profiling: don't free prof_cpu_mask " Tristan Madani
  0 siblings, 2 replies; 9+ messages in thread
From: Tristan Madani @ 2026-06-21 19:23 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Ingo Molnar, Dave Hansen, Tetsuo Handa, linux-kernel, stable,
	Tristan Madani

From: Tristan Madani <tristan@talencesecurity.com>

When profiling is enabled at runtime via /sys/kernel/profiling,
profile_setup() sets prof_on and profile_init() allocates prof_cpu_mask
and attempts to allocate prof_buffer. If all prof_buffer allocations
fail, the error path frees prof_cpu_mask but leaves prof_on set.

Since profile_tick() runs from timer interrupt context and checks
cpumask_available(prof_cpu_mask) without first checking prof_on, it can
dereference the freed cpumask between the free and the next reboot.

Clear prof_on before freeing prof_cpu_mask so the profiling state remains
consistent on allocation failure. Also gate the cpumask access in
profile_tick() on prof_on to prevent accessing stale state during the
teardown window.

Fixes: 22b8ce94708f ("profiling: dynamically enable readprofile at runtime")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 kernel/profile.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/profile.c b/kernel/profile.c
index 984f819b701c9..a166ad9512714 100644
--- a/kernel/profile.c
+++ b/kernel/profile.c
@@ -123,6 +123,7 @@ int __ref profile_init(void)
 	if (prof_buffer)
 		return 0;
 
+	prof_on = 0;
 	free_cpumask_var(prof_cpu_mask);
 	return -ENOMEM;
 }
@@ -325,7 +326,7 @@ void profile_tick(int type)
 {
 	struct pt_regs *regs = get_irq_regs();
 
-	if (!user_mode(regs) && cpumask_available(prof_cpu_mask) &&
+	if (!user_mode(regs) && prof_on && cpumask_available(prof_cpu_mask) &&
 	    cpumask_test_cpu(smp_processor_id(), prof_cpu_mask))
 		profile_hit(type, (void *)profile_pc(regs));
 }
-- 
2.47.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] profiling: prevent stale prof_cpu_mask access on init failure
  2026-06-21 19:23 [PATCH] profiling: prevent stale prof_cpu_mask access on init failure Tristan Madani
@ 2026-06-21 22:49 ` Tetsuo Handa
  2026-06-21 23:45   ` Tristan Madani
  2026-06-22  0:00 ` [PATCH v2] profiling: don't free prof_cpu_mask " Tristan Madani
  1 sibling, 1 reply; 9+ messages in thread
From: Tetsuo Handa @ 2026-06-21 22:49 UTC (permalink / raw)
  To: Tristan Madani, Andrew Morton
  Cc: Ingo Molnar, Dave Hansen, linux-kernel, stable, Tristan Madani

On 2026/06/22 4:23, Tristan Madani wrote:
> diff --git a/kernel/profile.c b/kernel/profile.c
> index 984f819b701c9..a166ad9512714 100644
> --- a/kernel/profile.c
> +++ b/kernel/profile.c
> @@ -123,6 +123,7 @@ int __ref profile_init(void)
>  	if (prof_buffer)
>  		return 0;
>  
> +	prof_on = 0;
>  	free_cpumask_var(prof_cpu_mask);

Which tree are you talking about?

>  	return -ENOMEM;
>  }
> @@ -325,7 +326,7 @@ void profile_tick(int type)
>  {
>  	struct pt_regs *regs = get_irq_regs();
>  
> -	if (!user_mode(regs) && cpumask_available(prof_cpu_mask) &&
> +	if (!user_mode(regs) && prof_on && cpumask_available(prof_cpu_mask) &&
>  	    cpumask_test_cpu(smp_processor_id(), prof_cpu_mask))

NAK. This is a use-after-free read bug.

  CPU0                            CPU1
  
                                  if (!user_mode(regs) && prof_on && cpumask_available(prof_cpu_mask) &&
  prof_on = 0;
  free_cpumask_var(prof_cpu_mask);
                                      cpumask_test_cpu(smp_processor_id(), prof_cpu_mask)) // <= prof_cpu_mask was already freed.

Correct fix is to remove a commit which adds "free_cpumask_var(prof_cpu_mask);".

>  		profile_hit(type, (void *)profile_pc(regs));
>  }


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] profiling: prevent stale prof_cpu_mask access on init failure
  2026-06-21 22:49 ` Tetsuo Handa
@ 2026-06-21 23:45   ` Tristan Madani
  0 siblings, 0 replies; 9+ messages in thread
From: Tristan Madani @ 2026-06-21 23:45 UTC (permalink / raw)
  To: Tetsuo Handa, Andrew Morton
  Cc: Ingo Molnar, Dave Hansen, linux-kernel, stable, Tristan Madani

On 2026/06/22 07:49, Tetsuo Handa wrote:
> NAK. This is a use-after-free read bug.
>
> Correct fix is to remove a commit which adds "free_cpumask_var(prof_cpu_mask);".

You're right, the flag check races with the free. v2 will just
remove the free_cpumask_var() call instead.

> Which tree are you talking about?

This is for stable (6.1.y, 6.6.y, 6.8.y) where prof_cpu_mask
still exists.

Thanks,
Tristan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] profiling: don't free prof_cpu_mask on init failure
  2026-06-21 19:23 [PATCH] profiling: prevent stale prof_cpu_mask access on init failure Tristan Madani
  2026-06-21 22:49 ` Tetsuo Handa
@ 2026-06-22  0:00 ` Tristan Madani
  2026-06-22 11:32   ` Tetsuo Handa
  2026-06-25  1:11   ` Andrew Morton
  1 sibling, 2 replies; 9+ messages in thread
From: Tristan Madani @ 2026-06-22  0:00 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Tetsuo Handa, Ingo Molnar, Dave Hansen, linux-kernel, stable,
	Tristan Madani

From: Tristan Madani <tristan@talencesecurity.com>

When profiling is enabled at runtime via /sys/kernel/profiling,
profile_setup() sets prof_on and profile_init() allocates prof_cpu_mask
then attempts to allocate prof_buffer. If all prof_buffer allocations
fail, the error path frees prof_cpu_mask but leaves prof_on set.

Since profile_tick() runs from timer interrupt context and checks
cpumask_available(prof_cpu_mask), it can access the freed cpumask
between the free and the next reboot.

Remove the free_cpumask_var() call from the error path. The cpumask
allocation already succeeded and is small; keeping it on this rare
failure path is harmless.

Fixes: 22b8ce94708f ("profiling: dynamically enable readprofile at runtime")
Cc: stable@vger.kernel.org
Suggested-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
Changes in v2:
- Remove the free_cpumask_var() call instead of adding a prof_on
  guard in profile_tick(), which still raced with the free (Tetsuo Handa)
 kernel/profile.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/profile.c b/kernel/profile.c
index 984f819b701c9..93180f9d21467 100644
--- a/kernel/profile.c
+++ b/kernel/profile.c
@@ -123,7 +123,6 @@ int __ref profile_init(void)
 	if (prof_buffer)
 		return 0;
 
-	free_cpumask_var(prof_cpu_mask);
 	return -ENOMEM;
 }
 
-- 
2.47.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] profiling: don't free prof_cpu_mask on init failure
  2026-06-22  0:00 ` [PATCH v2] profiling: don't free prof_cpu_mask " Tristan Madani
@ 2026-06-22 11:32   ` Tetsuo Handa
  2026-06-22 18:28     ` Tristan Madani
  2026-06-25  1:11   ` Andrew Morton
  1 sibling, 1 reply; 9+ messages in thread
From: Tetsuo Handa @ 2026-06-22 11:32 UTC (permalink / raw)
  To: Tristan Madani, Andrew Morton
  Cc: Ingo Molnar, Dave Hansen, linux-kernel, stable, Tristan Madani

On 2026/06/22 9:00, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
> 
> When profiling is enabled at runtime via /sys/kernel/profiling,
> profile_setup() sets prof_on and profile_init() allocates prof_cpu_mask
> then attempts to allocate prof_buffer. If all prof_buffer allocations
> fail, the error path frees prof_cpu_mask but leaves prof_on set.
> 
> Since profile_tick() runs from timer interrupt context and checks
> cpumask_available(prof_cpu_mask), it can access the freed cpumask
> between the free and the next reboot.
> 
> Remove the free_cpumask_var() call from the error path. The cpumask
> allocation already succeeded and is small; keeping it on this rare
> failure path is harmless.
> 
> Fixes: 22b8ce94708f ("profiling: dynamically enable readprofile at runtime")

Why 22b8ce94708f ? That commit did not add free_cpumask_var().
Since free_cpumask_var() was removed by 7c51f7bbf057, your patch might want
explanation about why you choose to only avoid UAF-read for stable kernels
instead of try to apply 7c51f7bbf057.

> Cc: stable@vger.kernel.org
> Suggested-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
> Changes in v2:
> - Remove the free_cpumask_var() call instead of adding a prof_on
>   guard in profile_tick(), which still raced with the free (Tetsuo Handa)
>  kernel/profile.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/kernel/profile.c b/kernel/profile.c
> index 984f819b701c9..93180f9d21467 100644
> --- a/kernel/profile.c
> +++ b/kernel/profile.c
> @@ -123,7 +123,6 @@ int __ref profile_init(void)
>  	if (prof_buffer)
>  		return 0;
>  
> -	free_cpumask_var(prof_cpu_mask);
>  	return -ENOMEM;
>  }
>  


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] profiling: don't free prof_cpu_mask on init failure
  2026-06-22 11:32   ` Tetsuo Handa
@ 2026-06-22 18:28     ` Tristan Madani
  2026-06-22 22:50       ` Tetsuo Handa
  0 siblings, 1 reply; 9+ messages in thread
From: Tristan Madani @ 2026-06-22 18:28 UTC (permalink / raw)
  To: Tetsuo Handa, Andrew Morton
  Cc: Dave Hansen, linux-kernel, stable, Tristan Madani

On 2026/06/22 20:32, Tetsuo Handa wrote:
> Why 22b8ce94708f ? That commit did not add free_cpumask_var().
> Since free_cpumask_var() was removed by 7c51f7bbf057, your patch might want
> explanation about why you choose to only avoid UAF-read for stable kernels
> instead of try to apply 7c51f7bbf057.

Thanks for the review. You're right, the Fixes tag should be
c309b917cab5 ("cpumask: convert kernel/profile.c").

I went with the minimal fix you suggested since 7c51f7bbf057 touches
two files and adds serialization, which felt heavier for a stable
backport.

Would a v3 with the corrected Fixes tag and a note explaining the
choice over 7c51f7bbf057 work, or would you prefer to just Cc stable
on your commit?

Tristan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] profiling: don't free prof_cpu_mask on init failure
  2026-06-22 18:28     ` Tristan Madani
@ 2026-06-22 22:50       ` Tetsuo Handa
  0 siblings, 0 replies; 9+ messages in thread
From: Tetsuo Handa @ 2026-06-22 22:50 UTC (permalink / raw)
  To: Tristan Madani, Andrew Morton
  Cc: Dave Hansen, linux-kernel, stable, Tristan Madani

On 2026/06/23 3:28, Tristan Madani wrote:
> I went with the minimal fix you suggested since 7c51f7bbf057 touches
> two files and adds serialization, which felt heavier for a stable
> backport.

OK.

> 
> Would a v3 with the corrected Fixes tag and a note explaining the
> choice over 7c51f7bbf057 work, or would you prefer to just Cc stable
> on your commit?

Just as you like. I am not the person who makes decisions on for-stable-only
patches.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] profiling: don't free prof_cpu_mask on init failure
  2026-06-22  0:00 ` [PATCH v2] profiling: don't free prof_cpu_mask " Tristan Madani
  2026-06-22 11:32   ` Tetsuo Handa
@ 2026-06-25  1:11   ` Andrew Morton
  2026-06-26 15:36     ` Tristan Madani
  1 sibling, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2026-06-25  1:11 UTC (permalink / raw)
  To: Tristan Madani
  Cc: Tetsuo Handa, Ingo Molnar, Dave Hansen, linux-kernel, stable,
	Tristan Madani

On Mon, 22 Jun 2026 00:00:22 +0000 Tristan Madani <tristmd@gmail.com> wrote:

> From: Tristan Madani <tristan@talencesecurity.com>
> 
> When profiling is enabled at runtime via /sys/kernel/profiling,
> profile_setup() sets prof_on and profile_init() allocates prof_cpu_mask
> then attempts to allocate prof_buffer. If all prof_buffer allocations
> fail, the error path frees prof_cpu_mask but leaves prof_on set.
> 
> Since profile_tick() runs from timer interrupt context and checks
> cpumask_available(prof_cpu_mask), it can access the freed cpumask
> between the free and the next reboot.
> 
> Remove the free_cpumask_var() call from the error path. The cpumask
> allocation already succeeded and is small; keeping it on this rare
> failure path is harmless.
> 
> ...
>
> --- a/kernel/profile.c
> +++ b/kernel/profile.c
> @@ -123,7 +123,6 @@ int __ref profile_init(void)
>  	if (prof_buffer)
>  		return 0;
>  
> -	free_cpumask_var(prof_cpu_mask);
>  	return -ENOMEM;
>  }

Confused.  Current mainline has no free_cpumask_var() here?

If we're to deliberately leak the mask here then let's have a little
comment explaining the reasoning, so we don't later receive "profiling:
fix memory leak" patches.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] profiling: don't free prof_cpu_mask on init failure
  2026-06-25  1:11   ` Andrew Morton
@ 2026-06-26 15:36     ` Tristan Madani
  0 siblings, 0 replies; 9+ messages in thread
From: Tristan Madani @ 2026-06-26 15:36 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Tetsuo Handa, Ingo Molnar, Dave Hansen, linux-kernel, stable,
	Tristan Madani

On Wed, 24 Jun 2026 18:11:13 -0700 Andrew Morton wrote:
> Confused.  Current mainline has no free_cpumask_var() here?

Correct -- Tetsuo's 7c51f7bbf057 ("profiling: remove prof_cpu_mask")
removed the variable and all its references in v6.11. This patch is
for 6.1.y and 6.6.y where the old error-path free is still present.

> If we're to deliberately leak the mask here then let's have a little
> comment explaining the reasoning, so we don't later receive "profiling:
> fix memory leak" patches.

Good point. I can send v3 with:
  - a comment above the return explaining the deliberate leak
  - corrected Fixes tag (c309b917cab5, per Tetsuo's review)
  - stable-only framing in the commit message

Alternatively, Cc stable on 7c51f7bbf057 would also fix this, though
that commit removes prof_cpu_mask entirely, adds mutex serialization
in ksysfs.c, and drops the hotplug online callback (2 files, +13/-40).

Happy to go either way.

Tristan

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-06-26 15:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-21 19:23 [PATCH] profiling: prevent stale prof_cpu_mask access on init failure Tristan Madani
2026-06-21 22:49 ` Tetsuo Handa
2026-06-21 23:45   ` Tristan Madani
2026-06-22  0:00 ` [PATCH v2] profiling: don't free prof_cpu_mask " Tristan Madani
2026-06-22 11:32   ` Tetsuo Handa
2026-06-22 18:28     ` Tristan Madani
2026-06-22 22:50       ` Tetsuo Handa
2026-06-25  1:11   ` Andrew Morton
2026-06-26 15:36     ` Tristan Madani

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®