From patchwork Mon Jul 30 12:04:25 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Vorontsov X-Patchwork-Id: 10370 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 46E962402A for ; Mon, 30 Jul 2012 12:06:37 +0000 (UTC) Received: from mail-yx0-f180.google.com (mail-yx0-f180.google.com [209.85.213.180]) by fiordland.canonical.com (Postfix) with ESMTP id EB9CCA180AA for ; Mon, 30 Jul 2012 12:06:36 +0000 (UTC) Received: by yenq6 with SMTP id q6so4593015yen.11 for ; Mon, 30 Jul 2012 05:06:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf:date:from :to:cc:subject:message-id:references:mime-version:content-type :content-disposition:in-reply-to:user-agent:x-gm-message-state; bh=MIpodMLaKMVoZluZe+e3Ybhe4U2xH1c/z/XFF2IIgYA=; b=L/mI0l+I5JR/8aA3PdeRBZhvQSjR/IN+TAyIcxspGEy9CSUD0EMXcOxzsxzTqxc/04 qyCKqWswgrfYLKrnEzU35yE1PiZGvg2i8SndTKMTEtFQYNL67LL9DS23QQumqs/pmjLz 9349cT6yCY9JhbjAuYStE1BtvXkbQ+oH5LeKx/7Ti5oc0z7f6TOQ9f3Ef6vqUT7ZKoPR FQnWLdW0Xg/8k2ml4+IbkmHynTXsv6JfKxxYCXiSYpxfVmVbuYHAxGfxAtMmTRTVR1o7 Kz2dIWxbHnUA9uslKETmuqVUOWGpSTEPqLSi6uRxJAWb6/5lqiwhGdlqCRPK0EqJn+xK otIw== Received: by 10.50.149.134 with SMTP id ua6mr10646983igb.11.1343649996285; Mon, 30 Jul 2012 05:06:36 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.50.87.40 with SMTP id u8csp84840igz; Mon, 30 Jul 2012 05:06:35 -0700 (PDT) Received: by 10.68.131.10 with SMTP id oi10mr34323609pbb.122.1343649995224; Mon, 30 Jul 2012 05:06:35 -0700 (PDT) Received: from mail-pb0-f50.google.com (mail-pb0-f50.google.com [209.85.160.50]) by mx.google.com with ESMTPS id tc8si17316631pbc.294.2012.07.30.05.06.34 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 30 Jul 2012 05:06:35 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.160.50 is neither permitted nor denied by best guess record for domain of anton.vorontsov@linaro.org) client-ip=209.85.160.50; Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.160.50 is neither permitted nor denied by best guess record for domain of anton.vorontsov@linaro.org) smtp.mail=anton.vorontsov@linaro.org Received: by pbbrr4 with SMTP id rr4so11046671pbb.37 for ; Mon, 30 Jul 2012 05:06:34 -0700 (PDT) Received: by 10.68.216.130 with SMTP id oq2mr9964040pbc.121.1343649994815; Mon, 30 Jul 2012 05:06:34 -0700 (PDT) Received: from localhost (c-71-204-165-222.hsd1.ca.comcast.net. [71.204.165.222]) by mx.google.com with ESMTPS id hf4sm7829400pbc.4.2012.07.30.05.06.32 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 30 Jul 2012 05:06:33 -0700 (PDT) Date: Mon, 30 Jul 2012 05:04:25 -0700 From: Anton Vorontsov To: Alan Cox Cc: Jason Wessel , Andrew Morton , Steven Rostedt , John Stultz , arve@android.com, linux-kernel@vger.kernel.org, linaro-kernel@lists.linaro.org, patches@linaro.org, kernel-team@android.com, kgdb-bugreport@lists.sourceforge.net Subject: [PATCH v2 6/7] kdb: Mark safe commands as KDB_SAFE and KDB_SAFE_NO_ARGS Message-ID: <20120730120425.GA18189@lizard> References: <20120726142514.GA32158@lizard> <1343312791-9138-6-git-send-email-anton.vorontsov@linaro.org> <20120726180709.09777a3b@pyramind.ukuu.org.uk> <20120726173902.GA20023@lizard> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20120726173902.GA20023@lizard> User-Agent: Mutt/1.5.21 (2010-09-15) X-Gm-Message-State: ALoCoQkuKCMK6P0nt1Ds62/3M6xc38Irt8sNSJLMPJU/Buo7AZr1Xuy/gAahT9PH7xMSm4eUugLy This patch introduces two new flags: KDB_SAFE, denotes a safe command, and KDB_SAFE_NO_ARGS, denotes a safe command when used without arguments. The word "safe" here used in the sense that the commands cannot be used to leak sensitive data from the memory, and cannot be used to change program flow in a predefined manner. These flags will be used by the "kiosk" mode, i.e. when it is possible for the ordinary user to enter the KDB (or user can get the access to KDB after the crash), but we do not allow user to read dump the memory [and thus read some sensitive data]. The following commands were marked as "safe": Display exception frame Stack traceback Display stack for process Display stack all processes Backtrace current process on each cpu Execute cmd for each element in linked list Show environment variables Set environment variables Display Help Message Switch to new cpu Display active task list Switch to another task Reboot the machine immediately List loaded kernel modules Magic SysRq key Display syslog buffer Define a set of commands, down to endefcmd Summarize the system The following commands were marked as safe when issued with no arguments: Continue Execution And the following commands are unsafe: Clear Breakpoint Enable Breakpoint Disable Breakpoint Single step Single step to branch/call Continue Execution (with address argument) Display Memory Contents Display Raw Memory Display Physical Memory Display Memory Symbolically Modify Memory Contents Display Registers Modify Registers Backtrace process given its struct task address Send a signal to a process Enter kgdb mode Display per_cpu variables Signed-off-by: Anton Vorontsov --- On Thu, Jul 26, 2012 at 10:39:02AM -0700, Anton Vorontsov wrote: > > > Clear Breakpoint > > > Enable Breakpoint > > > Disable Breakpoint > > > Display exception frame > > > Stack traceback > > > > This is sufficient to steal cryptographic keys in many environments. In > > fact you merely need two or three breakpoints and to log the order they > > are hit through the crypto computation. > > Neat. :-) > > Breakpoints are no good then. > > > > Display stack for process > > > > Exposes all sorts of user data unless you mean just the call trace, in > > which case it's still quite useful. > > > > > Display stack all processes > > > > Ditto > > What I think is, should we just mark single stepping (as well as > breakpoints) as unsafe, then it's hard to impossible to use the call > trace as something meaningful? > > > > Send a signal to a process > > > > Like say sending SIGSTOP to security monitoring threads or the battery > > manager on locked devices that rely on software battery management ? > > Yeah, will need to zap it too. So, here comes v2 of this patch. It removes the mentioned commands from the safe list: now we don't allow any code flow changes apart from 'continue'. include/linux/kdb.h | 2 ++ kernel/debug/kdb/kdb_main.c | 42 +++++++++++++++++++++--------------------- kernel/trace/trace_kdb.c | 2 +- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/include/linux/kdb.h b/include/linux/kdb.h index d39d41d..36f6d09 100644 --- a/include/linux/kdb.h +++ b/include/linux/kdb.h @@ -35,6 +35,8 @@ extern atomic_t kdb_event; typedef enum { KDB_REPEAT_NO_ARGS = 0x1, /* Repeat the command w/o arguments */ KDB_REPEAT_WITH_ARGS = 0x2, /* Repeat the command w/ its arguments */ + KDB_SAFE = 0x4, /* Security-wise safe command */ + KDB_SAFE_NO_ARGS = 0x8, /* Only safe if run w/o arguments */ } kdb_cmdflags_t; typedef int (*kdb_func_t)(int, const char **); diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index 8c870ea..4695606 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -2805,66 +2805,66 @@ static void __init kdb_inittab(void) kdb_register_flags("mm", kdb_mm, " ", "Modify Memory Contents", 0, KDB_REPEAT_NO_ARGS); kdb_register_flags("go", kdb_go, "[]", - "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, " ", "Modify Registers", 0, 0); kdb_register_flags("ef", kdb_ef, "", - "Display exception frame", 0, 0); + "Display exception frame", 0, KDB_SAFE); kdb_register_flags("bt", kdb_bt, "[]", - "Stack traceback", 1, 0); + "Stack traceback", 1, KDB_SAFE); kdb_register_flags("btp", kdb_bt, "", - "Display stack for process ", 0, 0); + "Display stack for process ", 0, KDB_SAFE); kdb_register_flags("bta", kdb_bt, "[DRSTCZEUIMA]", - "Display stack all processes", 0, 0); + "Display stack all processes", 0, KDB_SAFE); kdb_register_flags("btc", kdb_bt, "", - "Backtrace current process on each cpu", 0, 0); + "Backtrace current process on each cpu", 0, KDB_SAFE); kdb_register_flags("btt", kdb_bt, "", "Backtrace process given its struct task address", 0, 0); kdb_register_flags("ll", kdb_ll, " ", - "Execute cmd for each element in linked list", 0, 0); + "Execute cmd for each element in linked list", 0, KDB_SAFE); kdb_register_flags("env", kdb_env, "", - "Show environment variables", 0, 0); + "Show environment variables", 0, KDB_SAFE); kdb_register_flags("set", kdb_set, "", - "Set environment variables", 0, 0); + "Set environment variables", 0, KDB_SAFE); kdb_register_flags("help", kdb_help, "", - "Display Help Message", 1, 0); + "Display Help Message", 1, KDB_SAFE); kdb_register_flags("?", kdb_help, "", - "Display Help Message", 0, 0); + "Display Help Message", 0, KDB_SAFE); kdb_register_flags("cpu", kdb_cpu, "", - "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, "[|A]", - "Display active task list", 0, 0); + "Display active task list", 0, KDB_SAFE); kdb_register_flags("pid", kdb_pid, "", - "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, "", - "Magic SysRq key", 0, 0); + "Magic SysRq key", 0, KDB_SAFE); #endif #if defined(CONFIG_PRINTK) kdb_register_flags("dmesg", kdb_dmesg, "[lines]", - "Display syslog buffer", 0, 0); + "Display syslog buffer", 0, KDB_SAFE); #endif kdb_register_flags("defcmd", kdb_defcmd, "name \"usage\" \"help\"", - "Define a set of commands, down to endefcmd", 0, 0); + "Define a set of commands, down to endefcmd", 0, KDB_SAFE); kdb_register_flags("kill", kdb_kill, "<-signal> ", "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, " [] []", "Display per_cpu variables", 3, 0); kdb_register_flags("grephelp", kdb_grep_help, "", - "Display help on | grep", 0, 0); + "Display help on | grep", 0, KDB_SAFE); } /* Execute any commands defined in kdb_cmds. */ diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c index 1b68177..8353852 100644 --- a/kernel/trace/trace_kdb.c +++ b/kernel/trace/trace_kdb.c @@ -128,7 +128,7 @@ static int kdb_ftdump(int argc, const char **argv) static __init int kdb_ftrace_register(void) { kdb_register_flags("ftdump", kdb_ftdump, "[skip_#lines] [cpu]", - "Dump ftrace log", 0, 0); + "Dump ftrace log", 0, KDB_SAFE); return 0; }