From: Steven Rostedt <rostedt@goodmis.org>
To: Petr Mladek <pmladek@suse.cz>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Jiri Kosina <jkosina@suse.cz>
Subject: Re: [PATCH 2/2] tracing: Use trace_seq_used() and seq_buf_used() instead of len
Date: Wed, 19 Nov 2014 08:48:00 -0500 [thread overview]
Message-ID: <20141119084800.524beb62@gandalf.local.home> (raw)
In-Reply-To: <20141119114017.GA2332@dhcp128.suse.cz>
On Wed, 19 Nov 2014 12:40:17 +0100
Petr Mladek <pmladek@suse.cz> wrote:
> > >
> > > There is one more dangerous usage in trace_printk_seq(). It is on
> > > three lines there.
> >
> > You totally confused me. What usage in trace_printk_seq(), and what
> > three lines?
> >
> > In this patch, trace_printk_seq() looks like this:
> >
> > int trace_print_seq(struct seq_file *m, struct trace_seq *s)
> > {
> > int ret;
> >
> > __trace_seq_init(s);
> >
> > ret = seq_buf_print_seq(m, &s->seq);
> >
> > /*
> > * Only reset this buffer if we successfully wrote to the
> > * seq_file buffer. This lets the caller try again or
> > * do something else with the contents.
> > */
> > if (!ret)
> > trace_seq_init(s);
> >
> > return ret;
> > }
>
> The confusion is caused by the 'k' ("print" vs. "printk") in the
> function name. I was talking about the following function from
> kernel/trace/trace.c:
Silly 'k', Trix are for kids!
>
> void
> trace_printk_seq(struct trace_seq *s)
> {
> /* Probably should print a warning here. */
> if (s->seq.len >= TRACE_MAX_PRINT)
> s->seq.len = TRACE_MAX_PRINT;
>
> /* should be zero ended, but we are paranoid. */
> s->buffer[s->seq.len] = 0;
>
> printk(KERN_TRACE "%s", s->buffer);
>
> trace_seq_init(s);
> }
>
> I found it when checking the applied patches in origin/rfc/seq-buf
> branch. I hope that it was the correct place.
Yes, that's the working branch for this code.
Anyway, I saw this and thought about using trace_seq_used(), but then I
realized that this is trace_seq code which has a hard coded buffer
length of PAGE_SIZE which on all archs is more than 1000
(TRACE_MAX_PRINT).
Regardless of overflow or not (or even if trace_seq is full), that if
statement will prevent this from doing any buffer overflows.
s->seq.len will never be more than s->seq.size after the test against
TRACE_MAX_PRINT. So I see no harm here.
trace_printk_seq() is for dumping the ring buffer to console, which is
usually something done on panic. It's special.
-- Steve
next prev parent reply other threads:[~2014-11-19 13:48 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-15 4:58 [PATCH 00/26 v5] trace-seq/seq-buf/x86/printk: Print all stacks from NMI safely Steven Rostedt
2014-11-15 4:58 ` [PATCH 01/26 v5] tracing: Fix trace_seq_bitmask() to start at current position Steven Rostedt
2014-11-15 4:58 ` [PATCH 02/26 v5] tracing: Add trace_seq_has_overflowed() and trace_handle_return() Steven Rostedt
2014-11-15 4:58 ` [PATCH 03/26 v5] blktrace/tracing: Use trace_seq_has_overflowed() helper function Steven Rostedt
2014-11-15 4:58 ` [PATCH 04/26 v5] ring-buffer: Remove check of trace_seq_{puts,printf}() return values Steven Rostedt
2014-11-15 4:58 ` [PATCH 05/26 v5] tracing: Have branch tracer use trace_handle_return() helper function Steven Rostedt
2014-11-15 4:58 ` [PATCH 06/26 v5] tracing: Have function_graph use trace_seq_has_overflowed() Steven Rostedt
2014-11-15 4:58 ` [PATCH 07/26 v5] kprobes/tracing: Use trace_seq_has_overflowed() for overflow checks Steven Rostedt
2014-11-18 14:02 ` Petr Mladek
2014-11-15 4:58 ` [PATCH 08/26 v5] tracing: Do not check return values of trace_seq_p*() for mmio tracer Steven Rostedt
2014-11-18 14:06 ` Petr Mladek
2014-11-15 4:58 ` [PATCH 09/26 v5] tracing/probes: Do not use return value of trace_seq_printf() Steven Rostedt
2014-11-18 14:09 ` Petr Mladek
2014-11-15 4:58 ` [PATCH 10/26 v5] tracing/uprobes: Do not use return values " Steven Rostedt
2014-11-17 5:28 ` Masami Hiramatsu
2014-11-17 5:58 ` Srikar Dronamraju
2014-11-18 14:13 ` Petr Mladek
2014-11-15 4:58 ` [PATCH 11/26 v5] tracing: Do not use return values of trace_seq_printf() in syscall tracing Steven Rostedt
2014-11-15 4:58 ` [PATCH 12/26 v5] tracing: Remove return values of most trace_seq_*() functions Steven Rostedt
2014-11-18 14:18 ` Petr Mladek
2014-11-15 4:59 ` [PATCH 13/26 v5] tracing: Fix return value of ftrace_raw_output_prep() Steven Rostedt
2014-11-18 14:24 ` Petr Mladek
2014-11-15 4:59 ` [PATCH 14/26 v5] tracing: Create seq_buf layer in trace_seq Steven Rostedt
2014-11-19 14:51 ` Petr Mladek
2014-11-19 15:08 ` Steven Rostedt
2014-11-15 4:59 ` [PATCH 15/26 v5] tracing: Convert seq_buf_path() to be like seq_path() Steven Rostedt
2014-11-15 4:59 ` [PATCH 16/26 v5] tracing: Convert seq_buf fields to be like seq_file fields Steven Rostedt
2014-11-15 4:59 ` [PATCH 17/26 v5] tracing: Add a seq_buf_clear() helper and clear len and readpos in init Steven Rostedt
2014-11-15 4:59 ` [PATCH 18/26 v5] seq_buf: Create seq_buf_used() to find out how much was written Steven Rostedt
2014-11-18 15:02 ` Petr Mladek
2014-11-19 15:49 ` Steven Rostedt
2014-11-19 16:30 ` Petr Mladek
2014-11-15 4:59 ` [PATCH 19/26 v5] tracing: Use trace_seq_used() and seq_buf_used() instead of len Steven Rostedt
2014-11-17 17:32 ` Steven Rostedt
2014-11-17 19:11 ` [PATCH 1/2] tracing: Clean up tracing_fill_pipe_page() Steven Rostedt
2014-11-18 16:15 ` Petr Mladek
2014-11-19 16:17 ` Steven Rostedt
2014-11-19 16:51 ` Petr Mladek
2014-11-19 17:12 ` Steven Rostedt
2014-11-17 19:12 ` [PATCH 2/2] tracing: Use trace_seq_used() and seq_buf_used() instead of len Steven Rostedt
2014-11-18 16:33 ` Petr Mladek
2014-11-18 17:37 ` Steven Rostedt
2014-11-19 11:40 ` Petr Mladek
2014-11-19 13:48 ` Steven Rostedt [this message]
2014-11-19 14:40 ` Petr Mladek
2014-11-19 15:01 ` Steven Rostedt
2014-11-19 16:00 ` Steven Rostedt
2014-11-19 16:44 ` Petr Mladek
2014-11-15 4:59 ` [PATCH 20/26 v5] seq_buf: Add seq_buf_can_fit() helper function Steven Rostedt
2014-11-17 17:36 ` Steven Rostedt
2014-11-18 0:07 ` Joe Perches
2014-11-18 0:27 ` Steven Rostedt
2014-11-18 0:35 ` Joe Perches
2014-11-18 0:56 ` Steven Rostedt
2014-11-18 1:07 ` Joe Perches
2014-11-18 1:24 ` Steven Rostedt
2014-11-18 1:48 ` Joe Perches
2014-11-18 2:37 ` Steven Rostedt
2014-11-18 2:50 ` Joe Perches
2014-11-18 3:00 ` Steven Rostedt
2014-11-18 16:40 ` Petr Mladek
2014-11-15 4:59 ` [PATCH 21/26 v5] tracing: Have seq_buf use full buffer Steven Rostedt
2014-11-15 4:59 ` [PATCH 22/26 v5] tracing: Add seq_buf_get_buf() and seq_buf_commit() helper functions Steven Rostedt
2014-11-15 4:59 ` [PATCH 23/26 v5] seq-buf: Make seq_buf_bprintf() conditional on CONFIG_BINARY_PRINTF Steven Rostedt
2014-11-15 4:59 ` [PATCH 24/26 v5] seq_buf: Move the seq_buf code to lib/ Steven Rostedt
2014-11-15 4:59 ` [PATCH 25/26 v5] printk: Add per_cpu printk func to allow printk to be diverted Steven Rostedt
2014-11-15 4:59 ` [PATCH 26/26 v5] x86/nmi: Perform a safe NMI stack trace on all CPUs Steven Rostedt
2014-11-18 17:02 ` Petr Mladek
2014-11-15 5:08 ` [PATCH 00/26 v5] trace-seq/seq-buf/x86/printk: Print all stacks from NMI safely Steven Rostedt
2014-11-18 3:21 ` Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141119084800.524beb62@gandalf.local.home \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=jkosina@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=pmladek@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).