> On April 11, 2019 at 8:37 AM Michael Ellerman <m...@ellerman.id.au> wrote: > > > Christopher M Riedl <c...@informatik.wtf> writes: > >> On April 8, 2019 at 1:34 AM Oliver <ooh...@gmail.com> wrote: > >> On Mon, Apr 8, 2019 at 1:06 PM Christopher M. Riedl <c...@informatik.wtf> > >> wrote: > ... > >> > > >> > diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug > >> > index 4e00cb0a5464..0c7f21476018 100644 > >> > --- a/arch/powerpc/Kconfig.debug > >> > +++ b/arch/powerpc/Kconfig.debug > >> > @@ -117,6 +117,16 @@ config XMON_DISASSEMBLY > >> > to say Y here, unless you're building for a memory-constrained > >> > system. > >> > > >> > >> > +config XMON_RW > >> > + bool "Allow xmon read and write operations" > >> > + depends on XMON > >> > + default !STRICT_KERNEL_RWX > >> > + help > >> > + Allow xmon to read and write to memory and special-purpose > >> > registers. > >> > + Conversely, prevent xmon write access when set to N. Read and > >> > write > >> > + access can also be explicitly controlled with 'xmon=rw' or > >> > 'xmon=ro' > >> > + (read-only) cmdline options. Default is !STRICT_KERNEL_RWX. > >> > >> Maybe I am a dumb, but I found this *extremely* confusing. > >> Conventionally Kconfig options will control what code is and is not > >> included in the kernel (see XMON_DISASSEMBLY) rather than changing the > >> default behaviour of code. It's not wrong to do so and I'm going to > >> assume that you were following the pattern of XMON_DEFAULT, but I > >> think you need to be a little more clear about what option actually > >> does. Renaming it to XMON_DEFAULT_RO_MODE and re-wording the > >> description to indicate it's a only a mode change would help a lot. > >> > >> Sorry if this comes across as pointless bikeshedding since it's the > >> opposite of what Christophe said in the last patch, but this was a bit > >> of a head scratcher. > > > > If anyone is dumb here it's me for making this confusing :) > > I chatted with Michael Ellerman about this, so let me try to explain this > > more clearly. > > Yeah it's my fault :) >
"Signed-off-by: Christopher M. Riedl" -- I take full responsibility hah. > > > There are two things I am trying to address with XMON_RW: > > 1) provide a default access mode for xmon based on system "security" > > I think I've gone off this idea. Tying them together is just enforcing a > linkage that people may not want. > > I think XMON_RW should just be an option that stands on its own. It > should probably be default n, to give people a safe default. > Next version includes this along with making it clear that this option provides the default mode for XMON. > > > 2) replace XMON in the decision to write-protect kernel text at compile-time > > We should do that as a separate patch. That's actually a bug in the > current STRICT_KERNEL_RWX support. > > ie. STRICT_KERNEL_RWX should always give you PAGE_KERNEL_ROX, regardless > of XMON or anything else. > > > I think a single Kconfig for both of those things is sensible as ultimately > > the > > point is to allow xmon to operate in read-only mode on "secure" systems -- > > without > > violating any integrity/security guarantees (such as write-protected kernel > > text). > > > > Christophe suggested looking at STRICT_KERNEL_RWX and I think that option > > makes the > > most sense to base XMON_RW on since the description for STRICT_KERNEL_RWX > > states: > > Once we fix the bugs in STRICT_KERNEL_RWX people are going to enable > that by default, so it will essentially be always on in future. > > > > With that said, I will remove the 'xmon=rw' cmdline option as it really > > doesn't work > > since kernel text is write-protected at compile time. > > I think 'xmon=rw' still makes sense. Only some of the RW functionality > relies on being able to patch kernel text. > > And once you have proccall() you can just call a function to make it > read/write anyway, or use memex to manually frob the page tables. > > cheers Great, adding this back in the next version.
