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@xxxxxxxxxxx> wrote: > > > > > From: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > > > > > > 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
