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] [day] [month] [year] [list]
Date: Mon, 22 Jan 2024 17:57:01 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: "Ricardo B. Marliere" <ricardo@...liere.net>
Cc: Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, linux-kernel@...r.kernel.org,
 linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH] tracing: add trace_seq_reset function

On Mon, 22 Jan 2024 19:45:41 -0300
"Ricardo B. Marliere" <ricardo@...liere.net> wrote:

> > Perhaps we need a:
> > 
> > 	if (WARN_ON_ONCE(!s->seq.size))
> > 		seq_buf_init(&s->seq, s->buffer, TRACE_SEQ_BUFFER_SIZE);
> > 	else
> > 		seq_buf_clear(&s->seq);
> > 
> > 
> > in the trace_seq_reset() to catch any place that doesn't have it
> > initialized yet.  
> 
> But that would be temporary, right? Kind of a "trace_seq_init_or_reset".
> If that's the case it would be best to simply work out all the places
> instead. Would the current tests [1] cover everything or should
> something else be made to make sure no place was missing from the patch?

No it would be permanent. And no, it is *not* a "trace_seq_init_or_reset".
That WARN_ON_ONCE() means this should never happen, but if it does, the
if () statement keeps it from crashing.

If we later make it dynamic, where there is no s->buffer, that would then
change to:

	if (WARN_ON_ONCE(!s->seq.size))
		seq_buf_init(&s->seq, NULL, 0);
	else
		seq_buf_clear(&s->seq);

Where the trace_seq is basically disabled from that point on.

I would try to fix everything, but this will find those places you missed. ;-)

The kernel is like this, because it's not like any other application. If an
application crashes, you may get annoyed, and just restart it. If the
kernel crashes, you can easily lose data and you have to reboot your
machine. Thus, we treat things that could cause harm much more carefully.

We do not want to crash the kernel just because we missed a location that
did not initialize trace_seq. That's why this would be a permanent change.

-- Steve

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ