linux-kernel - Re: [GIT PULL] ring-buffer: Revert previous fix as it wasn't the actual fix

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241114105412.43b00a95@gandalf.local.home>
Date: Thu, 14 Nov 2024 10:54:12 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Masami Hiramatsu
 <mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Subject: Re: [GIT PULL] ring-buffer: Revert previous fix as it wasn't the
 actual fix


Hi Linus,

You can hold off on pulling this. I just found another issue I want to fix
and will likely have another pull request tomorrow on top of this one.

-- Steve


On Thu, 14 Nov 2024 10:41:49 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:

> Linus,
> 
> Revert: "ring-buffer: Do not have boot mapped buffers hook to CPU hotplug"
> 
> - A crash that happened on cpu hotplug was actually caused by the incorrect
>   ref counting that was fixed by commit 2cf9733891a4 ("ring-buffer: Fix
>   refcount setting of boot mapped buffers"). The removal of calling cpu
>   hotplug callbacks on memory mapped buffers was not an issue even though
>   the tests at the time pointed toward it. But in fact, there's a check in
>   that code that tests to see if the buffers are already allocated or not,
>   and will not allocate them again if they are. Not calling the cpu hotplug
>   callbacks ended up not initializing the non boot CPU buffers.
> 
>   Simply remove that change.
> 
> 
> Please pull the latest trace-ringbuffer-v6.12-rc7 tree, which can be found at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
> trace-ringbuffer-v6.12-rc7
> 
> Tag SHA1: 81ccc26cac30f76e5fb6d76aec178677c3e209ad
> Head SHA1: 580bb355bcae7e9a6606ce9644af09b2a793f1bb
> 
> 
> Steven Rostedt (1):
>       Revert: "ring-buffer: Do not have boot mapped buffers hook to CPU hotplug"
> 
> ----
>  kernel/trace/ring_buffer.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> ---------------------------
> commit 580bb355bcae7e9a6606ce9644af09b2a793f1bb
> Author: Steven Rostedt <rostedt@...dmis.org>
> Date:   Wed Nov 13 23:08:39 2024 -0500
> 
>     Revert: "ring-buffer: Do not have boot mapped buffers hook to CPU hotplug"
>     
>     A crash happened when testing cpu hotplug with respect to the memory
>     mapped ring buffers. It was assumed that the hot plug code was adding a
>     per CPU buffer that was already created that caused the crash. The real
>     problem was due to ref counting and was fixed by commit 2cf9733891a4
>     ("ring-buffer: Fix refcount setting of boot mapped buffers").
>     
>     When a per CPU buffer is created, it will not be created again even with
>     CPU hotplug, so the fix to not use CPU hotplug was a red herring. In fact,
>     it caused only the boot CPU buffer to be created, leaving the other CPU
>     per CPU buffers disabled.
>     
>     Revert that change as it was not the culprit of the fix it was intended to
>     be.
>     
>     Cc: Masami Hiramatsu <mhiramat@...nel.org>
>     Cc: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
>     Link: https://lore.kernel.org/20241113230839.6c03640f@gandalf.local.home
>     Fixes: 912da2c384d5 ("ring-buffer: Do not have boot mapped buffers hook to CPU hotplug")
>     Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 3ea4f7bb1837..5807116bcd0b 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -2337,12 +2337,9 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
>  	if (!buffer->buffers[cpu])
>  		goto fail_free_buffers;
>  
> -	/* If already mapped, do not hook to CPU hotplug */
> -	if (!start) {
> -		ret = cpuhp_state_add_instance(CPUHP_TRACE_RB_PREPARE, &buffer->node);
> -		if (ret < 0)
> -			goto fail_free_buffers;
> -	}
> +	ret = cpuhp_state_add_instance(CPUHP_TRACE_RB_PREPARE, &buffer->node);
> +	if (ret < 0)
> +		goto fail_free_buffers;
>  
>  	mutex_init(&buffer->mutex);
>