linux-kernel - Re: [RFC PATCH 1/1] tracefs: add TP_printk_no_nl

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: <20230731112122.41a76154@gandalf.local.home>
Date:   Mon, 31 Jul 2023 11:21:22 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Petr Mladek <pmladek@...e.com>
Cc:     John Ogness <john.ogness@...utronix.de>,
        Jim Cromie <jim.cromie@...il.com>,
        linux-kernel@...r.kernel.org,
        Vincent Whitchurch <vincent.whitchurch@...s.com>,
        daniel@...ll.ch, sergey.senozhatsky@...il.com,
        Simon Ser <contact@...rsion.fr>,
        Sean Paul <seanpaul@...omium.org>
Subject: Re: [RFC PATCH 1/1] tracefs: add TP_printk_no_nl - RFC

On Mon, 31 Jul 2023 12:21:29 +0200
Petr Mladek <pmladek@...e.com> wrote:

> > Grepping through the kernel, I am surprised how many messages are
> > missing the trailing newline when it is obvious they are not intended to
> > be extended. I consider these bugs.  
> 
> I consider this bug as well.
> 
> My understanding is that this patch affects only messages printed to
> the ftrace buffer so that it does not affect printk() directly.
> 
> But still. It creates a habit that is bad for printk(). Also relies
> on the fact that people are aware of this macro and use it. IMHO,
> it will not work in practice. Or do I miss something?

I believe that the problem Jim is addressing is that there's printk()
messages that also go to a tracepoint() function. The tracepoint function
will add its own '\n' to the message, and now we have two '\n's and this
causes extra spaces in the output of the ftrace ring buffer.

Perhaps what we should do is to make sure that the output doesn't add more
newlines than just one.

Something like this: (totally untested)

diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index db575094c498..a1b73ffa1552 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -333,6 +333,8 @@ int trace_output_raw(struct trace_iterator *iter, char *name,
 
 	trace_seq_printf(s, "%s: ", name);
 	trace_seq_vprintf(s, trace_event_format(iter, fmt), ap);
+	trace_seq_trim_newlines(s);
+	trace_seq_putc(s, '\n');
 
 	return trace_handle_return(s);
 }
diff --git a/kernel/trace/trace_seq.c b/kernel/trace/trace_seq.c
index bac06ee3b98b..d4e2049809e3 100644
--- a/kernel/trace/trace_seq.c
+++ b/kernel/trace/trace_seq.c
@@ -221,6 +221,22 @@ void trace_seq_puts(struct trace_seq *s, const char *str)
 }
 EXPORT_SYMBOL_GPL(trace_seq_puts);
 
+/**
+ * trace_seq_trim_newlines - remove ending newlines
+ * @s: trace sequence descriptor
+ *
+ * Remove ending newlines from the buffer.
+ */
+void trace_seq_trim_newlines(struct trace_seq *s)
+{
+	if (unlikely(!s->seq.size || !s->seq.len))
+		return;
+
+	while (s->seq.len && s->buffer[s->seq.len - 1] == '\n') {
+		s->seq.len--;
+	}
+}
+
 /**
  * trace_seq_putc - trace sequence printing of simple character
  * @s: trace sequence descriptor



A similar thing would need to go into libtraceevent.

-- Steve