All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [RFC][PATCH] tracing: Add trace_printk_ptr() to force non use of trace_bprintk()
Date: Wed, 29 Jun 2016 09:24:28 +0200	[thread overview]
Message-ID: <20160629072428.GA14656@krava> (raw)
In-Reply-To: <20160628191929.4d238183@gandalf.local.home>

On Tue, Jun 28, 2016 at 07:19:29PM -0400, Steven Rostedt wrote:
> trace_printk() is a very helpful tool for debugging the kernel. It adds
> lots of tricks to optimize itself to prevent any "heisenbugs". That is,
> having the addition of tracing cause the bug to change its timing and
> disappear. One of this tricks is to use trace_bprintk() when possible,
> which just stores the format and the arguments into the ring buffer to
> be processed later at the time of reading the trace output.
> 
> The issue with this is that there's some printf() fields that can do
> redirection. There's a list of "%p*" values that will dereference the
> pointer saved in the buffer. This is an issue with trace_printk()
> because the pointer could have been freed between the time the
> trace_printk() was called and the time the buffer is read. This will
> cause a bad pointer dereference.
> 
> The preferable fix is most likely to change bprintk() to recognize
> these pointers and instead of saving the pointer in the buffer to be
> processed later, it could do the conversion and save the value in the
> buffer. But this added processing kills the whole point of bprintk()
> from being fast and not doing any processing during the recording.
> Perhaps it should simply warn and/or refuse to print.
> 
> The simpler solution is to add an alternate trace_printk() that always
> uses the non optimized version that does the string processing at the
> time of the record, and saves just the string to the ring buffer.
> 
> There's been many times that I myself wanted this version. So here it
> is.
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

if we dont go with this change:
  http://marc.info/?l=linux-kernel&m=146715171527229&w=2

this patch works for me:

Tested-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

      reply	other threads:[~2016-06-29  7:24 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-28 23:19 [RFC][PATCH] tracing: Add trace_printk_ptr() to force non use of trace_bprintk() Steven Rostedt
2016-06-29  7:24 ` Jiri Olsa [this message]

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=20160629072428.GA14656@krava \
    --to=jolsa@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=mingo@kernel.org \
    --cc=rostedt@goodmis.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.