Hello Jason,
Just as promised, I'm resending the series after the merge window.
This patchset implements "kiosk" mode for KDB debugger. The mode reduces kdb features, so that it is no longer possible to leak sensitive data via the debugger, and not possible to change program flow in a predefined manner by an ordinary user. Root can control the capability.
There are a few patches, some are just cleanups, some are churn-ish cleanups, but inevitable. And the rest implements the mode -- after all the preparations, everything is pretty straightforward.
Thanks! Anton.
-- include/linux/kdb.h | 20 ++-- kernel/debug/kdb/kdb_bp.c | 24 ++--- kernel/debug/kdb/kdb_main.c | 189 ++++++++++++++++++---------------- kernel/debug/kdb/kdb_private.h | 3 +- kernel/trace/trace_kdb.c | 4 +- 5 files changed, 125 insertions(+), 115 deletions(-)
The struct member is never used in the code, so we can remove it.
We will introduce real flags soon by renaming cmd_repeat to cmd_flags.
Signed-off-by: Anton Vorontsov anton.vorontsov@linaro.org --- kernel/debug/kdb/kdb_main.c | 1 - kernel/debug/kdb/kdb_private.h | 1 - 2 files changed, 2 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index 4d5f8d5..cdaaa52 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -2748,7 +2748,6 @@ int kdb_register_repeat(char *cmd, kp->cmd_func = func; kp->cmd_usage = usage; kp->cmd_help = help; - kp->cmd_flags = 0; kp->cmd_minlen = minlen; kp->cmd_repeat = repeat;
diff --git a/kernel/debug/kdb/kdb_private.h b/kernel/debug/kdb/kdb_private.h index 392ec6a..f8245b3 100644 --- a/kernel/debug/kdb/kdb_private.h +++ b/kernel/debug/kdb/kdb_private.h @@ -175,7 +175,6 @@ typedef struct _kdbtab { kdb_func_t cmd_func; /* Function to execute command */ char *cmd_usage; /* Usage String for this command */ char *cmd_help; /* Help message for this command */ - short cmd_flags; /* Parsing flags */ short cmd_minlen; /* Minimum legal # command * chars required */ kdb_repeat_t cmd_repeat; /* Does command auto repeat on enter? */
We're about to add more options for command behaviour, so let's expand the meaning of kdb_repeat_t.
So far we just do various renames, there should be no functional changes.
Signed-off-by: Anton Vorontsov anton.vorontsov@linaro.org --- include/linux/kdb.h | 4 ++-- kernel/debug/kdb/kdb_main.c | 6 +++--- kernel/debug/kdb/kdb_private.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h index 7f6fe6e..cbd1c28 100644 --- a/include/linux/kdb.h +++ b/include/linux/kdb.h @@ -17,7 +17,7 @@ typedef enum { KDB_REPEAT_NONE = 0, /* Do not repeat this command */ KDB_REPEAT_NO_ARGS, /* Repeat the command without arguments */ KDB_REPEAT_WITH_ARGS, /* Repeat the command including its arguments */ -} kdb_repeat_t; +} kdb_cmdflags_t;
typedef int (*kdb_func_t)(int, const char **);
@@ -146,7 +146,7 @@ static inline const char *kdb_walk_kallsyms(loff_t *pos) /* Dynamic kdb shell command registration */ extern int kdb_register(char *, kdb_func_t, char *, char *, short); extern int kdb_register_repeat(char *, kdb_func_t, char *, char *, - short, kdb_repeat_t); + short, kdb_cmdflags_t); extern int kdb_unregister(char *); #else /* ! CONFIG_KGDB_KDB */ static inline __printf(1, 2) int kdb_printf(const char *fmt, ...) { return 0; } diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index cdaaa52..c7a1797 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -992,7 +992,7 @@ int kdb_parse(const char *cmdstr) if (result && ignore_errors && result > KDB_CMD_GO) result = 0; KDB_STATE_CLEAR(CMD); - switch (tp->cmd_repeat) { + switch (tp->cmd_flags) { case KDB_REPEAT_NONE: argc = 0; if (argv[0]) @@ -2700,7 +2700,7 @@ int kdb_register_repeat(char *cmd, char *usage, char *help, short minlen, - kdb_repeat_t repeat) + kdb_cmdflags_t flags) { int i; kdbtab_t *kp; @@ -2749,7 +2749,7 @@ int kdb_register_repeat(char *cmd, kp->cmd_usage = usage; kp->cmd_help = help; kp->cmd_minlen = minlen; - kp->cmd_repeat = repeat; + kp->cmd_flags = flags;
return 0; } diff --git a/kernel/debug/kdb/kdb_private.h b/kernel/debug/kdb/kdb_private.h index f8245b3..9e1b8e9 100644 --- a/kernel/debug/kdb/kdb_private.h +++ b/kernel/debug/kdb/kdb_private.h @@ -177,7 +177,7 @@ typedef struct _kdbtab { char *cmd_help; /* Help message for this command */ short cmd_minlen; /* Minimum legal # command * chars required */ - kdb_repeat_t cmd_repeat; /* Does command auto repeat on enter? */ + kdb_cmdflags_t cmd_flags; /* Command behaviour flags */ } kdbtab_t;
extern int kdb_bt(int, const char **); /* KDB display back trace */
We're about to add more options for commands behaviour, so let's give a more generic name to the low-level kdb command registration function.
There are just various renames, no functional changes.
Signed-off-by: Anton Vorontsov anton.vorontsov@linaro.org --- include/linux/kdb.h | 10 +++--- kernel/debug/kdb/kdb_bp.c | 16 ++++----- kernel/debug/kdb/kdb_main.c | 88 ++++++++++++++++++++++----------------------- kernel/trace/trace_kdb.c | 2 +- 4 files changed, 58 insertions(+), 58 deletions(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h index cbd1c28..0142cd3 100644 --- a/include/linux/kdb.h +++ b/include/linux/kdb.h @@ -145,17 +145,17 @@ static inline const char *kdb_walk_kallsyms(loff_t *pos)
/* Dynamic kdb shell command registration */ extern int kdb_register(char *, kdb_func_t, char *, char *, short); -extern int kdb_register_repeat(char *, kdb_func_t, char *, char *, - short, kdb_cmdflags_t); +extern int kdb_register_flags(char *, kdb_func_t, char *, char *, + short, kdb_cmdflags_t); extern int kdb_unregister(char *); #else /* ! CONFIG_KGDB_KDB */ static inline __printf(1, 2) int kdb_printf(const char *fmt, ...) { return 0; } static inline void kdb_init(int level) {} static inline int kdb_register(char *cmd, kdb_func_t func, char *usage, char *help, short minlen) { return 0; } -static inline int kdb_register_repeat(char *cmd, kdb_func_t func, char *usage, - char *help, short minlen, - kdb_repeat_t repeat) { return 0; } +static inline int kdb_register_flags(char *cmd, kdb_func_t func, char *usage, + char *help, short minlen, + kdb_repeat_t repeat) { return 0; } static inline int kdb_unregister(char *cmd) { return 0; } #endif /* CONFIG_KGDB_KDB */ enum { diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c index 8418c2f..d2cb80d 100644 --- a/kernel/debug/kdb/kdb_bp.c +++ b/kernel/debug/kdb/kdb_bp.c @@ -545,23 +545,23 @@ void __init kdb_initbptab(void) for (i = 0, bp = kdb_breakpoints; i < KDB_MAXBPT; i++, bp++) bp->bp_free = 1;
- kdb_register_repeat("bp", kdb_bp, "[<vaddr>]", + kdb_register_flags("bp", kdb_bp, "[<vaddr>]", "Set/Display breakpoints", 0, KDB_REPEAT_NO_ARGS); - kdb_register_repeat("bl", kdb_bp, "[<vaddr>]", + kdb_register_flags("bl", kdb_bp, "[<vaddr>]", "Display breakpoints", 0, KDB_REPEAT_NO_ARGS); if (arch_kgdb_ops.flags & KGDB_HW_BREAKPOINT) - kdb_register_repeat("bph", kdb_bp, "[<vaddr>]", + kdb_register_flags("bph", kdb_bp, "[<vaddr>]", "[datar [length]|dataw [length]] Set hw brk", 0, KDB_REPEAT_NO_ARGS); - kdb_register_repeat("bc", kdb_bc, "<bpnum>", + kdb_register_flags("bc", kdb_bc, "<bpnum>", "Clear Breakpoint", 0, KDB_REPEAT_NONE); - kdb_register_repeat("be", kdb_bc, "<bpnum>", + kdb_register_flags("be", kdb_bc, "<bpnum>", "Enable Breakpoint", 0, KDB_REPEAT_NONE); - kdb_register_repeat("bd", kdb_bc, "<bpnum>", + kdb_register_flags("bd", kdb_bc, "<bpnum>", "Disable Breakpoint", 0, KDB_REPEAT_NONE);
- kdb_register_repeat("ss", kdb_ss, "", + kdb_register_flags("ss", kdb_ss, "", "Single Step", 1, KDB_REPEAT_NO_ARGS); - kdb_register_repeat("ssb", kdb_ss, "", + kdb_register_flags("ssb", kdb_ss, "", "Single step to branch/call", 0, KDB_REPEAT_NO_ARGS); /* * Architecture dependent initialization. diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index c7a1797..bae9a1d 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -2683,7 +2683,7 @@ static int kdb_grep_help(int argc, const char **argv) }
/* - * kdb_register_repeat - This function is used to register a kernel + * kdb_register_flags - This function is used to register a kernel * debugger command. * Inputs: * cmd Command name @@ -2695,12 +2695,12 @@ static int kdb_grep_help(int argc, const char **argv) * zero for success, one if a duplicate command. */ #define kdb_command_extend 50 /* arbitrary */ -int kdb_register_repeat(char *cmd, - kdb_func_t func, - char *usage, - char *help, - short minlen, - kdb_cmdflags_t flags) +int kdb_register_flags(char *cmd, + kdb_func_t func, + char *usage, + char *help, + short minlen, + kdb_cmdflags_t flags) { int i; kdbtab_t *kp; @@ -2753,13 +2753,13 @@ int kdb_register_repeat(char *cmd,
return 0; } -EXPORT_SYMBOL_GPL(kdb_register_repeat); +EXPORT_SYMBOL_GPL(kdb_register_flags);
/* * kdb_register - Compatibility register function for commands that do * not need to specify a repeat state. Equivalent to - * kdb_register_repeat with KDB_REPEAT_NONE. + * kdb_register_flags with KDB_REPEAT_NONE. * Inputs: * cmd Command name * func Function to execute the command @@ -2774,8 +2774,8 @@ int kdb_register(char *cmd, char *help, short minlen) { - return kdb_register_repeat(cmd, func, usage, help, minlen, - KDB_REPEAT_NONE); + return kdb_register_flags(cmd, func, usage, help, minlen, + KDB_REPEAT_NONE); } EXPORT_SYMBOL_GPL(kdb_register);
@@ -2817,81 +2817,81 @@ static void __init kdb_inittab(void) for_each_kdbcmd(kp, i) kp->cmd_name = NULL;
- kdb_register_repeat("md", kdb_md, "<vaddr>", + kdb_register_flags("md", kdb_md, "<vaddr>", "Display Memory Contents, also mdWcN, e.g. md8c1", 1, KDB_REPEAT_NO_ARGS); - kdb_register_repeat("mdr", kdb_md, "<vaddr> <bytes>", + kdb_register_flags("mdr", kdb_md, "<vaddr> <bytes>", "Display Raw Memory", 0, KDB_REPEAT_NO_ARGS); - kdb_register_repeat("mdp", kdb_md, "<paddr> <bytes>", + kdb_register_flags("mdp", kdb_md, "<paddr> <bytes>", "Display Physical Memory", 0, KDB_REPEAT_NO_ARGS); - kdb_register_repeat("mds", kdb_md, "<vaddr>", + kdb_register_flags("mds", kdb_md, "<vaddr>", "Display Memory Symbolically", 0, KDB_REPEAT_NO_ARGS); - kdb_register_repeat("mm", kdb_mm, "<vaddr> <contents>", + kdb_register_flags("mm", kdb_mm, "<vaddr> <contents>", "Modify Memory Contents", 0, KDB_REPEAT_NO_ARGS); - kdb_register_repeat("go", kdb_go, "[<vaddr>]", + kdb_register_flags("go", kdb_go, "[<vaddr>]", "Continue Execution", 1, KDB_REPEAT_NONE); - kdb_register_repeat("rd", kdb_rd, "", + kdb_register_flags("rd", kdb_rd, "", "Display Registers", 0, KDB_REPEAT_NONE); - kdb_register_repeat("rm", kdb_rm, "<reg> <contents>", + kdb_register_flags("rm", kdb_rm, "<reg> <contents>", "Modify Registers", 0, KDB_REPEAT_NONE); - kdb_register_repeat("ef", kdb_ef, "<vaddr>", + kdb_register_flags("ef", kdb_ef, "<vaddr>", "Display exception frame", 0, KDB_REPEAT_NONE); - kdb_register_repeat("bt", kdb_bt, "[<vaddr>]", + kdb_register_flags("bt", kdb_bt, "[<vaddr>]", "Stack traceback", 1, KDB_REPEAT_NONE); - kdb_register_repeat("btp", kdb_bt, "<pid>", + kdb_register_flags("btp", kdb_bt, "<pid>", "Display stack for process <pid>", 0, KDB_REPEAT_NONE); - kdb_register_repeat("bta", kdb_bt, "[DRSTCZEUIMA]", + kdb_register_flags("bta", kdb_bt, "[DRSTCZEUIMA]", "Display stack all processes", 0, KDB_REPEAT_NONE); - kdb_register_repeat("btc", kdb_bt, "", + kdb_register_flags("btc", kdb_bt, "", "Backtrace current process on each cpu", 0, KDB_REPEAT_NONE); - kdb_register_repeat("btt", kdb_bt, "<vaddr>", + kdb_register_flags("btt", kdb_bt, "<vaddr>", "Backtrace process given its struct task address", 0, KDB_REPEAT_NONE); - kdb_register_repeat("ll", kdb_ll, "<first-element> <linkoffset> <cmd>", + kdb_register_flags("ll", kdb_ll, "<first-element> <linkoffset> <cmd>", "Execute cmd for each element in linked list", 0, KDB_REPEAT_NONE); - kdb_register_repeat("env", kdb_env, "", + kdb_register_flags("env", kdb_env, "", "Show environment variables", 0, KDB_REPEAT_NONE); - kdb_register_repeat("set", kdb_set, "", + kdb_register_flags("set", kdb_set, "", "Set environment variables", 0, KDB_REPEAT_NONE); - kdb_register_repeat("help", kdb_help, "", + kdb_register_flags("help", kdb_help, "", "Display Help Message", 1, KDB_REPEAT_NONE); - kdb_register_repeat("?", kdb_help, "", + kdb_register_flags("?", kdb_help, "", "Display Help Message", 0, KDB_REPEAT_NONE); - kdb_register_repeat("cpu", kdb_cpu, "<cpunum>", + kdb_register_flags("cpu", kdb_cpu, "<cpunum>", "Switch to new cpu", 0, KDB_REPEAT_NONE); - kdb_register_repeat("kgdb", kdb_kgdb, "", + kdb_register_flags("kgdb", kdb_kgdb, "", "Enter kgdb mode", 0, KDB_REPEAT_NONE); - kdb_register_repeat("ps", kdb_ps, "[<flags>|A]", + kdb_register_flags("ps", kdb_ps, "[<flags>|A]", "Display active task list", 0, KDB_REPEAT_NONE); - kdb_register_repeat("pid", kdb_pid, "<pidnum>", + kdb_register_flags("pid", kdb_pid, "<pidnum>", "Switch to another task", 0, KDB_REPEAT_NONE); - kdb_register_repeat("reboot", kdb_reboot, "", + kdb_register_flags("reboot", kdb_reboot, "", "Reboot the machine immediately", 0, KDB_REPEAT_NONE); #if defined(CONFIG_MODULES) - kdb_register_repeat("lsmod", kdb_lsmod, "", + kdb_register_flags("lsmod", kdb_lsmod, "", "List loaded kernel modules", 0, KDB_REPEAT_NONE); #endif #if defined(CONFIG_MAGIC_SYSRQ) - kdb_register_repeat("sr", kdb_sr, "<key>", + kdb_register_flags("sr", kdb_sr, "<key>", "Magic SysRq key", 0, KDB_REPEAT_NONE); #endif #if defined(CONFIG_PRINTK) - kdb_register_repeat("dmesg", kdb_dmesg, "[lines]", + kdb_register_flags("dmesg", kdb_dmesg, "[lines]", "Display syslog buffer", 0, KDB_REPEAT_NONE); #endif if (arch_kgdb_ops.enable_nmi) { - kdb_register_repeat("disable_nmi", kdb_disable_nmi, "", + kdb_register_flags("disable_nmi", kdb_disable_nmi, "", "Disable NMI entry to KDB", 0, KDB_REPEAT_NONE); } - kdb_register_repeat("defcmd", kdb_defcmd, "name "usage" "help"", + kdb_register_flags("defcmd", kdb_defcmd, "name "usage" "help"", "Define a set of commands, down to endefcmd", 0, KDB_REPEAT_NONE); - kdb_register_repeat("kill", kdb_kill, "<-signal> <pid>", + kdb_register_flags("kill", kdb_kill, "<-signal> <pid>", "Send a signal to a process", 0, KDB_REPEAT_NONE); - kdb_register_repeat("summary", kdb_summary, "", + kdb_register_flags("summary", kdb_summary, "", "Summarize the system", 4, KDB_REPEAT_NONE); - kdb_register_repeat("per_cpu", kdb_per_cpu, "<sym> [<bytes>] [<cpu>]", + kdb_register_flags("per_cpu", kdb_per_cpu, "<sym> [<bytes>] [<cpu>]", "Display per_cpu variables", 3, KDB_REPEAT_NONE); - kdb_register_repeat("grephelp", kdb_grep_help, "", + kdb_register_flags("grephelp", kdb_grep_help, "", "Display help on | grep", 0, KDB_REPEAT_NONE); }
diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c index 3c5c5df..e9db346 100644 --- a/kernel/trace/trace_kdb.c +++ b/kernel/trace/trace_kdb.c @@ -127,7 +127,7 @@ static int kdb_ftdump(int argc, const char **argv)
static __init int kdb_ftrace_register(void) { - kdb_register_repeat("ftdump", kdb_ftdump, "[skip_#lines] [cpu]", + kdb_register_flags("ftdump", kdb_ftdump, "[skip_#lines] [cpu]", "Dump ftrace log", 0, KDB_REPEAT_NONE); return 0; }
The actual values of KDB_REPEAT_* enum values and overall logic stayed the same, but we now treat the values as flags.
This makes it possible to add other flags and combine them, plus makes the code a lot simpler and shorter. But functionality-wise, there should be no changes.
Signed-off-by: Anton Vorontsov anton.vorontsov@linaro.org --- include/linux/kdb.h | 4 ++-- kernel/debug/kdb/kdb_main.c | 21 +++++++-------------- 2 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h index 0142cd3..c6f1ec3 100644 --- a/include/linux/kdb.h +++ b/include/linux/kdb.h @@ -15,8 +15,8 @@
typedef enum { KDB_REPEAT_NONE = 0, /* Do not repeat this command */ - KDB_REPEAT_NO_ARGS, /* Repeat the command without arguments */ - KDB_REPEAT_WITH_ARGS, /* Repeat the command including its arguments */ + KDB_REPEAT_NO_ARGS = 0x1, /* Repeat the command w/o arguments */ + KDB_REPEAT_WITH_ARGS = 0x2, /* Repeat the command w/ its arguments */ } kdb_cmdflags_t;
typedef int (*kdb_func_t)(int, const char **); diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index bae9a1d..7245bab 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -992,20 +992,13 @@ int kdb_parse(const char *cmdstr) if (result && ignore_errors && result > KDB_CMD_GO) result = 0; KDB_STATE_CLEAR(CMD); - switch (tp->cmd_flags) { - case KDB_REPEAT_NONE: - argc = 0; - if (argv[0]) - *(argv[0]) = '\0'; - break; - case KDB_REPEAT_NO_ARGS: - argc = 1; - if (argv[1]) - *(argv[1]) = '\0'; - break; - case KDB_REPEAT_WITH_ARGS: - break; - } + + if (tp->cmd_flags & KDB_REPEAT_WITH_ARGS) + return result; + + argc = tp->cmd_flags & KDB_REPEAT_NO_ARGS ? 1 : 0; + if (argv[argc]) + *(argv[argc]) = '\0'; return result; }
Since we now treat KDB_REPEAT_* as flags, there is no need to pass KDB_REPEAT_NONE. It's just the default behaviour when no flags are specified.
Signed-off-by: Anton Vorontsov anton.vorontsov@linaro.org --- include/linux/kdb.h | 1 - kernel/debug/kdb/kdb_bp.c | 6 ++--- kernel/debug/kdb/kdb_main.c | 61 ++++++++++++++++++++++----------------------- kernel/trace/trace_kdb.c | 2 +- 4 files changed, 34 insertions(+), 36 deletions(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h index c6f1ec3..792779c 100644 --- a/include/linux/kdb.h +++ b/include/linux/kdb.h @@ -14,7 +14,6 @@ */
typedef enum { - KDB_REPEAT_NONE = 0, /* Do not repeat this command */ KDB_REPEAT_NO_ARGS = 0x1, /* Repeat the command w/o arguments */ KDB_REPEAT_WITH_ARGS = 0x2, /* Repeat the command w/ its arguments */ } kdb_cmdflags_t; diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c index d2cb80d..928e9e9 100644 --- a/kernel/debug/kdb/kdb_bp.c +++ b/kernel/debug/kdb/kdb_bp.c @@ -553,11 +553,11 @@ void __init kdb_initbptab(void) kdb_register_flags("bph", kdb_bp, "[<vaddr>]", "[datar [length]|dataw [length]] Set hw brk", 0, KDB_REPEAT_NO_ARGS); kdb_register_flags("bc", kdb_bc, "<bpnum>", - "Clear Breakpoint", 0, KDB_REPEAT_NONE); + "Clear Breakpoint", 0, 0); kdb_register_flags("be", kdb_bc, "<bpnum>", - "Enable Breakpoint", 0, KDB_REPEAT_NONE); + "Enable Breakpoint", 0, 0); kdb_register_flags("bd", kdb_bc, "<bpnum>", - "Disable Breakpoint", 0, KDB_REPEAT_NONE); + "Disable Breakpoint", 0, 0);
kdb_register_flags("ss", kdb_ss, "", "Single Step", 1, KDB_REPEAT_NO_ARGS); diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index 7245bab..172b726 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -2752,7 +2752,7 @@ EXPORT_SYMBOL_GPL(kdb_register_flags); /* * kdb_register - Compatibility register function for commands that do * not need to specify a repeat state. Equivalent to - * kdb_register_flags with KDB_REPEAT_NONE. + * kdb_register_flags with flags set to 0. * Inputs: * cmd Command name * func Function to execute the command @@ -2767,8 +2767,7 @@ int kdb_register(char *cmd, char *help, short minlen) { - return kdb_register_flags(cmd, func, usage, help, minlen, - KDB_REPEAT_NONE); + return kdb_register_flags(cmd, func, usage, help, minlen, 0); } EXPORT_SYMBOL_GPL(kdb_register);
@@ -2822,70 +2821,70 @@ static void __init kdb_inittab(void) kdb_register_flags("mm", kdb_mm, "<vaddr> <contents>", "Modify Memory Contents", 0, KDB_REPEAT_NO_ARGS); kdb_register_flags("go", kdb_go, "[<vaddr>]", - "Continue Execution", 1, KDB_REPEAT_NONE); + "Continue Execution", 1, 0); kdb_register_flags("rd", kdb_rd, "", - "Display Registers", 0, KDB_REPEAT_NONE); + "Display Registers", 0, 0); kdb_register_flags("rm", kdb_rm, "<reg> <contents>", - "Modify Registers", 0, KDB_REPEAT_NONE); + "Modify Registers", 0, 0); kdb_register_flags("ef", kdb_ef, "<vaddr>", - "Display exception frame", 0, KDB_REPEAT_NONE); + "Display exception frame", 0, 0); kdb_register_flags("bt", kdb_bt, "[<vaddr>]", - "Stack traceback", 1, KDB_REPEAT_NONE); + "Stack traceback", 1, 0); kdb_register_flags("btp", kdb_bt, "<pid>", - "Display stack for process <pid>", 0, KDB_REPEAT_NONE); + "Display stack for process <pid>", 0, 0); kdb_register_flags("bta", kdb_bt, "[DRSTCZEUIMA]", - "Display stack all processes", 0, KDB_REPEAT_NONE); + "Display stack all processes", 0, 0); kdb_register_flags("btc", kdb_bt, "", - "Backtrace current process on each cpu", 0, KDB_REPEAT_NONE); + "Backtrace current process on each cpu", 0, 0); kdb_register_flags("btt", kdb_bt, "<vaddr>", "Backtrace process given its struct task address", 0, - KDB_REPEAT_NONE); + 0); kdb_register_flags("ll", kdb_ll, "<first-element> <linkoffset> <cmd>", - "Execute cmd for each element in linked list", 0, KDB_REPEAT_NONE); + "Execute cmd for each element in linked list", 0, 0); kdb_register_flags("env", kdb_env, "", - "Show environment variables", 0, KDB_REPEAT_NONE); + "Show environment variables", 0, 0); kdb_register_flags("set", kdb_set, "", - "Set environment variables", 0, KDB_REPEAT_NONE); + "Set environment variables", 0, 0); kdb_register_flags("help", kdb_help, "", - "Display Help Message", 1, KDB_REPEAT_NONE); + "Display Help Message", 1, 0); kdb_register_flags("?", kdb_help, "", - "Display Help Message", 0, KDB_REPEAT_NONE); + "Display Help Message", 0, 0); kdb_register_flags("cpu", kdb_cpu, "<cpunum>", - "Switch to new cpu", 0, KDB_REPEAT_NONE); + "Switch to new cpu", 0, 0); kdb_register_flags("kgdb", kdb_kgdb, "", - "Enter kgdb mode", 0, KDB_REPEAT_NONE); + "Enter kgdb mode", 0, 0); kdb_register_flags("ps", kdb_ps, "[<flags>|A]", - "Display active task list", 0, KDB_REPEAT_NONE); + "Display active task list", 0, 0); kdb_register_flags("pid", kdb_pid, "<pidnum>", - "Switch to another task", 0, KDB_REPEAT_NONE); + "Switch to another task", 0, 0); kdb_register_flags("reboot", kdb_reboot, "", - "Reboot the machine immediately", 0, KDB_REPEAT_NONE); + "Reboot the machine immediately", 0, 0); #if defined(CONFIG_MODULES) kdb_register_flags("lsmod", kdb_lsmod, "", - "List loaded kernel modules", 0, KDB_REPEAT_NONE); + "List loaded kernel modules", 0, 0); #endif #if defined(CONFIG_MAGIC_SYSRQ) kdb_register_flags("sr", kdb_sr, "<key>", - "Magic SysRq key", 0, KDB_REPEAT_NONE); + "Magic SysRq key", 0, 0); #endif #if defined(CONFIG_PRINTK) kdb_register_flags("dmesg", kdb_dmesg, "[lines]", - "Display syslog buffer", 0, KDB_REPEAT_NONE); + "Display syslog buffer", 0, 0); #endif if (arch_kgdb_ops.enable_nmi) { kdb_register_flags("disable_nmi", kdb_disable_nmi, "", - "Disable NMI entry to KDB", 0, KDB_REPEAT_NONE); + "Disable NMI entry to KDB", 0, 0); } kdb_register_flags("defcmd", kdb_defcmd, "name "usage" "help"", - "Define a set of commands, down to endefcmd", 0, KDB_REPEAT_NONE); + "Define a set of commands, down to endefcmd", 0, 0); kdb_register_flags("kill", kdb_kill, "<-signal> <pid>", - "Send a signal to a process", 0, KDB_REPEAT_NONE); + "Send a signal to a process", 0, 0); kdb_register_flags("summary", kdb_summary, "", - "Summarize the system", 4, KDB_REPEAT_NONE); + "Summarize the system", 4, 0); kdb_register_flags("per_cpu", kdb_per_cpu, "<sym> [<bytes>] [<cpu>]", - "Display per_cpu variables", 3, KDB_REPEAT_NONE); + "Display per_cpu variables", 3, 0); kdb_register_flags("grephelp", kdb_grep_help, "", - "Display help on | grep", 0, KDB_REPEAT_NONE); + "Display help on | grep", 0, 0); }
/* Execute any commands defined in kdb_cmds. */ diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c index e9db346..1b68177 100644 --- a/kernel/trace/trace_kdb.c +++ b/kernel/trace/trace_kdb.c @@ -128,7 +128,7 @@ static int kdb_ftdump(int argc, const char **argv) static __init int kdb_ftrace_register(void) { kdb_register_flags("ftdump", kdb_ftdump, "[skip_#lines] [cpu]", - "Dump ftrace log", 0, KDB_REPEAT_NONE); + "Dump ftrace log", 0, 0); return 0; }
This patch introduces two new flags: KDB_SAFE, denotes a safe command, and KDB_SAFE_NO_ARGS, denotes a safe command when used without arguments.
The word "safe" here used in the sense that the commands cannot be used to leak sensitive data from the memory, and cannot be used to change program flow in a predefined manner.
These flags will be used by the "kiosk" mode, i.e. when it is possible for the ordinary user to enter the KDB (or user can get the access to KDB after the crash), but we do not allow user to read dump the memory [and thus read some sensitive data].
The following commands were marked as "safe":
Display exception frame Stack traceback Display stack for process Display stack all processes Backtrace current process on each cpu Execute cmd for each element in linked list Show environment variables Set environment variables Display Help Message Switch to new cpu Display active task list Switch to another task Reboot the machine immediately List loaded kernel modules Magic SysRq key Display syslog buffer Define a set of commands, down to endefcmd Summarize the system Disable NMI entry to KDB
The following commands were marked as safe when issued with no arguments:
Continue Execution
And the following commands are unsafe:
Clear Breakpoint Enable Breakpoint Disable Breakpoint Single step Single step to branch/call Continue Execution (with address argument) Display Memory Contents Display Raw Memory Display Physical Memory Display Memory Symbolically Modify Memory Contents Display Registers Modify Registers Backtrace process given its struct task address Send a signal to a process Enter kgdb mode Display per_cpu variables
Note that we mark "display registers" command unsafe, this is because single stepping + constantly dumping registers in string or memory functions can be used as a way to read sensitive data (it's actually trivial to exploit). Later we can do a bit better, i.e. not displaying general-purpose registers, but printing control registers.
Signed-off-by: Anton Vorontsov anton.vorontsov@linaro.org --- include/linux/kdb.h | 2 ++ kernel/debug/kdb/kdb_main.c | 44 ++++++++++++++++++++++---------------------- kernel/trace/trace_kdb.c | 2 +- 3 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h index 792779c..abe927c 100644 --- a/include/linux/kdb.h +++ b/include/linux/kdb.h @@ -16,6 +16,8 @@ typedef enum { KDB_REPEAT_NO_ARGS = 0x1, /* Repeat the command w/o arguments */ KDB_REPEAT_WITH_ARGS = 0x2, /* Repeat the command w/ its arguments */ + KDB_SAFE = 0x4, /* Security-wise safe command */ + KDB_SAFE_NO_ARGS = 0x8, /* Only safe if run w/o arguments */ } kdb_cmdflags_t;
typedef int (*kdb_func_t)(int, const char **); diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index 172b726..83c3f60 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -2821,70 +2821,70 @@ static void __init kdb_inittab(void) kdb_register_flags("mm", kdb_mm, "<vaddr> <contents>", "Modify Memory Contents", 0, KDB_REPEAT_NO_ARGS); kdb_register_flags("go", kdb_go, "[<vaddr>]", - "Continue Execution", 1, 0); + "Continue Execution", 1, KDB_SAFE_NO_ARGS); kdb_register_flags("rd", kdb_rd, "", "Display Registers", 0, 0); kdb_register_flags("rm", kdb_rm, "<reg> <contents>", "Modify Registers", 0, 0); kdb_register_flags("ef", kdb_ef, "<vaddr>", - "Display exception frame", 0, 0); + "Display exception frame", 0, KDB_SAFE); kdb_register_flags("bt", kdb_bt, "[<vaddr>]", - "Stack traceback", 1, 0); + "Stack traceback", 1, KDB_SAFE); kdb_register_flags("btp", kdb_bt, "<pid>", - "Display stack for process <pid>", 0, 0); + "Display stack for process <pid>", 0, KDB_SAFE); kdb_register_flags("bta", kdb_bt, "[DRSTCZEUIMA]", - "Display stack all processes", 0, 0); + "Display stack all processes", 0, KDB_SAFE); kdb_register_flags("btc", kdb_bt, "", - "Backtrace current process on each cpu", 0, 0); + "Backtrace current process on each cpu", 0, KDB_SAFE); kdb_register_flags("btt", kdb_bt, "<vaddr>", "Backtrace process given its struct task address", 0, 0); kdb_register_flags("ll", kdb_ll, "<first-element> <linkoffset> <cmd>", - "Execute cmd for each element in linked list", 0, 0); + "Execute cmd for each element in linked list", 0, KDB_SAFE); kdb_register_flags("env", kdb_env, "", - "Show environment variables", 0, 0); + "Show environment variables", 0, KDB_SAFE); kdb_register_flags("set", kdb_set, "", - "Set environment variables", 0, 0); + "Set environment variables", 0, KDB_SAFE); kdb_register_flags("help", kdb_help, "", - "Display Help Message", 1, 0); + "Display Help Message", 1, KDB_SAFE); kdb_register_flags("?", kdb_help, "", - "Display Help Message", 0, 0); + "Display Help Message", 0, KDB_SAFE); kdb_register_flags("cpu", kdb_cpu, "<cpunum>", - "Switch to new cpu", 0, 0); + "Switch to new cpu", 0, KDB_SAFE); kdb_register_flags("kgdb", kdb_kgdb, "", "Enter kgdb mode", 0, 0); kdb_register_flags("ps", kdb_ps, "[<flags>|A]", - "Display active task list", 0, 0); + "Display active task list", 0, KDB_SAFE); kdb_register_flags("pid", kdb_pid, "<pidnum>", - "Switch to another task", 0, 0); + "Switch to another task", 0, KDB_SAFE); kdb_register_flags("reboot", kdb_reboot, "", - "Reboot the machine immediately", 0, 0); + "Reboot the machine immediately", 0, KDB_SAFE); #if defined(CONFIG_MODULES) kdb_register_flags("lsmod", kdb_lsmod, "", - "List loaded kernel modules", 0, 0); + "List loaded kernel modules", 0, KDB_SAFE); #endif #if defined(CONFIG_MAGIC_SYSRQ) kdb_register_flags("sr", kdb_sr, "<key>", - "Magic SysRq key", 0, 0); + "Magic SysRq key", 0, KDB_SAFE); #endif #if defined(CONFIG_PRINTK) kdb_register_flags("dmesg", kdb_dmesg, "[lines]", - "Display syslog buffer", 0, 0); + "Display syslog buffer", 0, KDB_SAFE); #endif if (arch_kgdb_ops.enable_nmi) { kdb_register_flags("disable_nmi", kdb_disable_nmi, "", - "Disable NMI entry to KDB", 0, 0); + "Disable NMI entry to KDB", 0, KDB_SAFE); } kdb_register_flags("defcmd", kdb_defcmd, "name "usage" "help"", - "Define a set of commands, down to endefcmd", 0, 0); + "Define a set of commands, down to endefcmd", 0, KDB_SAFE); kdb_register_flags("kill", kdb_kill, "<-signal> <pid>", "Send a signal to a process", 0, 0); kdb_register_flags("summary", kdb_summary, "", - "Summarize the system", 4, 0); + "Summarize the system", 4, KDB_SAFE); kdb_register_flags("per_cpu", kdb_per_cpu, "<sym> [<bytes>] [<cpu>]", "Display per_cpu variables", 3, 0); kdb_register_flags("grephelp", kdb_grep_help, "", - "Display help on | grep", 0, 0); + "Display help on | grep", 0, KDB_SAFE); }
/* Execute any commands defined in kdb_cmds. */ diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c index 1b68177..8353852 100644 --- a/kernel/trace/trace_kdb.c +++ b/kernel/trace/trace_kdb.c @@ -128,7 +128,7 @@ static int kdb_ftdump(int argc, const char **argv) static __init int kdb_ftrace_register(void) { kdb_register_flags("ftdump", kdb_ftdump, "[skip_#lines] [cpu]", - "Dump ftrace log", 0, 0); + "Dump ftrace log", 0, KDB_SAFE); return 0; }
By issuing 'echo 1 > /sys/module/kdb/parameters/kiosk' or booting with kdb.kiosk=1 kernel command line option, one can still have a somewhat usable debugging facility, but not fearing that the debugger can be used to easily gain root access or dump sensitive data.
Without the kiosk mode, obtaining the root rights via KDB is a matter of a few commands, and works everywhere. For example, log in as a normal user:
cbou:~$ id uid=1001(cbou) gid=1001(cbou) groups=1001(cbou)
Now enter KDB (for example via sysrq):
Entering kdb (current=0xffff8800065bc740, pid 920) due to Keyboard Entry kdb> ps 23 sleeping system daemon (state M) processes suppressed, use 'ps A' to see all. Task Addr Pid Parent [*] cpu State Thread Command 0xffff8800065bc740 920 919 1 0 R 0xffff8800065bca20 *bash
0xffff880007078000 1 0 0 0 S 0xffff8800070782e0 init [...snip...] 0xffff8800065be3c0 918 1 0 0 S 0xffff8800065be6a0 getty 0xffff8800065b9c80 919 1 0 0 S 0xffff8800065b9f60 login 0xffff8800065bc740 920 919 1 0 R 0xffff8800065bca20 *bash
All we need is the offset of cred pointers. We can look up the offset in the distro's kernel source, but it is unnecessary. We can just start dumping init's task_struct, until we see the process name:
kdb> md 0xffff880007078000 0xffff880007078000 0000000000000001 ffff88000703c000 ................ 0xffff880007078010 0040210000000002 0000000000000000 .....!@......... [...snip...] 0xffff8800070782b0 ffff8800073e0580 ffff8800073e0580 ..>.......>..... 0xffff8800070782c0 0000000074696e69 0000000000000000 init............
^ Here, 'init'. Creds are just above it, so the offset is 0x02b0.
Now we set up init's creds for our non-privileged shell:
kdb> mm 0xffff8800065bc740+0x02b0 0xffff8800073e0580 0xffff8800065bc9f0 = 0xffff8800073e0580 kdb> mm 0xffff8800065bc740+0x02b8 0xffff8800073e0580 0xffff8800065bc9f8 = 0xffff8800073e0580
And thus gaining the root:
kdb> go cbou:~$ id uid=0(root) gid=0(root) groups=0(root) cbou:~$ bash root:~#
p.s. No distro enables kdb by default (although, with a nice KDB-over-KMS feature availability, I would expect at least some would enable it), so it's not actually some kind of a major issue.
Signed-off-by: Anton Vorontsov anton.vorontsov@linaro.org --- include/linux/kdb.h | 1 + kernel/debug/kdb/kdb_main.c | 20 +++++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h index abe927c..3a2c554 100644 --- a/include/linux/kdb.h +++ b/include/linux/kdb.h @@ -63,6 +63,7 @@ extern atomic_t kdb_event; #define KDB_BADLENGTH (-19) #define KDB_NOBP (-20) #define KDB_BADADDR (-21) +#define KDB_NOPERM (-22)
/* * kdb_diemsg diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index 83c3f60..36e4c2a 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -12,6 +12,7 @@ */
#include <linux/ctype.h> +#include <linux/types.h> #include <linux/string.h> #include <linux/kernel.h> #include <linux/kmsg_dump.h> @@ -23,6 +24,7 @@ #include <linux/vmalloc.h> #include <linux/atomic.h> #include <linux/module.h> +#include <linux/moduleparam.h> #include <linux/mm.h> #include <linux/init.h> #include <linux/kallsyms.h> @@ -42,6 +44,12 @@ #include <linux/slab.h> #include "kdb_private.h"
+#undef MODULE_PARAM_PREFIX +#define MODULE_PARAM_PREFIX "kdb." + +static bool kdb_kiosk; +module_param_named(kiosk, kdb_kiosk, bool, 0600); + #define GREP_LEN 256 char kdb_grep_string[GREP_LEN]; int kdb_grepping_flag; @@ -121,6 +129,7 @@ static kdbmsg_t kdbmsgs[] = { KDBMSG(BADLENGTH, "Invalid length field"), KDBMSG(NOBP, "No Breakpoint exists"), KDBMSG(BADADDR, "Invalid address"), + KDBMSG(NOPERM, "Permission denied"), }; #undef KDBMSG
@@ -987,6 +996,14 @@ int kdb_parse(const char *cmdstr)
if (i < kdb_max_commands) { int result; + + if (kdb_kiosk) { + if (!(tp->cmd_flags & (KDB_SAFE | KDB_SAFE_NO_ARGS))) + return KDB_NOPERM; + if (tp->cmd_flags & KDB_SAFE_NO_ARGS && argc > 1) + return KDB_NOPERM; + } + KDB_STATE_SET(CMD); result = (*tp->cmd_func)(argc-1, (const char **)argv); if (result && ignore_errors && result > KDB_CMD_GO) @@ -1009,7 +1026,7 @@ int kdb_parse(const char *cmdstr) * obtaining the address of a variable, or the nearest symbol * to an address contained in a register. */ - { + if (!kdb_kiosk) { unsigned long value; char *name = NULL; long offset; @@ -1025,6 +1042,7 @@ int kdb_parse(const char *cmdstr) kdb_printf("\n"); return 0; } + return KDB_NOPERM; }
On 10/15/2012 06:17 PM, Anton Vorontsov wrote:
Hello Jason,
Just as promised, I'm resending the series after the merge window.
This patchset implements "kiosk" mode for KDB debugger. The mode reduces kdb features, so that it is no longer possible to leak sensitive data via the debugger, and not possible to change program flow in a predefined manner by an ordinary user. Root can control the capability.
There are a few patches, some are just cleanups, some are churn-ish cleanups, but inevitable. And the rest implements the mode -- after all the preparations, everything is pretty straightforward.
Ping? Did this patchset fall through the cracks? Any issues left to address?
thanks -john
linaro-kernel@lists.linaro.org
-
Anton Vorontsov -
John Stultz