This patchset implements "kiosk" mode for KDB debugger and is a
continuation of previous work by Anton Vorontsov (dating back to late
2012).
When kiosk mode is engaged several kdb commands become disabled leaving
only status reporting functions working normally. In particular arbitrary
memory read/write is prevented and it is no longer possible to alter
program flow.
Note that the commands that remain enabled are sufficient to run the
post-mortem macro commands, dumpcommon, dumpall and dumpcpu. One of the
motivating use-cases for this work is to realize post-mortem on embedded
devices (such as phones) without allowing the debug facility to be easily
exploited to compromise user privacy. In principle this means the feature
can be enabled on production devices.
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. The first patch
is actually a pure bug fix (arguably unrelated to kiosk mode) but
collides with the kiosk code to honour the sysrq mask so I have included
it here.
Changes since v1 (circa 2012):
* ef (Display exception frame) is essentially an overly complex peek
and has therefore been marked unsafe
* bt (Stack traceback) has been marked safe only with no arguments
* sr (Magic SysRq key) honours the sysrq mask when called in kiosk
mode
* Fixed over-zealous blocking of macro commands
* Symbol lookup is forbidden by kdbgetaddrarg (more robust, better
error reporting to user)
* Fix deadlock in sr (Magic SysRq key)
* Better help text in kiosk mode
* Default (kiosk on/off) can be changed From the config file.
Anton Vorontsov (7):
kdb: Remove currently unused kdbtab_t->cmd_flags
kdb: Rename kdb_repeat_t to kdb_cmdflags_t, cmd_repeat to cmd_flags
kdb: Rename kdb_register_repeat() to kdb_register_flags()
kdb: Use KDB_REPEAT_* values as flags
kdb: Remove KDB_REPEAT_NONE flag
kdb: Mark safe commands as KDB_SAFE and KDB_SAFE_NO_ARGS
kdb: Add kiosk mode
Daniel Thompson (3):
sysrq: Implement __handle_sysrq_nolock to avoid recursive locking in
kdb
kdb: Improve usability of help text when running in kiosk mode
kdb: Allow access to sensitive commands to be restricted by default
drivers/tty/sysrq.c | 11 ++-
include/linux/kdb.h | 20 ++--
include/linux/sysrq.h | 1 +
kernel/debug/kdb/kdb_bp.c | 22 ++---
kernel/debug/kdb/kdb_main.c | 207 +++++++++++++++++++++++------------------
kernel/debug/kdb/kdb_private.h | 3 +-
kernel/trace/trace_kdb.c | 4 +-
lib/Kconfig.kgdb | 21 +++++
8 files changed, 172 insertions(+), 117 deletions(-)
--
1.9.0
If kdb is triggered using SysRq-g then any use of the sr command results
in the SysRq key table lock being recursively acquired, killing the debug
session. That patch resolves the problem by introducing a _nolock
alternative for __handle_sysrq.
Strictly speaking this approach risks racing on the key table when kdb is
triggered by something other than SysRq-g however in that case any other
CPU involved should release the spin lock before kgdb parks the slave
CPUs.
Signed-off-by: Daniel Thompson <[email protected]>
---
drivers/tty/sysrq.c | 11 ++++++++---
include/linux/sysrq.h | 1 +
kernel/debug/kdb/kdb_main.c | 2 +-
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index ce396ec..7b47b2d 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -505,14 +505,12 @@ static void __sysrq_put_key_op(int key, struct sysrq_key_op *op_p)
sysrq_key_table[i] = op_p;
}
-void __handle_sysrq(int key, bool check_mask)
+void __handle_sysrq_nolock(int key, bool check_mask)
{
struct sysrq_key_op *op_p;
int orig_log_level;
int i;
- unsigned long flags;
- spin_lock_irqsave(&sysrq_key_table_lock, flags);
/*
* Raise the apparent loglevel to maximum so that the sysrq header
* is shown to provide the user with positive feedback. We do not
@@ -554,6 +552,13 @@ void __handle_sysrq(int key, bool check_mask)
printk("\n");
console_loglevel = orig_log_level;
}
+}
+
+void __handle_sysrq(int key, bool check_mask)
+{
+ unsigned long flags;
+ spin_lock_irqsave(&sysrq_key_table_lock, flags);
+ __handle_sysrq_nolock(key, check_mask);
spin_unlock_irqrestore(&sysrq_key_table_lock, flags);
}
diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
index 387fa7d..1d51d64 100644
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -44,6 +44,7 @@ struct sysrq_key_op {
void handle_sysrq(int key);
void __handle_sysrq(int key, bool check_mask);
+void __handle_sysrq_nolock(int key, bool check_mask);
int register_sysrq_key(int key, struct sysrq_key_op *op);
int unregister_sysrq_key(int key, struct sysrq_key_op *op);
struct sysrq_key_op *__sysrq_get_key_op(int key);
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 0b097c8..f39f926 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1924,7 +1924,7 @@ static int kdb_sr(int argc, const char **argv)
if (argc != 1)
return KDB_ARGCOUNT;
kdb_trap_printk++;
- __handle_sysrq(*argv[1], false);
+ __handle_sysrq_nolock(*argv[1], false);
kdb_trap_printk--;
return 0;
--
1.9.0
From: Anton Vorontsov <[email protected]>
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 <[email protected]>
Signed-off-by: John Stultz <[email protected]>
Signed-off-by: Daniel Thompson <[email protected]>
---
include/linux/kdb.h | 10 +++---
kernel/debug/kdb/kdb_bp.c | 14 ++++----
kernel/debug/kdb/kdb_main.c | 86 ++++++++++++++++++++++-----------------------
kernel/trace/trace_kdb.c | 2 +-
4 files changed, 56 insertions(+), 56 deletions(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 0911633..02bae26 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -146,17 +146,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 70a5046..7be2c5d 100644
--- a/kernel/debug/kdb/kdb_bp.c
+++ b/kernel/debug/kdb/kdb_bp.c
@@ -531,21 +531,21 @@ 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);
/*
* Architecture dependent initialization.
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index c328cc6..17f9042 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2629,7 +2629,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
@@ -2641,12 +2641,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;
@@ -2699,13 +2699,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
@@ -2720,8 +2720,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);
@@ -2763,79 +2763,79 @@ 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, "[D|R|S|T|C|Z|E|U|I|M|A]",
+ kdb_register_flags("bta", kdb_bt, "[D|R|S|T|C|Z|E|U|I|M|A]",
"Backtrace all processes matching state flag", 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("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 bd90e1b..1e3b36c 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.9.0
Currently kiosk mode must be explicitly requested by the bootloader or
userspace. It is convenient to be able to change the default value in a
similar manner to CONFIG_MAGIC_SYSRQ_DEFAULT_MASK.
Signed-off-by: Daniel Thompson <[email protected]>
---
kernel/debug/kdb/kdb_main.c | 2 +-
lib/Kconfig.kgdb | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 77b6e61..34f0989 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -47,7 +47,7 @@
#undef MODULE_PARAM_PREFIX
#define MODULE_PARAM_PREFIX "kdb."
-static bool kdb_kiosk;
+static bool kdb_kiosk = CONFIG_KDB_KIOSK_DEFAULT_ENABLE;
module_param_named(kiosk, kdb_kiosk, bool, 0600);
#define GREP_LEN 256
diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb
index 358eb81..a284327 100644
--- a/lib/Kconfig.kgdb
+++ b/lib/Kconfig.kgdb
@@ -73,6 +73,27 @@ config KGDB_KDB
help
KDB frontend for kernel
+config KDB_KIOSK_DEFAULT_ENABLE
+ bool "KDB: enable kiosk mode at kernel boot time"
+ depends on KGDB_KDB
+ default n
+ help
+ Kiosk mode disables kdb commands that can be trivially used to
+ escalate privilege or dump sensitive data. Those commands that
+ remain are sufficient for certain types of fault diagnosis but
+ not fully fledged debugging.
+
+ Note that it is assumed that neither the process list, the
+ kernel log buffer nor the (kernel) backtrace of running
+ processes contain sensitive information.
+
+ The config option merely sets the default at boot time. Both
+ issuing 'echo X > /sys/module/kdb/parameters/kiosk' or
+ booting with kdb.kiosk=X kernel command line option will override
+ the default settings.
+
+ If unsure, say N.
+
config KDB_KEYBOARD
bool "KGDB_KDB: keyboard as input device"
depends on VT && KGDB_KDB
--
1.9.0
Currently 'help' in kiosk mode results in help text being issued for many
commands the user cannot actually run. Filter the help list when kiosk
mode is engaged to ensure help is fully relevant.
Filtering the list is also greatly simplifies scanning for commands that
may have been accidentally classified as safe.
Signed-off-by: Daniel Thompson <[email protected]>
---
kernel/debug/kdb/kdb_main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 808bf55..77b6e61 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2401,6 +2401,9 @@ static int kdb_help(int argc, const char **argv)
return 0;
if (!kt->cmd_name)
continue;
+ if (kdb_kiosk &&
+ !(kt->cmd_flags & (KDB_SAFE | KDB_SAFE_NO_ARGS)))
+ continue;
if (strlen(kt->cmd_usage) > 20)
space = "\n ";
kdb_printf("%-15.15s %-20s%s%s\n", kt->cmd_name,
--
1.9.0
From: Anton Vorontsov <[email protected]>
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 <[email protected]>
Signed-off-by: John Stultz <[email protected]>
Signed-off-by: Daniel Thompson <[email protected]>
---
include/linux/kdb.h | 1 +
kernel/debug/kdb/kdb_main.c | 31 ++++++++++++++++++++++++++++++-
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 784b22f..cc5ece9 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 0205e4a..808bf55 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
@@ -476,6 +485,14 @@ int kdbgetaddrarg(int argc, const char **argv, int *nextarg,
kdb_symtab_t symtab;
/*
+ * In kiosk mode there's no arbitrary memory access (only
+ * pre-canned status commands) thus no reasonable need for
+ * symbol lookup.
+ */
+ if (kdb_kiosk)
+ return KDB_NOPERM;
+
+ /*
* Process arguments which follow the following syntax:
*
* symbol | numeric-address [+/- numeric-offset]
@@ -1007,6 +1024,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)
@@ -1915,13 +1940,17 @@ static int kdb_rm(int argc, const char **argv)
* kdb_sr - This function implements the 'sr' (SYSRQ key) command
* which interfaces to the soi-disant MAGIC SYSRQ functionality.
* sr <magic-sysrq-code>
+ *
+ * Normally this command bypasses the SYSRQ enables tests, however
+ * when kiosk mode is engaged certain SYSRQ commands can be masked
+ * out.
*/
static int kdb_sr(int argc, const char **argv)
{
if (argc != 1)
return KDB_ARGCOUNT;
kdb_trap_printk++;
- __handle_sysrq_nolock(*argv[1], false);
+ __handle_sysrq_nolock(*argv[1], kdb_kiosk);
kdb_trap_printk--;
return 0;
--
1.9.0
From: Anton Vorontsov <[email protected]>
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 <[email protected]>
Signed-off-by: John Stultz <[email protected]>
Signed-off-by: Daniel Thompson <[email protected]>
---
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 290db12..0911633 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 **);
@@ -147,7 +147,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 938c47b..c328cc6 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1008,7 +1008,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])
@@ -2646,7 +2646,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;
@@ -2695,7 +2695,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 c4c46c7..eaacd16 100644
--- a/kernel/debug/kdb/kdb_private.h
+++ b/kernel/debug/kdb/kdb_private.h
@@ -174,7 +174,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.9.0
From: Anton Vorontsov <[email protected]>
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 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
Macro commands (subject to finer grain checking)
The following commands were marked as safe when issued with no arguments:
Stack traceback
Continue Execution
And the following commands are unsafe:
Display exception frame
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 <[email protected]>
Signed-off-by: John Stultz <[email protected]>
Signed-off-by: Daniel Thompson <[email protected]>
---
include/linux/kdb.h | 2 ++
kernel/debug/kdb/kdb_main.c | 48 ++++++++++++++++++++++++---------------------
kernel/trace/trace_kdb.c | 2 +-
3 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 4b656d6..784b22f 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 a40b05b..0205e4a 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -641,8 +641,12 @@ static int kdb_defcmd2(const char *cmdstr, const char *argv0)
if (!s->count)
s->usable = 0;
if (s->usable)
- kdb_register(s->name, kdb_exec_defcmd,
- s->usage, s->help, 0);
+ /* macros are safe because when executed each
+ * internal command re-enters kdb_parse() and is
+ * safety checked individually.
+ */
+ kdb_register_flags(s->name, kdb_exec_defcmd,
+ s->usage, s->help, 0, KDB_SAFE);
return 0;
}
if (!s->usable)
@@ -2767,7 +2771,7 @@ 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>",
@@ -2775,60 +2779,60 @@ static void __init kdb_inittab(void)
kdb_register_flags("ef", kdb_ef, "<vaddr>",
"Display exception frame", 0, 0);
kdb_register_flags("bt", kdb_bt, "[<vaddr>]",
- "Stack traceback", 1, 0);
+ "Stack traceback", 1, KDB_SAFE_NO_ARGS);
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, "[D|R|S|T|C|Z|E|U|I|M|A]",
- "Backtrace all processes matching state flag", 0, 0);
+ "Backtrace all processes matching state flag", 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("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 3da7e30..52f9ad6 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.9.0
From: Anton Vorontsov <[email protected]>
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 <[email protected]>
Signed-off-by: John Stultz <[email protected]>
Signed-off-by: Daniel Thompson <[email protected]>
---
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 02bae26..f5cc722 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 17f9042..5d67ff6 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1008,20 +1008,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.9.0
From: Anton Vorontsov <[email protected]>
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 <[email protected]>
Signed-off-by: John Stultz <[email protected]>
Signed-off-by: Daniel Thompson <[email protected]>
---
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 f5cc722..4b656d6 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 7be2c5d..87343f0 100644
--- a/kernel/debug/kdb/kdb_bp.c
+++ b/kernel/debug/kdb/kdb_bp.c
@@ -539,11 +539,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 5d67ff6..a40b05b 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2698,7 +2698,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
@@ -2713,8 +2713,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);
@@ -2768,68 +2767,68 @@ 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, "[D|R|S|T|C|Z|E|U|I|M|A]",
- "Backtrace all processes matching state flag", 0, KDB_REPEAT_NONE);
+ "Backtrace all processes matching state flag", 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("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 1e3b36c..3da7e30 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.9.0
From: Anton Vorontsov <[email protected]>
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 <[email protected]>
Signed-off-by: John Stultz <[email protected]>
Signed-off-by: Daniel Thompson <[email protected]>
---
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 f39f926..938c47b 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2694,7 +2694,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 7afd3c8..c4c46c7 100644
--- a/kernel/debug/kdb/kdb_private.h
+++ b/kernel/debug/kdb/kdb_private.h
@@ -172,7 +172,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.9.0
Currently kiosk mode must be explicitly requested by the bootloader or
userspace. It is convenient to be able to change the default value in a
similar manner to CONFIG_MAGIC_SYSRQ_DEFAULT_MASK.
Signed-off-by: Daniel Thompson <[email protected]>
---
kernel/debug/kdb/kdb_main.c | 2 +-
lib/Kconfig.kgdb | 25 +++++++++++++++++++++++++
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 4e9340c..b68f30e 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -47,7 +47,7 @@
#undef MODULE_PARAM_PREFIX
#define MODULE_PARAM_PREFIX "kdb."
-static int kdb_cmd_enabled;
+static int kdb_cmd_enabled = CONFIG_KDB_DEFAULT_ENABLE;
module_param_named(cmd_enable, kdb_cmd_enabled, int, 0600);
#define GREP_LEN 256
diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb
index 358eb81..fbbcff6 100644
--- a/lib/Kconfig.kgdb
+++ b/lib/Kconfig.kgdb
@@ -73,6 +73,31 @@ config KGDB_KDB
help
KDB frontend for kernel
+config KDB_DEFAULT_ENABLE
+ hex "KDB: Select kdb command functions to be enabled by default"
+ depends on KGDB_KDB
+ default 0x1
+ help
+ Specifiers which kdb commands are enabled by default. This may
+ be set to 1 or 0 to enable all commands or disable almost all
+ commands.
+
+ Alternatively the following bitmask applies:
+
+ 0x0002 - allow arbitrary reads from memory and symbol lookup
+ 0x0004 - allow arbitrary writes to memory
+ 0x0008 - allow current register state to be inspected
+ 0x0010 - allow current register state to be modified
+ 0x0020 - allow passive inspection (backtrace, process list, lsmod)
+ 0x0040 - allow flow control management (breakpoint, single step)
+ 0x0080 - enable signalling of processes
+ 0x0100 - allow machine to be rebooted
+
+ The config option merely sets the default at boot time. Both
+ issuing 'echo X > /sys/module/kdb/parameters/kdb.cmd_enable' or
+ setting with kdb.cmd_enable=X kernel command line option will
+ override the default settings.
+
config KDB_KEYBOARD
bool "KGDB_KDB: keyboard as input device"
depends on VT && KGDB_KDB
--
1.9.0
From: Anton Vorontsov <[email protected]>
Currently all kdb commands are enabled whenever kdb is deployed. This
makes it difficult to deploy kdb to help debug certain types of
systems.
Android phones provide one example; the FIQ debugger found on some
Android devices has a deliberately weak set of commands to allow the
debugger to enabled very late in the production cycle.
Certain kiosk environments offer another interesting case where an
engineer might wish to probe the system state using passive inspection
commands without providing sufficient power for a passer by to root it.
Without any restrictions, 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 <[email protected]>
Signed-off-by: John Stultz <[email protected]>
Signed-off-by: Daniel Thompson <[email protected]>
---
include/linux/kdb.h | 1 +
kernel/debug/kdb/kdb_main.c | 30 +++++++++++++++++++++++++++++-
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 2f65c7a..ed323c2 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -109,6 +109,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 e88fbb1..4e9340c 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 int kdb_cmd_enabled;
+module_param_named(cmd_enable, kdb_cmd_enabled, int, 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
@@ -476,6 +485,15 @@ int kdbgetaddrarg(int argc, const char **argv, int *nextarg,
kdb_symtab_t symtab;
/*
+ * If the enable flags prohibit both arbitrary memory access
+ * and flow control then there are no reasonable grounds to
+ * provide symbol lookup.
+ */
+ if (!kdb_check_flags(KDB_ENABLE_MEM_READ | KDB_ENABLE_FLOW_CTRL,
+ kdb_cmd_enabled, false))
+ return KDB_NOPERM;
+
+ /*
* Process arguments which follow the following syntax:
*
* symbol | numeric-address [+/- numeric-offset]
@@ -1008,6 +1026,10 @@ int kdb_parse(const char *cmdstr)
if (i < kdb_max_commands) {
int result;
+
+ if (!kdb_check_flags(tp->cmd_flags, kdb_cmd_enabled, 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)
@@ -1919,10 +1941,14 @@ static int kdb_rm(int argc, const char **argv)
*/
static int kdb_sr(int argc, const char **argv)
{
+ bool check_mask =
+ !kdb_check_flags(KDB_ENABLE_ALL, kdb_cmd_enabled, false);
+
if (argc != 1)
return KDB_ARGCOUNT;
+
kdb_trap_printk++;
- __handle_sysrq_nolock(*argv[1], false);
+ __handle_sysrq_nolock(*argv[1], check_mask);
kdb_trap_printk--;
return 0;
@@ -2373,6 +2399,8 @@ static int kdb_help(int argc, const char **argv)
return 0;
if (!kt->cmd_name)
continue;
+ if (!kdb_check_flags(kt->cmd_flags, kdb_cmd_enabled, true))
+ continue;
if (strlen(kt->cmd_usage) > 20)
space = "\n ";
kdb_printf("%-15.15s %-20s%s%s\n", kt->cmd_name,
--
1.9.0
This patch introduces several new flags to collect kdb commands into
groups (later allowing them to be optionally disabled).
This follows similar prior art to enable/disable magic sysrq
commands.
The commands have been categorized as follows:
Always on: go (w/o args), env, set, help, ?, cpu (w/o args), sr,
dmesg, disable_nmi, defcmd, summary, grephelp
Mem read: md, mdr, mdp, mds, ef, bt (with args), per_cpu
Mem write: mm
Reg read: rd
Reg write: go (with args), rm
Inspect: bt (w/o args), btp, bta, btc, btt, ps, pid, lsmod
Flow ctrl: bp, bl, bph, bc, be, bd, ss
Signal: kill
Reboot: reboot
All: cpu, kgdb, (and all of the above), nmi_console
Signed-off-by: Daniel Thompson <[email protected]>
---
include/linux/kdb.h | 52 ++++++++++++++++++++++-
kernel/debug/kdb/kdb_bp.c | 21 ++++++----
kernel/debug/kdb/kdb_main.c | 100 +++++++++++++++++++++++++++++---------------
kernel/trace/trace_kdb.c | 2 +-
4 files changed, 132 insertions(+), 43 deletions(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 4b656d6..2f65c7a 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -14,10 +14,58 @@
*/
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_ENABLE_ALL = 0x00000001, /* Enable everything */
+ KDB_ENABLE_MEM_READ = 0x00000002,
+ KDB_ENABLE_MEM_WRITE = 0x00000004,
+ KDB_ENABLE_REG_READ = 0x00000008,
+ KDB_ENABLE_REG_WRITE = 0x00000010,
+ KDB_ENABLE_INSPECT = 0x00000020,
+ KDB_ENABLE_FLOW_CTRL = 0x00000040,
+ KDB_ENABLE_SIGNAL = 0x00000080,
+ KDB_ENABLE_REBOOT = 0x00000100,
+ /* User exposed values stop here, all remaining flags are
+ * exclusively used to describe a commands behaviour.
+ */
+
+ KDB_ENABLE_ALWAYS_SAFE = 0x00000200,
+ KDB_ENABLE_MASK = 0x000003ff,
+
+ KDB_ENABLE_ALL_NO_ARGS = KDB_ENABLE_ALL << 16,
+ KDB_ENABLE_MEM_READ_NO_ARGS = KDB_ENABLE_MEM_READ << 16,
+ KDB_ENABLE_MEM_WRITE_NO_ARGS = KDB_ENABLE_MEM_WRITE << 16,
+ KDB_ENABLE_REG_READ_NO_ARGS = KDB_ENABLE_REG_READ << 16,
+ KDB_ENABLE_REG_WRITE_NO_ARGS = KDB_ENABLE_REG_WRITE << 16,
+ KDB_ENABLE_INSPECT_NO_ARGS = KDB_ENABLE_INSPECT << 16,
+ KDB_ENABLE_FLOW_CTRL_NO_ARGS = KDB_ENABLE_FLOW_CTRL << 16,
+ KDB_ENABLE_SIGNAL_NO_ARGS = KDB_ENABLE_SIGNAL << 16,
+ KDB_ENABLE_REBOOT_NO_ARGS = KDB_ENABLE_REBOOT << 16,
+ KDB_ENABLE_ALWAYS_SAFE_NO_ARGS = KDB_ENABLE_ALWAYS_SAFE << 16,
+ KDB_ENABLE_MASK_NO_ARGS = KDB_ENABLE_MASK << 16,
+
+ KDB_REPEAT_NO_ARGS = 0x40000000, /* Repeat the command w/o arguments */
+ KDB_REPEAT_WITH_ARGS = 0x80000000, /* Repeat the command with args */
} kdb_cmdflags_t;
+/*
+ * Check whether the flags of the current command and the permissions
+ * of the kdb console has allow a command to be run.
+ */
+static inline bool kdb_check_flags(kdb_cmdflags_t flags, int permissions,
+ bool no_args)
+{
+ /* permissions comes from userspace so needs massaging slightly */
+ permissions &= KDB_ENABLE_MASK;
+ permissions |= KDB_ENABLE_ALWAYS_SAFE;
+
+ /* some commands change group when launched with no arguments */
+ if (no_args)
+ permissions |= permissions << 16;
+
+ flags |= KDB_ENABLE_ALL;
+
+ return permissions & flags;
+}
+
typedef int (*kdb_func_t)(int, const char **);
#ifdef CONFIG_KGDB_KDB
diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c
index 87343f0..f5b3d89 100644
--- a/kernel/debug/kdb/kdb_bp.c
+++ b/kernel/debug/kdb/kdb_bp.c
@@ -532,21 +532,28 @@ 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_ENABLE_FLOW_CTRL | KDB_REPEAT_NO_ARGS);
kdb_register_flags("bl", kdb_bp, "[<vaddr>]",
- "Display breakpoints", 0, KDB_REPEAT_NO_ARGS);
+ "Display breakpoints", 0,
+ KDB_ENABLE_FLOW_CTRL | KDB_REPEAT_NO_ARGS);
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_ENABLE_FLOW_CTRL | KDB_REPEAT_NO_ARGS);
kdb_register_flags("bc", kdb_bc, "<bpnum>",
- "Clear Breakpoint", 0, 0);
+ "Clear Breakpoint", 0,
+ KDB_ENABLE_FLOW_CTRL);
kdb_register_flags("be", kdb_bc, "<bpnum>",
- "Enable Breakpoint", 0, 0);
+ "Enable Breakpoint", 0,
+ KDB_ENABLE_FLOW_CTRL);
kdb_register_flags("bd", kdb_bc, "<bpnum>",
- "Disable Breakpoint", 0, 0);
+ "Disable Breakpoint", 0,
+ KDB_ENABLE_FLOW_CTRL);
kdb_register_flags("ss", kdb_ss, "",
- "Single Step", 1, KDB_REPEAT_NO_ARGS);
+ "Single Step", 1,
+ KDB_ENABLE_FLOW_CTRL | KDB_REPEAT_NO_ARGS);
/*
* Architecture dependent initialization.
*/
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index a40b05b..e88fbb1 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -641,8 +641,13 @@ static int kdb_defcmd2(const char *cmdstr, const char *argv0)
if (!s->count)
s->usable = 0;
if (s->usable)
- kdb_register(s->name, kdb_exec_defcmd,
- s->usage, s->help, 0);
+ /* macros are always safe because when executed each
+ * internal command re-enters kdb_parse() and is
+ * safety checked individually.
+ */
+ kdb_register_flags(s->name, kdb_exec_defcmd, s->usage,
+ s->help, 0,
+ KDB_ENABLE_ALWAYS_SAFE);
return 0;
}
if (!s->usable)
@@ -2757,78 +2762,107 @@ static void __init kdb_inittab(void)
kdb_register_flags("md", kdb_md, "<vaddr>",
"Display Memory Contents, also mdWcN, e.g. md8c1", 1,
- KDB_REPEAT_NO_ARGS);
+ KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS);
kdb_register_flags("mdr", kdb_md, "<vaddr> <bytes>",
- "Display Raw Memory", 0, KDB_REPEAT_NO_ARGS);
+ "Display Raw Memory", 0,
+ KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS);
kdb_register_flags("mdp", kdb_md, "<paddr> <bytes>",
- "Display Physical Memory", 0, KDB_REPEAT_NO_ARGS);
+ "Display Physical Memory", 0,
+ KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS);
kdb_register_flags("mds", kdb_md, "<vaddr>",
- "Display Memory Symbolically", 0, KDB_REPEAT_NO_ARGS);
+ "Display Memory Symbolically", 0,
+ KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS);
kdb_register_flags("mm", kdb_mm, "<vaddr> <contents>",
- "Modify Memory Contents", 0, KDB_REPEAT_NO_ARGS);
+ "Modify Memory Contents", 0,
+ KDB_ENABLE_MEM_WRITE | KDB_REPEAT_NO_ARGS);
kdb_register_flags("go", kdb_go, "[<vaddr>]",
- "Continue Execution", 1, 0);
+ "Continue Execution", 1,
+ KDB_ENABLE_REG_WRITE | KDB_ENABLE_ALWAYS_SAFE_NO_ARGS);
kdb_register_flags("rd", kdb_rd, "",
- "Display Registers", 0, 0);
+ "Display Registers", 0,
+ KDB_ENABLE_REG_READ);
kdb_register_flags("rm", kdb_rm, "<reg> <contents>",
- "Modify Registers", 0, 0);
+ "Modify Registers", 0,
+ KDB_ENABLE_REG_WRITE);
kdb_register_flags("ef", kdb_ef, "<vaddr>",
- "Display exception frame", 0, 0);
+ "Display exception frame", 0,
+ KDB_ENABLE_MEM_READ);
kdb_register_flags("bt", kdb_bt, "[<vaddr>]",
- "Stack traceback", 1, 0);
+ "Stack traceback", 1,
+ KDB_ENABLE_MEM_READ | KDB_ENABLE_INSPECT_NO_ARGS);
kdb_register_flags("btp", kdb_bt, "<pid>",
- "Display stack for process <pid>", 0, 0);
+ "Display stack for process <pid>", 0,
+ KDB_ENABLE_INSPECT);
kdb_register_flags("bta", kdb_bt, "[D|R|S|T|C|Z|E|U|I|M|A]",
- "Backtrace all processes matching state flag", 0, 0);
+ "Backtrace all processes matching state flag", 0,
+ KDB_ENABLE_INSPECT);
kdb_register_flags("btc", kdb_bt, "",
- "Backtrace current process on each cpu", 0, 0);
+ "Backtrace current process on each cpu", 0,
+ KDB_ENABLE_INSPECT);
kdb_register_flags("btt", kdb_bt, "<vaddr>",
"Backtrace process given its struct task address", 0,
- 0);
+ KDB_ENABLE_MEM_READ | KDB_ENABLE_INSPECT_NO_ARGS);
kdb_register_flags("env", kdb_env, "",
- "Show environment variables", 0, 0);
+ "Show environment variables", 0,
+ KDB_ENABLE_ALWAYS_SAFE);
kdb_register_flags("set", kdb_set, "",
- "Set environment variables", 0, 0);
+ "Set environment variables", 0,
+ KDB_ENABLE_ALWAYS_SAFE);
kdb_register_flags("help", kdb_help, "",
- "Display Help Message", 1, 0);
+ "Display Help Message", 1,
+ KDB_ENABLE_ALWAYS_SAFE);
kdb_register_flags("?", kdb_help, "",
- "Display Help Message", 0, 0);
+ "Display Help Message", 0,
+ KDB_ENABLE_ALWAYS_SAFE);
kdb_register_flags("cpu", kdb_cpu, "<cpunum>",
- "Switch to new cpu", 0, 0);
+ "Switch to new cpu", 0,
+ KDB_ENABLE_ALWAYS_SAFE_NO_ARGS);
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_ENABLE_INSPECT);
kdb_register_flags("pid", kdb_pid, "<pidnum>",
- "Switch to another task", 0, 0);
+ "Switch to another task", 0,
+ KDB_ENABLE_INSPECT);
kdb_register_flags("reboot", kdb_reboot, "",
- "Reboot the machine immediately", 0, 0);
+ "Reboot the machine immediately", 0,
+ KDB_ENABLE_REBOOT);
#if defined(CONFIG_MODULES)
kdb_register_flags("lsmod", kdb_lsmod, "",
- "List loaded kernel modules", 0, 0);
+ "List loaded kernel modules", 0,
+ KDB_ENABLE_INSPECT);
#endif
#if defined(CONFIG_MAGIC_SYSRQ)
kdb_register_flags("sr", kdb_sr, "<key>",
- "Magic SysRq key", 0, 0);
+ "Magic SysRq key", 0,
+ KDB_ENABLE_ALWAYS_SAFE);
#endif
#if defined(CONFIG_PRINTK)
kdb_register_flags("dmesg", kdb_dmesg, "[lines]",
- "Display syslog buffer", 0, 0);
+ "Display syslog buffer", 0,
+ KDB_ENABLE_ALWAYS_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_ENABLE_ALWAYS_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_ENABLE_ALWAYS_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_ENABLE_SIGNAL);
kdb_register_flags("summary", kdb_summary, "",
- "Summarize the system", 4, 0);
+ "Summarize the system", 4,
+ KDB_ENABLE_ALWAYS_SAFE);
kdb_register_flags("per_cpu", kdb_per_cpu, "<sym> [<bytes>] [<cpu>]",
- "Display per_cpu variables", 3, 0);
+ "Display per_cpu variables", 3,
+ KDB_ENABLE_MEM_READ);
kdb_register_flags("grephelp", kdb_grep_help, "",
- "Display help on | grep", 0, 0);
+ "Display help on | grep", 0,
+ KDB_ENABLE_ALWAYS_SAFE);
}
/* Execute any commands defined in kdb_cmds. */
diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c
index 3da7e30..1058f6b 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_ENABLE_ALWAYS_SAFE);
return 0;
}
--
1.9.0
From: Anton Vorontsov <[email protected]>
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 <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
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 02bae26..f5cc722 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 17f9042..5d67ff6 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1008,20 +1008,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.9.0
From: Anton Vorontsov <[email protected]>
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 <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
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 f5cc722..4b656d6 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 7be2c5d..87343f0 100644
--- a/kernel/debug/kdb/kdb_bp.c
+++ b/kernel/debug/kdb/kdb_bp.c
@@ -539,11 +539,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 5d67ff6..a40b05b 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2698,7 +2698,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
@@ -2713,8 +2713,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);
@@ -2768,68 +2767,68 @@ 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, "[D|R|S|T|C|Z|E|U|I|M|A]",
- "Backtrace all processes matching state flag", 0, KDB_REPEAT_NONE);
+ "Backtrace all processes matching state flag", 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("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 1e3b36c..3da7e30 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.9.0
From: Anton Vorontsov <[email protected]>
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 <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
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 f39f926..938c47b 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2694,7 +2694,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 7afd3c8..c4c46c7 100644
--- a/kernel/debug/kdb/kdb_private.h
+++ b/kernel/debug/kdb/kdb_private.h
@@ -172,7 +172,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.9.0
From: Anton Vorontsov <[email protected]>
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 <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
include/linux/kdb.h | 10 +++---
kernel/debug/kdb/kdb_bp.c | 14 ++++----
kernel/debug/kdb/kdb_main.c | 86 ++++++++++++++++++++++-----------------------
kernel/trace/trace_kdb.c | 2 +-
4 files changed, 56 insertions(+), 56 deletions(-)
diff --git a/include/linux/kdb.h b/include/linux/kdb.h
index 0911633..02bae26 100644
--- a/include/linux/kdb.h
+++ b/include/linux/kdb.h
@@ -146,17 +146,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 70a5046..7be2c5d 100644
--- a/kernel/debug/kdb/kdb_bp.c
+++ b/kernel/debug/kdb/kdb_bp.c
@@ -531,21 +531,21 @@ 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);
/*
* Architecture dependent initialization.
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index c328cc6..17f9042 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2629,7 +2629,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
@@ -2641,12 +2641,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;
@@ -2699,13 +2699,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
@@ -2720,8 +2720,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);
@@ -2763,79 +2763,79 @@ 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, "[D|R|S|T|C|Z|E|U|I|M|A]",
+ kdb_register_flags("bta", kdb_bt, "[D|R|S|T|C|Z|E|U|I|M|A]",
"Backtrace all processes matching state flag", 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("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 bd90e1b..1e3b36c 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.9.0
### This RFC has significant updates since v2. Comments appreciated (I
### hope the next iteration can be PATCH rather than RFC).
This patchset implements restricted modes for the KDB debugger. It is a
continuation of previous kiosk mode work of Anton Vorontsov (dating
back to late 2012).
Modelled of the SysRq masking functionality it provides a means for the
root user to choose the set of kdb commands that are available on the
kdb console.
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. The first patch
is actually a pure bug fix (arguably unrelated to kiosk mode) but
collides with the code to honour the sysrq mask when capabilities
are stricted so I have included it here.
Changes since v2:
* Fixed stupid build error when CONFIG_KDB[_KIOSK]_DEFAULT_ENABLE was not
defined.
* Increase flexibility by allowing the userspace greater control over the
commands to be restricted.
* Removed the "kiosk" terminology. Its confusing.
Changes since v1 (circa 2012):
* ef (Display exception frame) is essentially an overly complex peek
and has therefore been marked unsafe
* bt (Stack traceback) has been marked safe only with no arguments
* sr (Magic SysRq key) honours the sysrq mask when called in kiosk
mode
* Fixed over-zealous blocking of macro commands
* Symbol lookup is forbidden by kdbgetaddrarg (more robust, better
error reporting to user)
* Fix deadlock in sr (Magic SysRq key)
* Better help text in kiosk mode
* Default (kiosk on/off) can be changed From the config file.
Anton Vorontsov (6):
kdb: Remove currently unused kdbtab_t->cmd_flags
kdb: Rename kdb_repeat_t to kdb_cmdflags_t, cmd_repeat to cmd_flags
kdb: Rename kdb_register_repeat() to kdb_register_flags()
kdb: Use KDB_REPEAT_* values as flags
kdb: Remove KDB_REPEAT_NONE flag
kdb: Add enable mask for groups of commands
Daniel Thompson (3):
sysrq: Implement __handle_sysrq_nolock to avoid recursive locking in
kdb
kdb: Categorize kdb commands (similar to SysRq categorization)
kdb: Allow access to sensitive commands to be restricted by default
drivers/tty/sysrq.c | 11 +-
include/linux/kdb.h | 66 +++++++++--
include/linux/sysrq.h | 1 +
kernel/debug/kdb/kdb_bp.c | 37 ++++---
kernel/debug/kdb/kdb_main.c | 243 +++++++++++++++++++++++++----------------
kernel/debug/kdb/kdb_private.h | 3 +-
kernel/trace/trace_kdb.c | 4 +-
lib/Kconfig.kgdb | 25 +++++
8 files changed, 264 insertions(+), 126 deletions(-)
--
1.9.0
From: Anton Vorontsov <[email protected]>
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 <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
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 290db12..0911633 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 **);
@@ -147,7 +147,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 938c47b..c328cc6 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1008,7 +1008,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])
@@ -2646,7 +2646,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;
@@ -2695,7 +2695,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 c4c46c7..eaacd16 100644
--- a/kernel/debug/kdb/kdb_private.h
+++ b/kernel/debug/kdb/kdb_private.h
@@ -174,7 +174,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.9.0
If kdb is triggered using SysRq-g then any use of the sr command results
in the SysRq key table lock being recursively acquired, killing the debug
session. That patch resolves the problem by introducing a _nolock
alternative for __handle_sysrq.
Strictly speaking this approach risks racing on the key table when kdb is
triggered by something other than SysRq-g however in that case any other
CPU involved should release the spin lock before kgdb parks the slave
CPUs.
Signed-off-by: Daniel Thompson <[email protected]>
---
drivers/tty/sysrq.c | 11 ++++++++---
include/linux/sysrq.h | 1 +
kernel/debug/kdb/kdb_main.c | 2 +-
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index ce396ec..7b47b2d 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -505,14 +505,12 @@ static void __sysrq_put_key_op(int key, struct sysrq_key_op *op_p)
sysrq_key_table[i] = op_p;
}
-void __handle_sysrq(int key, bool check_mask)
+void __handle_sysrq_nolock(int key, bool check_mask)
{
struct sysrq_key_op *op_p;
int orig_log_level;
int i;
- unsigned long flags;
- spin_lock_irqsave(&sysrq_key_table_lock, flags);
/*
* Raise the apparent loglevel to maximum so that the sysrq header
* is shown to provide the user with positive feedback. We do not
@@ -554,6 +552,13 @@ void __handle_sysrq(int key, bool check_mask)
printk("\n");
console_loglevel = orig_log_level;
}
+}
+
+void __handle_sysrq(int key, bool check_mask)
+{
+ unsigned long flags;
+ spin_lock_irqsave(&sysrq_key_table_lock, flags);
+ __handle_sysrq_nolock(key, check_mask);
spin_unlock_irqrestore(&sysrq_key_table_lock, flags);
}
diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
index 387fa7d..1d51d64 100644
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -44,6 +44,7 @@ struct sysrq_key_op {
void handle_sysrq(int key);
void __handle_sysrq(int key, bool check_mask);
+void __handle_sysrq_nolock(int key, bool check_mask);
int register_sysrq_key(int key, struct sysrq_key_op *op);
int unregister_sysrq_key(int key, struct sysrq_key_op *op);
struct sysrq_key_op *__sysrq_get_key_op(int key);
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 0b097c8..f39f926 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1924,7 +1924,7 @@ static int kdb_sr(int argc, const char **argv)
if (argc != 1)
return KDB_ARGCOUNT;
kdb_trap_printk++;
- __handle_sysrq(*argv[1], false);
+ __handle_sysrq_nolock(*argv[1], false);
kdb_trap_printk--;
return 0;
--
1.9.0
On Fri, 25 Apr 2014 17:29:22 +0100
Daniel Thompson <[email protected]> wrote:
> If kdb is triggered using SysRq-g then any use of the sr command results
> in the SysRq key table lock being recursively acquired, killing the debug
> session. That patch resolves the problem by introducing a _nolock
> alternative for __handle_sysrq.
>
> Strictly speaking this approach risks racing on the key table when kdb is
> triggered by something other than SysRq-g however in that case any other
> CPU involved should release the spin lock before kgdb parks the slave
> CPUs.
Is that case documented somewhere in the code comments?
-- Steve
On Fri, 25 Apr 2014 17:29:28 +0100
Daniel Thompson <[email protected]> wrote:
> This patch introduces several new flags to collect kdb commands into
> groups (later allowing them to be optionally disabled).
>
> This follows similar prior art to enable/disable magic sysrq
> commands.
>
> The commands have been categorized as follows:
>
> Always on: go (w/o args), env, set, help, ?, cpu (w/o args), sr,
> dmesg, disable_nmi, defcmd, summary, grephelp
> Mem read: md, mdr, mdp, mds, ef, bt (with args), per_cpu
> Mem write: mm
> Reg read: rd
> Reg write: go (with args), rm
> Inspect: bt (w/o args), btp, bta, btc, btt, ps, pid, lsmod
> Flow ctrl: bp, bl, bph, bc, be, bd, ss
> Signal: kill
> Reboot: reboot
> All: cpu, kgdb, (and all of the above), nmi_console
>
> Signed-off-by: Daniel Thompson <[email protected]>
> ---
> include/linux/kdb.h | 52 ++++++++++++++++++++++-
> kernel/debug/kdb/kdb_bp.c | 21 ++++++----
> kernel/debug/kdb/kdb_main.c | 100 +++++++++++++++++++++++++++++---------------
> kernel/trace/trace_kdb.c | 2 +-
> 4 files changed, 132 insertions(+), 43 deletions(-)
>
> diff --git a/include/linux/kdb.h b/include/linux/kdb.h
> index 4b656d6..2f65c7a 100644
> --- a/include/linux/kdb.h
> +++ b/include/linux/kdb.h
> @@ -14,10 +14,58 @@
> */
>
> 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_ENABLE_ALL = 0x00000001, /* Enable everything */
> + KDB_ENABLE_MEM_READ = 0x00000002,
> + KDB_ENABLE_MEM_WRITE = 0x00000004,
> + KDB_ENABLE_REG_READ = 0x00000008,
> + KDB_ENABLE_REG_WRITE = 0x00000010,
> + KDB_ENABLE_INSPECT = 0x00000020,
> + KDB_ENABLE_FLOW_CTRL = 0x00000040,
> + KDB_ENABLE_SIGNAL = 0x00000080,
> + KDB_ENABLE_REBOOT = 0x00000100,
> + /* User exposed values stop here, all remaining flags are
> + * exclusively used to describe a commands behaviour.
> + */
> +
> + KDB_ENABLE_ALWAYS_SAFE = 0x00000200,
> + KDB_ENABLE_MASK = 0x000003ff,
> +
> + KDB_ENABLE_ALL_NO_ARGS = KDB_ENABLE_ALL << 16,
> + KDB_ENABLE_MEM_READ_NO_ARGS = KDB_ENABLE_MEM_READ << 16,
> + KDB_ENABLE_MEM_WRITE_NO_ARGS = KDB_ENABLE_MEM_WRITE << 16,
> + KDB_ENABLE_REG_READ_NO_ARGS = KDB_ENABLE_REG_READ << 16,
> + KDB_ENABLE_REG_WRITE_NO_ARGS = KDB_ENABLE_REG_WRITE << 16,
> + KDB_ENABLE_INSPECT_NO_ARGS = KDB_ENABLE_INSPECT << 16,
> + KDB_ENABLE_FLOW_CTRL_NO_ARGS = KDB_ENABLE_FLOW_CTRL << 16,
> + KDB_ENABLE_SIGNAL_NO_ARGS = KDB_ENABLE_SIGNAL << 16,
> + KDB_ENABLE_REBOOT_NO_ARGS = KDB_ENABLE_REBOOT << 16,
> + KDB_ENABLE_ALWAYS_SAFE_NO_ARGS = KDB_ENABLE_ALWAYS_SAFE << 16,
> + KDB_ENABLE_MASK_NO_ARGS = KDB_ENABLE_MASK << 16,
I would recommend defining a KDB_NO_ARGS_SHIFT to be 16 and use that
instead of having a magic number 16 to deal with. This also makes it
easier if you need to shift a bit more in the future.
KDB_ENABLE_ALL_NO_ARGS = KDB_ENABLE_ALL << KDB_NO_ARGS_SHIFT,
Although, I'm not sure why you just didn't have a KDB_ENABLE_ARGS flag,
and then you don't need to repeat all the flags again. Seems rather
silly.
KDB_ENABLE_ALL_NO_ARGS would then be just
KDB_ENABLE_ALL|KDB_ENABLE_NO_ARGS.
Or can you have multiple settings? That is, MEM_READ and
MEM_WRITE_NO_ARGS both set such that you can't just have a simple args
or no args command for the entire flags?
-- Steve
> +
> + KDB_REPEAT_NO_ARGS = 0x40000000, /* Repeat the command w/o arguments */
> + KDB_REPEAT_WITH_ARGS = 0x80000000, /* Repeat the command with args */
> } kdb_cmdflags_t;
>
On 25/04/14 17:45, Steven Rostedt wrote:
> On Fri, 25 Apr 2014 17:29:22 +0100
> Daniel Thompson <[email protected]> wrote:
>
>> If kdb is triggered using SysRq-g then any use of the sr command results
>> in the SysRq key table lock being recursively acquired, killing the debug
>> session. That patch resolves the problem by introducing a _nolock
>> alternative for __handle_sysrq.
>>
>> Strictly speaking this approach risks racing on the key table when kdb is
>> triggered by something other than SysRq-g however in that case any other
>> CPU involved should release the spin lock before kgdb parks the slave
>> CPUs.
>
> Is that case documented somewhere in the code comments?
Perhaps not near enough to the _nolock but the primary bit of comment is
here (and in same file as kdb_sr).
--- cut here ---
* kdb_main_loop - After initial setup and assignment of the
* controlling cpu, all cpus are in this loop. One cpu is in
* control and will issue the kdb prompt, the others will spin
* until 'go' or cpu switch.
--- cut here ---
The mechanism kgdb uses to quiesce other CPUs means other CPUs cannot be
in irqsave critical sections.
On 25/04/14 17:57, Steven Rostedt wrote:
>> + KDB_ENABLE_ALL_NO_ARGS = KDB_ENABLE_ALL << 16,
>> + KDB_ENABLE_MEM_READ_NO_ARGS = KDB_ENABLE_MEM_READ << 16,
>> + KDB_ENABLE_MEM_WRITE_NO_ARGS = KDB_ENABLE_MEM_WRITE << 16,
>> + KDB_ENABLE_REG_READ_NO_ARGS = KDB_ENABLE_REG_READ << 16,
>> + KDB_ENABLE_REG_WRITE_NO_ARGS = KDB_ENABLE_REG_WRITE << 16,
>> + KDB_ENABLE_INSPECT_NO_ARGS = KDB_ENABLE_INSPECT << 16,
>> + KDB_ENABLE_FLOW_CTRL_NO_ARGS = KDB_ENABLE_FLOW_CTRL << 16,
>> + KDB_ENABLE_SIGNAL_NO_ARGS = KDB_ENABLE_SIGNAL << 16,
>> + KDB_ENABLE_REBOOT_NO_ARGS = KDB_ENABLE_REBOOT << 16,
>> + KDB_ENABLE_ALWAYS_SAFE_NO_ARGS = KDB_ENABLE_ALWAYS_SAFE << 16,
>> + KDB_ENABLE_MASK_NO_ARGS = KDB_ENABLE_MASK << 16,
>
> I would recommend defining a KDB_NO_ARGS_SHIFT to be 16 and use that
> instead of having a magic number 16 to deal with. This also makes it
> easier if you need to shift a bit more in the future.
>
> KDB_ENABLE_ALL_NO_ARGS = KDB_ENABLE_ALL << KDB_NO_ARGS_SHIFT,
Good point. I will fix this (and reduce KDB_NO_ARGS_SHIFT to the proper
value).
> Although, I'm not sure why you just didn't have a KDB_ENABLE_ARGS flag,
> and then you don't need to repeat all the flags again. Seems rather
> silly.
>
> KDB_ENABLE_ALL_NO_ARGS would then be just
> KDB_ENABLE_ALL|KDB_ENABLE_NO_ARGS.
>
> Or can you have multiple settings? That is, MEM_READ and
> MEM_WRITE_NO_ARGS both set such that you can't just have a simple args
> or no args command for the entire flags?
That's basically it. Some commands change classification if they have an
argument ('go' is the best example; it changes from always-safe to
register-write because if an argument is given it gets copied to the PC).
Actually only two of the _NO_ARGS values are currently used, however
having a shift relationship between the flags makes it much simpler to
do the permissions check.
Daniel.
On Mon, Apr 28, 2014 at 3:24 AM, Daniel Thompson
<[email protected]> wrote:
> On 25/04/14 17:45, Steven Rostedt wrote:
>> On Fri, 25 Apr 2014 17:29:22 +0100
>> Daniel Thompson <[email protected]> wrote:
>>
>>> If kdb is triggered using SysRq-g then any use of the sr command results
>>> in the SysRq key table lock being recursively acquired, killing the debug
>>> session. That patch resolves the problem by introducing a _nolock
>>> alternative for __handle_sysrq.
>>>
>>> Strictly speaking this approach risks racing on the key table when kdb is
>>> triggered by something other than SysRq-g however in that case any other
>>> CPU involved should release the spin lock before kgdb parks the slave
>>> CPUs.
>>
>> Is that case documented somewhere in the code comments?
>
> Perhaps not near enough to the _nolock but the primary bit of comment is
> here (and in same file as kdb_sr).
> --- cut here ---
> * kdb_main_loop - After initial setup and assignment of the
> * controlling cpu, all cpus are in this loop. One cpu is in
> * control and will issue the kdb prompt, the others will spin
> * until 'go' or cpu switch.
> --- cut here ---
>
> The mechanism kgdb uses to quiesce other CPUs means other CPUs cannot be
> in irqsave critical sections.
>
>
One of the advantages of FIQ debugger is that it can be triggered from
an FIQ (NMI for those in x86 land), and Jason and I have discussed
using FIQs for kgdb to allow interrupting cpus stuck in critical
sections. If that gets implemented the above assumption will no
longer be correct.
On 28/04/14 18:44, Colin Cross wrote:
>>> Is that case documented somewhere in the code comments?
>>
>> Perhaps not near enough to the _nolock but the primary bit of comment is
>> here (and in same file as kdb_sr).
>> --- cut here ---
>> * kdb_main_loop - After initial setup and assignment of the
>> * controlling cpu, all cpus are in this loop. One cpu is in
>> * control and will issue the kdb prompt, the others will spin
>> * until 'go' or cpu switch.
>> --- cut here ---
>>
>> The mechanism kgdb uses to quiesce other CPUs means other CPUs cannot be
>> in irqsave critical sections.
>>
>>
>
> One of the advantages of FIQ debugger is that it can be triggered from
> an FIQ (NMI for those in x86 land), and Jason and I have discussed
> using FIQs for kgdb to allow interrupting cpus stuck in critical
> sections. If that gets implemented the above assumption will no
> longer be correct.
Quite so (I've got Anton's old FIQ patches running on latest kernel and
am trying to port to a GICv2-without-trustzone qemu model I've written
in order to kick the idea about a bit on an ARM multi-arch kernel).
This patch has therefore pained me a little bit to not complete cover
this case in the patch. As posted I deliberately ignore the problem. In
this particular case the SysRq table is so infrequently updated the
chances of an badly timed NMI are vanishingly small and, at that point,
even if we did actually hit that tiny window its *still* better to have
the new behaviour (risk of race) than the old behaviour (guaranteed
deadlock).
I'd very much welcome other ideas (I have tried out quite a few in my
head but none solve the problem of NMI "gratuitiously" hitting critical
sections). However when NMI/FIQ finally comes along I'd be tempted to
borrow the "bounce to normal interrupt mode" idea from FIQ debugger and
ensure commands like "sr" command do not run from the NMI handler.
Daniel.
On 28/04/14 18:44, Colin Cross wrote:
>>> Is that case documented somewhere in the code comments?
>>
>> Perhaps not near enough to the _nolock but the primary bit of comment is
>> here (and in same file as kdb_sr).
>> --- cut here ---
>> * kdb_main_loop - After initial setup and assignment of the
>> * controlling cpu, all cpus are in this loop. One cpu is in
>> * control and will issue the kdb prompt, the others will spin
>> * until 'go' or cpu switch.
>> --- cut here ---
>>
>> The mechanism kgdb uses to quiesce other CPUs means other CPUs cannot be
>> in irqsave critical sections.
>>
>>
>
> One of the advantages of FIQ debugger is that it can be triggered from
> an FIQ (NMI for those in x86 land), and Jason and I have discussed
> using FIQs for kgdb to allow interrupting cpus stuck in critical
> sections. If that gets implemented the above assumption will no
> longer be correct.
Reviewing this I realized I missed one of the most critical points in
the above.
Today kdb, even if triggered by FIQ/NMI, would still be likely to wedge
waiting for the IPI interrupts to be delivered to other processors.
Did you and Jason discuss getting the active CPU to quiesce the other
processors using FIQ/NMI, or to allow the active CPU to timeout while
waiting for them the stop?
Daniel.
On Tue, Apr 29, 2014 at 1:59 AM, Daniel Thompson
<[email protected]> wrote:
> On 28/04/14 18:44, Colin Cross wrote:
>>>> Is that case documented somewhere in the code comments?
>>>
>>> Perhaps not near enough to the _nolock but the primary bit of comment is
>>> here (and in same file as kdb_sr).
>>> --- cut here ---
>>> * kdb_main_loop - After initial setup and assignment of the
>>> * controlling cpu, all cpus are in this loop. One cpu is in
>>> * control and will issue the kdb prompt, the others will spin
>>> * until 'go' or cpu switch.
>>> --- cut here ---
>>>
>>> The mechanism kgdb uses to quiesce other CPUs means other CPUs cannot be
>>> in irqsave critical sections.
>>>
>>>
>>
>> One of the advantages of FIQ debugger is that it can be triggered from
>> an FIQ (NMI for those in x86 land), and Jason and I have discussed
>> using FIQs for kgdb to allow interrupting cpus stuck in critical
>> sections. If that gets implemented the above assumption will no
>> longer be correct.
>
> Reviewing this I realized I missed one of the most critical points in
> the above.
>
> Today kdb, even if triggered by FIQ/NMI, would still be likely to wedge
> waiting for the IPI interrupts to be delivered to other processors.
>
> Did you and Jason discuss getting the active CPU to quiesce the other
> processors using FIQ/NMI, or to allow the active CPU to timeout while
> waiting for them the stop?
>
>
> Daniel.
Yes, all cpus would have to get an FIQ/NMI.