[PATCH] fgraph: Still initialize idle shadow stacks when starting

Steven Rostedt posted 1 patch 1 year ago
kernel/trace/fgraph.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
[PATCH] fgraph: Still initialize idle shadow stacks when starting
Posted by Steven Rostedt 1 year ago
From: Steven Rostedt <rostedt@goodmis.org>

A bug was discovered where the idle shadow stacks were not initialized
for offline CPUs when starting function graph tracer, and when they came
online they were not traced due to the missing shadow stack. To fix
this, the idle task shadow stack initialization was moved to using the
CPU hotplug callbacks. But it removed the initialization when the
function graph was enabled. The problem here is that the hotplug
callbacks are called when the CPUs come online, but the idle shadow
stack initialization only happens if function graph is currently
active. This caused the online CPUs to not get their shadow stack
initialized.

The idle shadow stack initialization still needs to be done when the
function graph is registered, as they will not be allocated if function
graph is not registered.

Cc: stable@vger.kernel.org
Fixes: 2c02f7375e65 ("fgraph: Use CPU hotplug mechanism to initialize idle shadow stacks")
Reported-by: Linus Walleij <linus.walleij@linaro.org>
Closes: https://lore.kernel.org/all/CACRpkdaTBrHwRbbrphVy-=SeDz6MSsXhTKypOtLrTQ+DgGAOcQ@mail.gmail.com/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 kernel/trace/fgraph.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 0bf78517b5d4..ddedcb50917f 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -1215,7 +1215,7 @@ void fgraph_update_pid_func(void)
 static int start_graph_tracing(void)
 {
 	unsigned long **ret_stack_list;
-	int ret;
+	int ret, cpu;
 
 	ret_stack_list = kcalloc(FTRACE_RETSTACK_ALLOC_SIZE,
 				 sizeof(*ret_stack_list), GFP_KERNEL);
@@ -1223,6 +1223,12 @@ static int start_graph_tracing(void)
 	if (!ret_stack_list)
 		return -ENOMEM;
 
+	/* The cpu_boot init_task->ret_stack will never be freed */
+	for_each_online_cpu(cpu) {
+		if (!idle_task(cpu)->ret_stack)
+			ftrace_graph_init_idle_task(idle_task(cpu), cpu);
+	}
+
 	do {
 		ret = alloc_retstack_tasklist(ret_stack_list);
 	} while (ret == -EAGAIN);
-- 
2.45.2
Re: [PATCH] fgraph: Still initialize idle shadow stacks when starting
Posted by Linus Walleij 1 year ago
On Wed, Dec 11, 2024 at 7:53 PM Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <rostedt@goodmis.org>
>
> A bug was discovered where the idle shadow stacks were not initialized
> for offline CPUs when starting function graph tracer, and when they came
> online they were not traced due to the missing shadow stack. To fix
> this, the idle task shadow stack initialization was moved to using the
> CPU hotplug callbacks. But it removed the initialization when the
> function graph was enabled. The problem here is that the hotplug
> callbacks are called when the CPUs come online, but the idle shadow
> stack initialization only happens if function graph is currently
> active. This caused the online CPUs to not get their shadow stack
> initialized.
>
> The idle shadow stack initialization still needs to be done when the
> function graph is registered, as they will not be allocated if function
> graph is not registered.
>
> Cc: stable@vger.kernel.org
> Fixes: 2c02f7375e65 ("fgraph: Use CPU hotplug mechanism to initialize idle shadow stacks")
> Reported-by: Linus Walleij <linus.walleij@linaro.org>
> Closes: https://lore.kernel.org/all/CACRpkdaTBrHwRbbrphVy-=SeDz6MSsXhTKypOtLrTQ+DgGAOcQ@mail.gmail.com/
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

Yep that solves my issue and I can go on debugging my boot!
Tested-by: Linus Walleij <linus.walleij@linaro.org>

Thanks for patching this up so quickly Stephen, you're the best.

Yours,
Linus Walleij
Re: [PATCH] fgraph: Still initialize idle shadow stacks when starting
Posted by Steven Rostedt 1 year ago

On December 12, 2024 3:49:37 PM EST, Linus Walleij <linus.walleij@linaro.org> wrote:
>On Wed, Dec 11, 2024 at 7:53 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
>> From: Steven Rostedt <rostedt@goodmis.org>
>>
>> A bug was discovered where the idle shadow stacks were not initialized
>> for offline CPUs when starting function graph tracer, and when they came
>> online they were not traced due to the missing shadow stack. To fix
>> this, the idle task shadow stack initialization was moved to using the
>> CPU hotplug callbacks. But it removed the initialization when the
>> function graph was enabled. The problem here is that the hotplug
>> callbacks are called when the CPUs come online, but the idle shadow
>> stack initialization only happens if function graph is currently
>> active. This caused the online CPUs to not get their shadow stack
>> initialized.
>>
>> The idle shadow stack initialization still needs to be done when the
>> function graph is registered, as they will not be allocated if function
>> graph is not registered.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 2c02f7375e65 ("fgraph: Use CPU hotplug mechanism to initialize idle shadow stacks")
>> Reported-by: Linus Walleij <linus.walleij@linaro.org>
>> Closes: https://lore.kernel.org/all/CACRpkdaTBrHwRbbrphVy-=SeDz6MSsXhTKypOtLrTQ+DgGAOcQ@mail.gmail.com/
>> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
>
>Yep that solves my issue and I can go on debugging my boot!
>Tested-by: Linus Walleij <linus.walleij@linaro.org>
>
>Thanks for patching this up so quickly Stephen, you're the best.
>

No problem, but I don't know who this "Stephen" is? ;-)

-- Steve

>Yours,
>Linus Walleij