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 :) > 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. > 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
