* [PATCH 1/7] kdb: Remove currently unused kdbtab_t->cmd_flags
2012-07-26 14:25 [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode Anton Vorontsov
@ 2012-07-26 14:26 ` Anton Vorontsov
2012-07-26 14:26 ` [PATCH 2/7] kdb: Rename kdb_repeat_t to kdb_cmdflags_t, cmd_repeat to cmd_flags Anton Vorontsov
` (6 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Anton Vorontsov @ 2012-07-26 14:26 UTC (permalink / raw)
To: Jason Wessel
Cc: Andrew Morton, Steven Rostedt, John Stultz, arve, linux-kernel,
linaro-kernel, patches, kernel-team, kgdb-bugreport
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 67b847d..e2b14ea 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2757,7 +2757,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 47c4e56..b7869c7 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? */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 2/7] kdb: Rename kdb_repeat_t to kdb_cmdflags_t, cmd_repeat to cmd_flags
2012-07-26 14:25 [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode Anton Vorontsov
2012-07-26 14:26 ` [PATCH 1/7] kdb: Remove currently unused kdbtab_t->cmd_flags Anton Vorontsov
@ 2012-07-26 14:26 ` Anton Vorontsov
2012-07-26 14:26 ` [PATCH 3/7] kdb: Rename kdb_register_repeat() to kdb_register_flags() Anton Vorontsov
` (5 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Anton Vorontsov @ 2012-07-26 14:26 UTC (permalink / raw)
To: Jason Wessel
Cc: Andrew Morton, Steven Rostedt, John Stultz, arve, linux-kernel,
linaro-kernel, patches, kernel-team, kgdb-bugreport
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 0647258..029219c 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -36,7 +36,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 **);
@@ -148,7 +148,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 */
#define kdb_printf(...)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index e2b14ea..c8e1c7b 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -991,7 +991,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])
@@ -2709,7 +2709,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;
@@ -2758,7 +2758,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 b7869c7..9665af6 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 */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 3/7] kdb: Rename kdb_register_repeat() to kdb_register_flags()
2012-07-26 14:25 [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode Anton Vorontsov
2012-07-26 14:26 ` [PATCH 1/7] kdb: Remove currently unused kdbtab_t->cmd_flags Anton Vorontsov
2012-07-26 14:26 ` [PATCH 2/7] kdb: Rename kdb_repeat_t to kdb_cmdflags_t, cmd_repeat to cmd_flags Anton Vorontsov
@ 2012-07-26 14:26 ` Anton Vorontsov
2012-07-26 14:26 ` [PATCH 4/7] kdb: Use KDB_REPEAT_* values as flags Anton Vorontsov
` (4 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Anton Vorontsov @ 2012-07-26 14:26 UTC (permalink / raw)
To: Jason Wessel
Cc: Andrew Morton, Steven Rostedt, John Stultz, arve, linux-kernel,
linaro-kernel, patches, kernel-team, kgdb-bugreport
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 | 6 +--
kernel/debug/kdb/kdb_bp.c | 16 ++++----
kernel/debug/kdb/kdb_main.c | 86 +++++++++++++++++++++----------------------
kernel/trace/trace_kdb.c | 2 +-
4 files changed, 55 insertions(+), 55 deletions(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 029219c..4ab0936 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -147,14 +147,14 @@ 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 */
#define kdb_printf(...)
#define kdb_init(x)
#define kdb_register(...)
-#define kdb_register_repeat(...)
+#define kdb_register_flags(...)
#define kdb_uregister(x)
#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 c8e1c7b..e9e33c1 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2692,7 +2692,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
@@ -2704,12 +2704,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;
@@ -2762,13 +2762,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
@@ -2783,8 +2783,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);
@@ -2826,77 +2826,77 @@ 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
- 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;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 4/7] kdb: Use KDB_REPEAT_* values as flags
2012-07-26 14:25 [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode Anton Vorontsov
` (2 preceding siblings ...)
2012-07-26 14:26 ` [PATCH 3/7] kdb: Rename kdb_register_repeat() to kdb_register_flags() Anton Vorontsov
@ 2012-07-26 14:26 ` Anton Vorontsov
2012-07-26 14:26 ` [PATCH 5/7] kdb: Remove KDB_REPEAT_NONE flag Anton Vorontsov
` (3 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Anton Vorontsov @ 2012-07-26 14:26 UTC (permalink / raw)
To: Jason Wessel
Cc: Andrew Morton, Steven Rostedt, John Stultz, arve, linux-kernel,
linaro-kernel, patches, kernel-team, kgdb-bugreport
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 4ab0936..0a047f9 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -34,8 +34,8 @@ extern atomic_t kdb_event;
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 e9e33c1..c7d023a 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -991,20 +991,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;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 5/7] kdb: Remove KDB_REPEAT_NONE flag
2012-07-26 14:25 [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode Anton Vorontsov
` (3 preceding siblings ...)
2012-07-26 14:26 ` [PATCH 4/7] kdb: Use KDB_REPEAT_* values as flags Anton Vorontsov
@ 2012-07-26 14:26 ` Anton Vorontsov
2012-07-26 14:26 ` [PATCH 6/7] kdb: Mark safe commands as KDB_SAFE and KDB_SAFE_NO_ARGS Anton Vorontsov
` (2 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Anton Vorontsov @ 2012-07-26 14:26 UTC (permalink / raw)
To: Jason Wessel
Cc: Andrew Morton, Steven Rostedt, John Stultz, arve, linux-kernel,
linaro-kernel, patches, kernel-team, kgdb-bugreport
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 | 59 +++++++++++++++++++++----------------------
kernel/trace/trace_kdb.c | 2 +-
4 files changed, 33 insertions(+), 35 deletions(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 0a047f9..d39d41d 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -33,7 +33,6 @@ extern atomic_t kdb_event;
#define KDB_MAXARGS 16 /* Maximum number of arguments to a function */
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 c7d023a..21e58fb 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2761,7 +2761,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
@@ -2776,8 +2776,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);
@@ -2831,66 +2830,66 @@ 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
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;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 6/7] kdb: Mark safe commands as KDB_SAFE and KDB_SAFE_NO_ARGS
2012-07-26 14:25 [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode Anton Vorontsov
` (4 preceding siblings ...)
2012-07-26 14:26 ` [PATCH 5/7] kdb: Remove KDB_REPEAT_NONE flag Anton Vorontsov
@ 2012-07-26 14:26 ` Anton Vorontsov
2012-07-26 17:07 ` Alan Cox
2012-07-26 14:26 ` [PATCH 7/7] kdb: Add kiosk mode Anton Vorontsov
2012-07-27 19:30 ` [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode Colin Cross
7 siblings, 1 reply; 17+ messages in thread
From: Anton Vorontsov @ 2012-07-26 14:26 UTC (permalink / raw)
To: Jason Wessel
Cc: Andrew Morton, Steven Rostedt, John Stultz, arve, linux-kernel,
linaro-kernel, patches, kernel-team, kgdb-bugreport
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":
Clear Breakpoint
Enable Breakpoint
Disable Breakpoint
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
Send a signal to a process
Summarize the system
The following commands were marked as safe when issued with no arguments:
Continue Execution
And the following commands are unsafe:
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
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_bp.c | 17 +++++++++--------
kernel/debug/kdb/kdb_main.c | 44 +++++++++++++++++++++----------------------
kernel/trace/trace_kdb.c | 2 +-
4 files changed, 34 insertions(+), 31 deletions(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index d39d41d..36f6d09 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -35,6 +35,8 @@ extern atomic_t kdb_event;
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_bp.c b/kernel/debug/kdb/kdb_bp.c
index 928e9e9..b95ddf7 100644
--- a/kernel/debug/kdb/kdb_bp.c
+++ b/kernel/debug/kdb/kdb_bp.c
@@ -546,23 +546,24 @@ void __init kdb_initbptab(void)
bp->bp_free = 1;
kdb_register_flags("bp", kdb_bp, "[<vaddr>]",
- "Set/Display breakpoints", 0, KDB_REPEAT_NO_ARGS);
+ "Set/Display breakpoints", 0, KDB_REPEAT_NO_ARGS | KDB_SAFE);
kdb_register_flags("bl", kdb_bp, "[<vaddr>]",
- "Display breakpoints", 0, KDB_REPEAT_NO_ARGS);
+ "Display breakpoints", 0, KDB_REPEAT_NO_ARGS | KDB_SAFE);
if (arch_kgdb_ops.flags & KGDB_HW_BREAKPOINT)
kdb_register_flags("bph", kdb_bp, "[<vaddr>]",
- "[datar [length]|dataw [length]] Set hw brk", 0, KDB_REPEAT_NO_ARGS);
+ "[datar [length]|dataw [length]] Set hw brk", 0,
+ KDB_REPEAT_NO_ARGS | KDB_SAFE);
kdb_register_flags("bc", kdb_bc, "<bpnum>",
- "Clear Breakpoint", 0, 0);
+ "Clear Breakpoint", 0, KDB_SAFE);
kdb_register_flags("be", kdb_bc, "<bpnum>",
- "Enable Breakpoint", 0, 0);
+ "Enable Breakpoint", 0, KDB_SAFE);
kdb_register_flags("bd", kdb_bc, "<bpnum>",
- "Disable Breakpoint", 0, 0);
+ "Disable Breakpoint", 0, KDB_SAFE);
kdb_register_flags("ss", kdb_ss, "",
- "Single Step", 1, KDB_REPEAT_NO_ARGS);
+ "Single Step", 1, KDB_REPEAT_NO_ARGS | KDB_SAFE);
kdb_register_flags("ssb", kdb_ss, "",
- "Single step to branch/call", 0, KDB_REPEAT_NO_ARGS);
+ "Single step to branch/call", 0, KDB_REPEAT_NO_ARGS | KDB_SAFE);
/*
* Architecture dependent initialization.
*/
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 21e58fb..1bb18e6 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2830,66 +2830,66 @@ 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
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);
+ "Send a signal to a process", 0, KDB_SAFE);
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;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 6/7] kdb: Mark safe commands as KDB_SAFE and KDB_SAFE_NO_ARGS
2012-07-26 14:26 ` [PATCH 6/7] kdb: Mark safe commands as KDB_SAFE and KDB_SAFE_NO_ARGS Anton Vorontsov
@ 2012-07-26 17:07 ` Alan Cox
2012-07-26 17:39 ` Anton Vorontsov
0 siblings, 1 reply; 17+ messages in thread
From: Alan Cox @ 2012-07-26 17:07 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Jason Wessel, Andrew Morton, Steven Rostedt, John Stultz, arve,
linux-kernel, linaro-kernel, patches, kernel-team, kgdb-bugreport
> The following commands were marked as "safe":
>
> Clear Breakpoint
> Enable Breakpoint
> Disable Breakpoint
> Display exception frame
> Stack traceback
This is sufficient to steal cryptographic keys in many environments. In
fact you merely need two or three breakpoints and to log the order they
are hit through the crypto computation.
> Display stack for process
Exposes all sorts of user data unless you mean just the call trace, in
which case it's still quite useful.
> Display stack all processes
Ditto
> Send a signal to a process
Like say sending SIGSTOP to security monitoring threads or the battery
manager on locked devices that rely on software battery management ?
It's an interesting idea but you need almost nothing to extract keys from
a system or to subvert it.
Alan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 6/7] kdb: Mark safe commands as KDB_SAFE and KDB_SAFE_NO_ARGS
2012-07-26 17:07 ` Alan Cox
@ 2012-07-26 17:39 ` Anton Vorontsov
2012-07-30 12:04 ` [PATCH v2 " Anton Vorontsov
0 siblings, 1 reply; 17+ messages in thread
From: Anton Vorontsov @ 2012-07-26 17:39 UTC (permalink / raw)
To: Alan Cox
Cc: Jason Wessel, Andrew Morton, Steven Rostedt, John Stultz, arve,
linux-kernel, linaro-kernel, patches, kernel-team, kgdb-bugreport
On Thu, Jul 26, 2012 at 06:07:09PM +0100, Alan Cox wrote:
> > The following commands were marked as "safe":
> >
> > Clear Breakpoint
> > Enable Breakpoint
> > Disable Breakpoint
> > Display exception frame
> > Stack traceback
>
> This is sufficient to steal cryptographic keys in many environments. In
> fact you merely need two or three breakpoints and to log the order they
> are hit through the crypto computation.
Neat. :-)
Breakpoints are no good then.
> > Display stack for process
>
> Exposes all sorts of user data unless you mean just the call trace, in
> which case it's still quite useful.
>
> > Display stack all processes
>
> Ditto
What I think is, should we just mark single stepping (as well as
breakpoints) as unsafe, then it's hard to impossible to use the call
trace as something meaningful?
> > Send a signal to a process
>
> Like say sending SIGSTOP to security monitoring threads or the battery
> manager on locked devices that rely on software battery management ?
Yeah, will need to zap it too.
> It's an interesting idea but you need almost nothing to extract keys from
> a system or to subvert it.
Apart from the above issues?
(Now it might seem that we cut almost everything from the KDB, but KDB is
not just about ordinary debugging facilities, like breakpoints or
variables watch. KDB is a shell that also implements commands to query
kernel about its state: e.g. in Android case there is "irqs" commands that
just shows interrupts counters, that is a nice feature if used w/ KDB
NMI/FIQ debugger[1], so you can see which interrupt is misbehaving.
There is also a 'dmesg' command, and 'summary' and maybe others.)
Thanks!
[1] http://lwn.net/Articles/506673/
--
Anton Vorontsov
Email: cbouatmailru@gmail.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 6/7] kdb: Mark safe commands as KDB_SAFE and KDB_SAFE_NO_ARGS
2012-07-26 17:39 ` Anton Vorontsov
@ 2012-07-30 12:04 ` Anton Vorontsov
0 siblings, 0 replies; 17+ messages in thread
From: Anton Vorontsov @ 2012-07-30 12:04 UTC (permalink / raw)
To: Alan Cox
Cc: Jason Wessel, Andrew Morton, Steven Rostedt, John Stultz, arve,
linux-kernel, linaro-kernel, patches, kernel-team, kgdb-bugreport
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
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
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
On Thu, Jul 26, 2012 at 10:39:02AM -0700, Anton Vorontsov wrote:
> > > Clear Breakpoint
> > > Enable Breakpoint
> > > Disable Breakpoint
> > > Display exception frame
> > > Stack traceback
> >
> > This is sufficient to steal cryptographic keys in many environments. In
> > fact you merely need two or three breakpoints and to log the order they
> > are hit through the crypto computation.
>
> Neat. :-)
>
> Breakpoints are no good then.
>
> > > Display stack for process
> >
> > Exposes all sorts of user data unless you mean just the call trace, in
> > which case it's still quite useful.
> >
> > > Display stack all processes
> >
> > Ditto
>
> What I think is, should we just mark single stepping (as well as
> breakpoints) as unsafe, then it's hard to impossible to use the call
> trace as something meaningful?
>
> > > Send a signal to a process
> >
> > Like say sending SIGSTOP to security monitoring threads or the battery
> > manager on locked devices that rely on software battery management ?
>
> Yeah, will need to zap it too.
So, here comes v2 of this patch. It removes the mentioned commands
from the safe list: now we don't allow any code flow changes apart
from 'continue'.
include/linux/kdb.h | 2 ++
kernel/debug/kdb/kdb_main.c | 42 +++++++++++++++++++++---------------------
kernel/trace/trace_kdb.c | 2 +-
3 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index d39d41d..36f6d09 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -35,6 +35,8 @@ extern atomic_t kdb_event;
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 8c870ea..4695606 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2805,66 +2805,66 @@ 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
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;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 7/7] kdb: Add kiosk mode
2012-07-26 14:25 [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode Anton Vorontsov
` (5 preceding siblings ...)
2012-07-26 14:26 ` [PATCH 6/7] kdb: Mark safe commands as KDB_SAFE and KDB_SAFE_NO_ARGS Anton Vorontsov
@ 2012-07-26 14:26 ` Anton Vorontsov
2012-07-27 19:30 ` [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode Colin Cross
7 siblings, 0 replies; 17+ messages in thread
From: Anton Vorontsov @ 2012-07-26 14:26 UTC (permalink / raw)
To: Jason Wessel
Cc: Andrew Morton, Steven Rostedt, John Stultz, arve, linux-kernel,
linaro-kernel, patches, kernel-team, kgdb-bugreport
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 36f6d09..8dad355 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -63,6 +63,7 @@ typedef int (*kdb_func_t)(int, const char **);
#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 1bb18e6..82973b6 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/reboot.h>
@@ -21,6 +22,7 @@
#include <linux/utsname.h>
#include <linux/vmalloc.h>
#include <linux/module.h>
+#include <linux/moduleparam.h>
#include <linux/mm.h>
#include <linux/init.h>
#include <linux/kallsyms.h>
@@ -40,6 +42,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;
@@ -119,6 +127,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
@@ -986,6 +995,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)
@@ -1008,7 +1025,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;
@@ -1024,6 +1041,7 @@ int kdb_parse(const char *cmdstr)
kdb_printf("\n");
return 0;
}
+ return KDB_NOPERM;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode
2012-07-26 14:25 [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode Anton Vorontsov
` (6 preceding siblings ...)
2012-07-26 14:26 ` [PATCH 7/7] kdb: Add kiosk mode Anton Vorontsov
@ 2012-07-27 19:30 ` Colin Cross
2012-07-28 1:26 ` Anton Vorontsov
7 siblings, 1 reply; 17+ messages in thread
From: Colin Cross @ 2012-07-27 19:30 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Jason Wessel, Andrew Morton, Steven Rostedt, John Stultz, arve,
linux-kernel, linaro-kernel, patches, kernel-team, kgdb-bugreport
On Thu, Jul 26, 2012 at 7:25 AM, Anton Vorontsov
<anton.vorontsov@linaro.org> wrote:
> Hi all,
>
> Here is a patchset that implements "kiosk" mode for KDB debugger. The
> mode provides reduced set of 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.
>
> The are two use-cases for the mode, one is evil, but another is quite
> legitimate.
>
> The evil use case is used by some (ahem) phone manufaturers that want
> to have a debuging facilities on a production device, but still don't
> want you to use the debugger to gain root access. I don't like locked
> phones, and I would not touch this/get my hands dirty by implementing
> the feature just for this evil (IMHO) use case.
The point of the reduced feature set in FIQ debugger is not to prevent
you from accessing your own phone, it designed to prevent others from
trivially rooting your phone and reading your data. Both locked and
unlocked phones run FIQ debugger. Would you carry a phone with
personal data on it and KGDB enabled on the serial console?
An alternate option would be to allow userspace to write a password
hash to a sysfs file, and require the password to be entered over the
serial console to unlock KGDB or enable unsafe KGDB commands.
> But there is another non-evil use case: limitting access to public
> devices, i.e. "kiosks", ATMs (is that too much?) or just public
> computers w/ guest access. I can imagine that an administrator would
> want to setup a kernel so that upon an oops (or a sysrq event) the
> kernel would enter KDB, but at the same time, he would not want to
> leak sensitive data from the PC by means of the debugger.
>
> There are seven patches, the first five of them are just cleanups and
> preparations. I believe these five patches are good even if not
> considering the kiosk mode. And the rest of patches actually implement
> the mode -- it is pretty straightforward.
>
> Note that we might impelement the same mode for KGDB stub, but so far
> we don't bother.
>
> Thanks!
>
> --
> include/linux/kdb.h | 16 ++--
> kernel/debug/kdb/kdb_bp.c | 35 ++++----
> kernel/debug/kdb/kdb_main.c | 183 +++++++++++++++++++++-------------------
> kernel/debug/kdb/kdb_private.h | 3 +-
> kernel/trace/trace_kdb.c | 4 +-
> 5 files changed, 126 insertions(+), 115 deletions(-)
>
> --
> Anton Vorontsov
> Email: cbouatmailru@gmail.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode
2012-07-27 19:30 ` [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode Colin Cross
@ 2012-07-28 1:26 ` Anton Vorontsov
2012-07-28 1:49 ` John Stultz
2012-07-28 1:53 ` Colin Cross
0 siblings, 2 replies; 17+ messages in thread
From: Anton Vorontsov @ 2012-07-28 1:26 UTC (permalink / raw)
To: Colin Cross
Cc: Jason Wessel, Andrew Morton, Steven Rostedt, John Stultz, arve,
linux-kernel, linaro-kernel, patches, kernel-team, kgdb-bugreport,
Alan Cox
On Fri, Jul 27, 2012 at 12:30:49PM -0700, Colin Cross wrote:
> > The are two use-cases for the mode, one is evil, but another is quite
> > legitimate.
> >
> > The evil use case is used by some (ahem) phone manufaturers that want
> > to have a debuging facilities on a production device, but still don't
> > want you to use the debugger to gain root access. I don't like locked
> > phones, and I would not touch this/get my hands dirty by implementing
> > the feature just for this evil (IMHO) use case.
>
> The point of the reduced feature set in FIQ debugger is not to prevent
> you from accessing your own phone, it designed to prevent others from
> trivially rooting your phone and reading your data. Both locked and
> unlocked phones run FIQ debugger. Would you carry a phone with
> personal data on it and KGDB enabled on the serial console?
Short answer: yes, I would carry such a phone. :-)
Long answer:
If someone was so interested in cracking the phone/data and so
ended up with attaching serial console and attempted to use debugger
techniques to gain access to my data, then thief's next step would be
soldering a few wires to JTAG spots, and it will be all done in
minutes. Knowledge-wise, using JTAG is even more trivial than using the
debugger techniques to get to my data, you just need some HW skills.
Of course, this is unless you also have the JTAG somehow vendor-locked,
but then, personally, I consider it as an utter evil. For a person who
really interested in my data, attaching to a flash directly is also not
a problem (the imaginary thief already soldered JTAG and if it didn't
work, soldering a few more wires is not a big deal).
Maybe I'm paranoid here, but for me it is hard to believe that the
reduced set was not considered as a "feature" to make life more
difficult for normal users wanting their device rooted. According to
copyright dates, the FIQ debugger started very early, in 2008, when
most Android phone vendors were very unfriendly to hackers.
Btw, why do the lovely vendors allow me to use an external SD card on
the phone? My data is not protected, but the vendors suddenly no longer
care. So what changed between my data on the external SD card and in the
phone itself? Nothing. Vendors care about the root access itself, not my
data.
Really, I tend to care more about my naked pictures^W^W^W NDA documents
I should not have taken out of the office^W^W^W^W^W loads of private data
on the SD card, and less about my email password stored in phone's
memory. That's because if SD card is stolen/lost, everyone can read it,
any time. And if password is stolen, I have some time to change it.
All I see is a very artful way to sell shackles to naive people, and
this is exactly what phone vendors do by locking their devices. If I
want my data protected, I use encryption with *my* keys, I don't want
to be "protected" by the ways described above.
And the KDB lock doesn't help in a way that thieves no longer need to
disassemble the phone to erase all the data and resell the phone (if
serial port is available outside). A guy who bought the stolen phone
on eBay will never know that the phone was disassembled: only a
professional can tell whether warranty seals are original. The thief
would probably not even bother with restoring the seals, he can always
say that the phone needed a screen replacement. (Now someone might be
wondering why do I know so much about phones' black market... :-)
Still, I'm not saying that the feature is not useful at all, it is
useful. But to me, it is much more useful for public PCs/ATMs, when
a few keystrokes on a panicked ATM can open ATM's money depository.
Or just administrator don't like when wise kids get root (yup) on
classroom's PCs.
But if you say that it wasn't the case, and no one thought about the
reducing the debugger in the "evil" way, so be it, I trust you. But I
still don't trust the phone vendors. They showed their bad attitude
in many ways towards hackers, so I think we both have quite legitimate
reasons to be a little bit paranoid. :-)
> An alternate option would be to allow userspace to write a password
> hash to a sysfs file, and require the password to be entered over the
> serial console to unlock KGDB or enable unsafe KGDB commands.
Yup, that's a very nice idea. This can be implemented by introducing
"unlock" KDB command. Although, that also requires tight integration
w/ user-space, i.e. on boot userland would need to supply hashing
method, salt and root's password hash. The same has to be done on every
password change. It is surely doable, but not sure if it is worth the
efforts. Maybe, some day.
Thanks,
--
Anton Vorontsov
Email: cbouatmailru@gmail.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode
2012-07-28 1:26 ` Anton Vorontsov
@ 2012-07-28 1:49 ` John Stultz
2012-07-28 1:53 ` Colin Cross
1 sibling, 0 replies; 17+ messages in thread
From: John Stultz @ 2012-07-28 1:49 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Colin Cross, Jason Wessel, Andrew Morton, Steven Rostedt, arve,
linux-kernel, linaro-kernel, patches, kernel-team, kgdb-bugreport,
Alan Cox
On 07/27/2012 06:26 PM, Anton Vorontsov wrote:
> On Fri, Jul 27, 2012 at 12:30:49PM -0700, Colin Cross wrote:
>>> The are two use-cases for the mode, one is evil, but another is quite
>>> legitimate.
>>>
>>> The evil use case is used by some (ahem) phone manufaturers that want
>>> to have a debuging facilities on a production device, but still don't
>>> want you to use the debugger to gain root access. I don't like locked
>>> phones, and I would not touch this/get my hands dirty by implementing
>>> the feature just for this evil (IMHO) use case.
>> The point of the reduced feature set in FIQ debugger is not to prevent
>> you from accessing your own phone, it designed to prevent others from
>> trivially rooting your phone and reading your data. Both locked and
>> unlocked phones run FIQ debugger. Would you carry a phone with
>> personal data on it and KGDB enabled on the serial console?
> Short answer: yes, I would carry such a phone. :-)
>
> Long answer:
>
> If someone was so interested in cracking the phone/data and so
> ended up with attaching serial console and attempted to use debugger
> techniques to gain access to my data, then thief's next step would be
> soldering a few wires to JTAG spots, and it will be all done in
> minutes. Knowledge-wise, using JTAG is even more trivial than using the
> debugger techniques to get to my data, you just need some HW skills.
The serial console on some of these phones are accessed via the
headphone jack.
Imagine an airline provides free noise cancelling headphones for
flights. Those headphones are of course "smart" and covertly try to
quickly capture data off of the phone's debugger interface, storing on
some headphone internal flash, all without the user noticing.
So I think Colin's concerns (regardless of any paranoia about phone
OEM's intentions) is reasonable.
thanks
-john
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/7] KDB: Kiosk (reduced capabilities) mode
2012-07-28 1:26 ` Anton Vorontsov
2012-07-28 1:49 ` John Stultz
@ 2012-07-28 1:53 ` Colin Cross
1 sibling, 0 replies; 17+ messages in thread
From: Colin Cross @ 2012-07-28 1:53 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Jason Wessel, Andrew Morton, Steven Rostedt, John Stultz, arve,
linux-kernel, linaro-kernel, patches, kernel-team, kgdb-bugreport,
Alan Cox
On Fri, Jul 27, 2012 at 6:26 PM, Anton Vorontsov <cbouatmailru@gmail.com> wrote:
<snip long rant>
> But if you say that it wasn't the case, and no one thought about the
> reducing the debugger in the "evil" way, so be it, I trust you. But I
> still don't trust the phone vendors. They showed their bad attitude
> in many ways towards hackers, so I think we both have quite legitimate
> reasons to be a little bit paranoid. :-)
I've never seen a non-Nexus phone that used the FIQ debugger, and I
believe every Nexus device has supported rooting. We leave the FIQ
debugger enabled on the devices we personally carry because it allows
easy debugging without compromising our data, and we leave it enabled
on production devices (and leave the serial console muxed out the
headphone jack) because it's more useful to end users than a blank
serial console.
>> An alternate option would be to allow userspace to write a password
>> hash to a sysfs file, and require the password to be entered over the
>> serial console to unlock KGDB or enable unsafe KGDB commands.
>
> Yup, that's a very nice idea. This can be implemented by introducing
> "unlock" KDB command. Although, that also requires tight integration
> w/ user-space, i.e. on boot userland would need to supply hashing
> method, salt and root's password hash. The same has to be done on every
> password change. It is surely doable, but not sure if it is worth the
> efforts. Maybe, some day.
>
> Thanks,
>
> --
> Anton Vorontsov
> Email: cbouatmailru@gmail.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/7] kdb: Mark safe commands as KDB_SAFE and KDB_SAFE_NO_ARGS
2012-10-16 1:17 Anton Vorontsov
@ 2012-10-16 1:18 ` Anton Vorontsov
0 siblings, 0 replies; 17+ messages in thread
From: Anton Vorontsov @ 2012-10-16 1:18 UTC (permalink / raw)
To: Jason Wessel
Cc: Colin Cross, Alan Cox, John Stultz, linux-kernel, linaro-kernel,
patches, kernel-team, kgdb-bugreport
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;
}
--
1.7.12.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/7] kdb: Mark safe commands as KDB_SAFE and KDB_SAFE_NO_ARGS
2012-11-23 0:53 [PATCH resend 0/7] KDB: Kiosk (reduced capabilities) mode Anton Vorontsov
@ 2012-11-23 0:53 ` Anton Vorontsov
0 siblings, 0 replies; 17+ messages in thread
From: Anton Vorontsov @ 2012-11-23 0:53 UTC (permalink / raw)
To: Andrew Morton
Cc: Jason Wessel, John Stultz, linux-kernel, linaro-kernel, patches,
kernel-team, kgdb-bugreport
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;
}
--
1.8.0
^ permalink raw reply related [flat|nested] 17+ messages in thread