linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Mark Salyzyn <salyzyn@android.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org,
	Nick Desaulniers <ndesaulniers@google.com>,
	Ingo Molnar <mingo@redhat.com>,
	kernel-team@android.com, stable@vger.kernel.org
Subject: Re: [PATCH] tracing: do not leak kernel addresses
Date: Thu, 26 Jul 2018 17:31:53 +0200	[thread overview]
Message-ID: <20180726153153.GA8327@kroah.com> (raw)
In-Reply-To: <11437c3e-5131-7190-c496-7b51eb7fcc2a@android.com>

On Thu, Jul 26, 2018 at 08:14:08AM -0700, Mark Salyzyn wrote:
> On 07/25/2018 06:07 PM, Steven Rostedt wrote:
> > On Wed, 25 Jul 2018 13:22:36 -0700
> > Mark Salyzyn <salyzyn@android.com> wrote:
> > 
> > > From: Nick Desaulniers <ndesaulniers@google.com>
> > > 
> > > Switch from 0x%lx to 0x%pK to print the kernel addresses.
> > > 
> > > Fixes: CVE-2017-0630
> > Wait!!!! This breaks perf and trace-cmd! They require this to be able
> > to print various strings in trace events. This file is root read only,
> > as the CVE says.
> > 
> > NAK for this fix. Come up with something that doesn't break perf and
> > trace-cmd. That will not be trivial, as the format is stored in the
> > ring buffer with an address, then referenced directly. It also handles
> > trace_printk() functions that simply point to the string format itself.
> > 
> > A fix would require having a pointer be the same that is referenced
> > inside the kernel as well as in this file. Maybe make the format string
> > placed in a location that doesn't leak where the rest of the kernel
> > exists?
> > 
> > -- Steve
> Thank you Steve, much appreciated feedback, I have asked the security
> developers to keep this in mind and come up with a correct fix.
> 
> The correct fix that meets your guidelines would _not_ be suitable for
> stable due to the invasiveness it sounds, only for the latest will such a
> rework make sense. As such, the fix proposed in this patch is the only one
> that meets the bar for stable patch simplicity, and merely(!) needs to state
> that if the fix is taken, perf and trace are broken.

Why would I take something for the stable trees that does not match what
is upstream?  It feels to me that this CVE is just invalid.  Yes, root
can read the kernel address, does that mean it is a problem?  Only if
you allow unprotected users to run with root privileges :)

What exactly is the problem here in the current kernel that you are
trying to solve?

thanks,

greg k-h

  parent reply	other threads:[~2018-07-26 15:32 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-25 20:22 [PATCH] tracing: do not leak kernel addresses Mark Salyzyn
2018-07-25 21:14 ` Nick Desaulniers
2018-07-26  1:07 ` Steven Rostedt
2018-07-26 15:14   ` Mark Salyzyn
2018-07-26 15:22     ` Steven Rostedt
2018-07-26 16:32       ` Nick Desaulniers
2018-07-26 16:37         ` Steven Rostedt
2018-07-26 16:59           ` Nick Desaulniers
2018-07-26 21:56             ` Nick Desaulniers
2018-07-26 15:31     ` Greg KH [this message]
2018-07-26 16:52       ` Nick Desaulniers
2018-07-26 22:15         ` Steven Rostedt
2018-07-27 12:07           ` Jordan Glover
2018-07-27 13:40             ` Jann Horn
2018-07-27 13:47               ` Steven Rostedt
2018-07-27 18:13                 ` Nick Desaulniers
2018-07-27 18:31                   ` Steven Rostedt
     [not found]                     ` <CAMx4XWv3OazvURuN1XU2+5C5tNDzPuTniMn_T=XTA4P8_uwS_A@mail.gmail.com>
2018-07-27 18:47                       ` Jann Horn
2018-07-27 18:58                         ` Jann Horn
2018-07-27 19:54                           ` Theodore Y. Ts'o
2018-07-27 20:11                             ` Steven Rostedt
2018-07-27 20:21                               ` Theodore Y. Ts'o
2018-07-27 20:53                                 ` Steven Rostedt
2018-07-27 22:05                                 ` Sandeep Patil
2018-07-28  0:04                                   ` Theodore Y. Ts'o
2018-07-30 14:35                                     ` Sandeep Patil

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=20180726153153.GA8327@kroah.com \
    --to=greg@kroah.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=ndesaulniers@google.com \
    --cc=rostedt@goodmis.org \
    --cc=salyzyn@android.com \
    --cc=stable@vger.kernel.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 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).