linux-kernel - Re: [GIT PULL] ring-buffer: Updates for 6.11

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: <7e29c303-c91f-4229-9b9d-b60d8a60c38b@efficios.com>
Date: Fri, 19 Jul 2024 10:59:37 -0400
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
 LKML <linux-kernel@...r.kernel.org>, Masami Hiramatsu <mhiramat@...nel.org>,
 Dan Carpenter <dan.carpenter@...aro.org>,
 Thorsten Blum <thorsten.blum@...lux.com>
Subject: Re: [GIT PULL] ring-buffer: Updates for 6.11

On 2024-07-19 10:32, Steven Rostedt wrote:
> On Tue, 16 Jul 2024 16:05:26 -0400
> Mathieu Desnoyers <mathieu.desnoyers@...icios.com> wrote:
> 
>> On 2024-07-16 15:51, Steven Rostedt wrote:
>>>
>>>
>>> Linus,
>>>
>>> tracing/ring-buffer: Have persistent buffer across reboots
>>
>> Hi Steven,
>>
>> Perhaps I'm missing something here, but we discussed previously that
>> you would document the fact that users of this feature are expected
>> to run the same kernel before/after reboot.
>>
>> Looking at this PR, I fail to find that documentation, or in fact
>> any documentation at all. Is this something that was overlooked ?
> 
> So I went to update this, and realized there's no place that actually
> mentions anything about this being used across reboots (besides in the
> change logs). The only documentation is in kernel-parameters.txt:
> 
>                          If memory has been reserved (see memmap for x86), the instance
>                          can use that memory:
>                          
>                                  memmap=12M$0x284500000 trace_instance=boot_map@...84500000:12M
> 
>                          The above will create a "boot_map" instance that uses the physical
>                          memory at 0x284500000 that is 12Megs. The per CPU buffers of that
>                          instance will be split up accordingly.
> 
> I do plan on adding more documentation on the use cases of this, but I
> was planning on doing that in the next merge window when the
> reserve_mem kernel parameter can be used. Right now, we only document
> what it does, and not what it is used for.
> 
> Do you still have an issue with that part missing? No where does it
> mention that this is used across boots. There is a file created in the
> boot mapped instance directory that hints to the usage
> (last_boot_info), but still there's no assumptions made.
> 
> In other words, why document a restriction on something that hasn't
> been documented?

AFAIU the intended use of this feature is to convey trace buffer
data from one kernel to the next across a reboot, which makes it
a whole different/new kind of ABI.

Having no documentation will not stop anyone from using this new
feature and make assumptions about ABI guarantees. I am concerned
that this ABI ends up being defined by accident/misuses rather than
by design if it is merged without documentation.

Very often when I find myself documenting a feature, I look back at
the user-facing result and modify the ABI where it does not make
sense. Merging this ABI without documentation prevents that.

So if you ask my honest opinion there, I would say that merging this
ABI without documentation feels rushed.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com