On 29/3/19 3:21 pm, cmr wrote:
Operations which write to memory should be restricted on secure systems
and optionally to avoid self-destructive behaviors.
For reference:
- https://github.com/linuxppc/issues/issues/219
- https://github.com/linuxppc/issues/issues/232
Perhaps clarify what is meant here by "secure systems".
Otherwise commit message looks good.
Add a config option, XMON_RO, to control default xmon behavior along
with kernel cmdline options xmon=ro and xmon=rw for explicit control.
The default is to enable read-only mode.
The following xmon operations are affected:
memops:
disable memmove
disable memset
memex:
no-op'd mwrite
super_regs:
no-op'd write_spr
bpt_cmds:
disable
proc_call:
disable
Signed-off-by: cmr <c...@informatik.wtf>
You have a git config to fix :)
---
arch/powerpc/Kconfig.debug | 7 +++++++
arch/powerpc/xmon/xmon.c | 24 ++++++++++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 4e00cb0a5464..33cc01adf4cb 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -117,6 +117,13 @@ config XMON_DISASSEMBLY
to say Y here, unless you're building for a memory-constrained
system.
+config XMON_RO
+ bool "Set xmon read-only mode"
+ depends on XMON
+ default y
+ help
+ Disable state- and memory-altering write operations in xmon.
The meaning of this option is a bit unclear.
From the code - it looks like what this option actually does is enable
RO mode *by default*. In which case it should probably be called
XMON_RO_DEFAULT and the description should note that RW mode can still
be enabled via a cmdline option.
+
config DEBUGGER
bool
depends on KGDB || XMON
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a0f44f992360..c13ee73cdfd4 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -80,6 +80,7 @@ static int set_indicator_token = RTAS_UNKNOWN_SERVICE;
#endif
static unsigned long in_xmon __read_mostly = 0;
static int xmon_on = IS_ENABLED(CONFIG_XMON_DEFAULT);
+static int xmon_ro = IS_ENABLED(CONFIG_XMON_RO);
static unsigned long adrs;
static int size = 1;
@@ -1042,6 +1043,8 @@ cmds(struct pt_regs *excp)
set_lpp_cmd();
break;
case 'b':
+ if (xmon_ro == 1)
+ break;
For all these cases - it would be much better to print an error message
somewhere when we abort due to read-only mode.
bpt_cmds();
break;
case 'C':
@@ -1055,6 +1058,8 @@ cmds(struct pt_regs *excp)
bootcmds();
break;
case 'p':
+ if (xmon_ro == 1)
+ break;
proccall();
break;
case 'P':
@@ -1777,6 +1782,9 @@ read_spr(int n, unsigned long *vp)
static void
write_spr(int n, unsigned long val)
{
+ if (xmon_ro == 1)
+ return;
+
if (setjmp(bus_error_jmp) == 0) {
catch_spr_faults = 1;
sync();
@@ -2016,6 +2024,10 @@ mwrite(unsigned long adrs, void *buf, int size)
char *p, *q;
n = 0;
+
+ if (xmon_ro == 1)
+ return n;
+
if (setjmp(bus_error_jmp) == 0) {
catch_memory_errors = 1;
sync();
@@ -2884,9 +2896,13 @@ memops(int cmd)
scanhex((void *)&mcount);
switch( cmd ){
case 'm':
+ if (xmon_ro == 1)
+ break;
memmove((void *)mdest, (void *)msrc, mcount);
break;
case 's':
+ if (xmon_ro == 1)
+ break;
memset((void *)mdest, mval, mcount);
break;
case 'd':
@@ -3796,6 +3812,14 @@ static int __init early_parse_xmon(char *p)
} else if (strncmp(p, "on", 2) == 0) {
xmon_init(1);
xmon_on = 1;
+ } else if (strncmp(p, "rw", 2) == 0) {
+ xmon_init(1);
+ xmon_on = 1;
+ xmon_ro = 0;
+ } else if (strncmp(p, "ro", 2) == 0) {
+ xmon_init(1);
+ xmon_on = 1;
+ xmon_ro = 1;
} else if (strncmp(p, "off", 3) == 0)
xmon_on = 0;
else
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com IBM Australia Limited
