Lee, Chun-Yi 251570
From: Daniel Thompson <daniel.thompson@linaro.org>
Lee, Chun-Yi 251570
Date: Mon, 23 May 2022 19:11:02 +0100
Lee, Chun-Yi 251570
Subject: lockdown: also lock down previous kgdb use
Lee, Chun-Yi 251570
Patch-mainline: Queued in subsystem maintainer repository
Lee, Chun-Yi 251570
Git-repo: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
Lee, Chun-Yi 251570
Git-commit: eadb2f47a3ced5c64b23b90fd2a3463f63726066
Lee, Chun-Yi 251570
References: bsc#1199426 CVE-2022-21499
Lee, Chun-Yi 251570
Lee, Chun-Yi 251570
KGDB and KDB allow read and write access to kernel memory, and thus
Lee, Chun-Yi 251570
should be restricted during lockdown.  An attacker with access to a
Lee, Chun-Yi 251570
serial port (for example, via a hypervisor console, which some cloud
Lee, Chun-Yi 251570
vendors provide over the network) could trigger the debugger so it is
Lee, Chun-Yi 251570
important that the debugger respect the lockdown mode when/if it is
Lee, Chun-Yi 251570
triggered.
Lee, Chun-Yi 251570
Lee, Chun-Yi 251570
Fix this by integrating lockdown into kdb's existing permissions
Lee, Chun-Yi 251570
mechanism.  Unfortunately kgdb does not have any permissions mechanism
Lee, Chun-Yi 251570
(although it certainly could be added later) so, for now, kgdb is simply
Lee, Chun-Yi 251570
and brutally disabled by immediately exiting the gdb stub without taking
Lee, Chun-Yi 251570
any action.
Lee, Chun-Yi 251570
Lee, Chun-Yi 251570
For lockdowns established early in the boot (e.g. the normal case) then
Lee, Chun-Yi 251570
this should be fine but on systems where kgdb has set breakpoints before
Lee, Chun-Yi 251570
the lockdown is enacted than "bad things" will happen.
Lee, Chun-Yi 251570
Lee, Chun-Yi 251570
CVE: CVE-2022-21499
Lee, Chun-Yi 251570
Co-developed-by: Stephen Brennan <stephen.s.brennan@oracle.com>
Lee, Chun-Yi 251570
Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
Lee, Chun-Yi 251570
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Lee, Chun-Yi 251570
Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Lee, Chun-Yi 251570
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Lee, Chun-Yi 251570
Acked-by: Lee, Chun-Yi <jlee@suse.com>
Lee, Chun-Yi 251570
---
Lee, Chun-Yi 251570
 include/linux/security.h    |    2 +
Lee, Chun-Yi 251570
 kernel/debug/debug_core.c   |   24 +++++++++++++++++
Lee, Chun-Yi 251570
 kernel/debug/kdb/kdb_main.c |   62 +++++++++++++++++++++++++++++++++++++++++---
Lee, Chun-Yi 251570
 security/security.c         |    2 +
Lee, Chun-Yi 251570
 4 files changed, 87 insertions(+), 3 deletions(-)
Lee, Chun-Yi 251570
Lee, Chun-Yi 251570
--- a/include/linux/security.h
Lee, Chun-Yi 251570
+++ b/include/linux/security.h
Lee, Chun-Yi 251570
@@ -121,10 +121,12 @@ enum lockdown_reason {
Lee, Chun-Yi 251570
 	LOCKDOWN_DEBUGFS,
Lee, Chun-Yi 251570
 	LOCKDOWN_XMON_WR,
Lee, Chun-Yi 251570
 	LOCKDOWN_BPF_WRITE_USER,
Lee, Chun-Yi 251570
+	LOCKDOWN_DBG_WRITE_KERNEL,
Lee, Chun-Yi 251570
 	LOCKDOWN_INTEGRITY_MAX,
Lee, Chun-Yi 251570
 	LOCKDOWN_KCORE,
Lee, Chun-Yi 251570
 	LOCKDOWN_KPROBES,
Lee, Chun-Yi 251570
 	LOCKDOWN_BPF_READ_KERNEL,
Lee, Chun-Yi 251570
+	LOCKDOWN_DBG_READ_KERNEL,
Lee, Chun-Yi 251570
 	LOCKDOWN_PERF,
Lee, Chun-Yi 251570
 	LOCKDOWN_TRACEFS,
Lee, Chun-Yi 251570
 	LOCKDOWN_XMON_RW,
Lee, Chun-Yi 251570
--- a/kernel/debug/debug_core.c
Lee, Chun-Yi 251570
+++ b/kernel/debug/debug_core.c
Lee, Chun-Yi 251570
@@ -56,6 +56,7 @@
Lee, Chun-Yi 251570
 #include <linux/vmacache.h>
Lee, Chun-Yi 251570
 #include <linux/rcupdate.h>
Lee, Chun-Yi 251570
 #include <linux/irq.h>
Lee, Chun-Yi 251570
+#include <linux/security.h>
Lee, Chun-Yi 251570
 
Lee, Chun-Yi 251570
 #include <asm/cacheflush.h>
Lee, Chun-Yi 251570
 #include <asm/byteorder.h>
Lee, Chun-Yi 251570
@@ -755,6 +756,29 @@ cpu_master_loop:
Lee, Chun-Yi 251570
 				continue;
Lee, Chun-Yi 251570
 			kgdb_connected = 0;
Lee, Chun-Yi 251570
 		} else {
Lee, Chun-Yi 251570
+			/*
Lee, Chun-Yi 251570
+			 * This is a brutal way to interfere with the debugger
Lee, Chun-Yi 251570
+			 * and prevent gdb being used to poke at kernel memory.
Lee, Chun-Yi 251570
+			 * This could cause trouble if lockdown is applied when
Lee, Chun-Yi 251570
+			 * there is already an active gdb session. For now the
Lee, Chun-Yi 251570
+			 * answer is simply "don't do that". Typically lockdown
Lee, Chun-Yi 251570
+			 * *will* be applied before the debug core gets started
Lee, Chun-Yi 251570
+			 * so only developers using kgdb for fairly advanced
Lee, Chun-Yi 251570
+			 * early kernel debug can be biten by this. Hopefully
Lee, Chun-Yi 251570
+			 * they are sophisticated enough to take care of
Lee, Chun-Yi 251570
+			 * themselves, especially with help from the lockdown
Lee, Chun-Yi 251570
+			 * message printed on the console!
Lee, Chun-Yi 251570
+			 */
Lee, Chun-Yi 251570
+			if (security_locked_down(LOCKDOWN_DBG_WRITE_KERNEL)) {
Lee, Chun-Yi 251570
+				if (IS_ENABLED(CONFIG_KGDB_KDB)) {
Lee, Chun-Yi 251570
+					/* Switch back to kdb if possible... */
Lee, Chun-Yi 251570
+					dbg_kdb_mode = 1;
Lee, Chun-Yi 251570
+					continue;
Lee, Chun-Yi 251570
+				} else {
Lee, Chun-Yi 251570
+					/* ... otherwise just bail */
Lee, Chun-Yi 251570
+					break;
Lee, Chun-Yi 251570
+				}
Lee, Chun-Yi 251570
+			}
Lee, Chun-Yi 251570
 			error = gdb_serial_stub(ks);
Lee, Chun-Yi 251570
 		}
Lee, Chun-Yi 251570
 
Lee, Chun-Yi 251570
--- a/kernel/debug/kdb/kdb_main.c
Lee, Chun-Yi 251570
+++ b/kernel/debug/kdb/kdb_main.c
Lee, Chun-Yi 251570
@@ -46,6 +46,7 @@
Lee, Chun-Yi 251570
 #include <linux/proc_fs.h>
Lee, Chun-Yi 251570
 #include <linux/uaccess.h>
Lee, Chun-Yi 251570
 #include <linux/slab.h>
Lee, Chun-Yi 251570
+#include <linux/security.h>
Lee, Chun-Yi 251570
 #include "kdb_private.h"
Lee, Chun-Yi 251570
 
Lee, Chun-Yi 251570
 #undef	MODULE_PARAM_PREFIX
Lee, Chun-Yi 251570
@@ -167,10 +168,62 @@ struct task_struct *kdb_curr_task(int cp
Lee, Chun-Yi 251570
 }
Lee, Chun-Yi 251570
 
Lee, Chun-Yi 251570
 /*
Lee, Chun-Yi 251570
- * Check whether the flags of the current command and the permissions
Lee, Chun-Yi 251570
- * of the kdb console has allow a command to be run.
Lee, Chun-Yi 251570
+ * Update the permissions flags (kdb_cmd_enabled) to match the
Lee, Chun-Yi 251570
+ * current lockdown state.
Lee, Chun-Yi 251570
+ *
Lee, Chun-Yi 251570
+ * Within this function the calls to security_locked_down() are "lazy". We
Lee, Chun-Yi 251570
+ * avoid calling them if the current value of kdb_cmd_enabled already excludes
Lee, Chun-Yi 251570
+ * flags that might be subject to lockdown. Additionally we deliberately check
Lee, Chun-Yi 251570
+ * the lockdown flags independently (even though read lockdown implies write
Lee, Chun-Yi 251570
+ * lockdown) since that results in both simpler code and clearer messages to
Lee, Chun-Yi 251570
+ * the user on first-time debugger entry.
Lee, Chun-Yi 251570
+ *
Lee, Chun-Yi 251570
+ * The permission masks during a read+write lockdown permits the following
Lee, Chun-Yi 251570
+ * flags: INSPECT, SIGNAL, REBOOT (and ALWAYS_SAFE).
Lee, Chun-Yi 251570
+ *
Lee, Chun-Yi 251570
+ * The INSPECT commands are not blocked during lockdown because they are
Lee, Chun-Yi 251570
+ * not arbitrary memory reads. INSPECT covers the backtrace family (sometimes
Lee, Chun-Yi 251570
+ * forcing them to have no arguments) and lsmod. These commands do expose
Lee, Chun-Yi 251570
+ * some kernel state but do not allow the developer seated at the console to
Lee, Chun-Yi 251570
+ * choose what state is reported. SIGNAL and REBOOT should not be controversial,
Lee, Chun-Yi 251570
+ * given these are allowed for root during lockdown already.
Lee, Chun-Yi 251570
+ */
Lee, Chun-Yi 251570
+static void kdb_check_for_lockdown(void)
Lee, Chun-Yi 251570
+{
Lee, Chun-Yi 251570
+	const int write_flags = KDB_ENABLE_MEM_WRITE |
Lee, Chun-Yi 251570
+				KDB_ENABLE_REG_WRITE |
Lee, Chun-Yi 251570
+				KDB_ENABLE_FLOW_CTRL;
Lee, Chun-Yi 251570
+	const int read_flags = KDB_ENABLE_MEM_READ |
Lee, Chun-Yi 251570
+			       KDB_ENABLE_REG_READ;
Lee, Chun-Yi 251570
+
Lee, Chun-Yi 251570
+	bool need_to_lockdown_write = false;
Lee, Chun-Yi 251570
+	bool need_to_lockdown_read = false;
Lee, Chun-Yi 251570
+
Lee, Chun-Yi 251570
+	if (kdb_cmd_enabled & (KDB_ENABLE_ALL | write_flags))
Lee, Chun-Yi 251570
+		need_to_lockdown_write =
Lee, Chun-Yi 251570
+			security_locked_down(LOCKDOWN_DBG_WRITE_KERNEL);
Lee, Chun-Yi 251570
+
Lee, Chun-Yi 251570
+	if (kdb_cmd_enabled & (KDB_ENABLE_ALL | read_flags))
Lee, Chun-Yi 251570
+		need_to_lockdown_read =
Lee, Chun-Yi 251570
+			security_locked_down(LOCKDOWN_DBG_READ_KERNEL);
Lee, Chun-Yi 251570
+
Lee, Chun-Yi 251570
+	/* De-compose KDB_ENABLE_ALL if required */
Lee, Chun-Yi 251570
+	if (need_to_lockdown_write || need_to_lockdown_read)
Lee, Chun-Yi 251570
+		if (kdb_cmd_enabled & KDB_ENABLE_ALL)
Lee, Chun-Yi 251570
+			kdb_cmd_enabled = KDB_ENABLE_MASK & ~KDB_ENABLE_ALL;
Lee, Chun-Yi 251570
+
Lee, Chun-Yi 251570
+	if (need_to_lockdown_write)
Lee, Chun-Yi 251570
+		kdb_cmd_enabled &= ~write_flags;
Lee, Chun-Yi 251570
+
Lee, Chun-Yi 251570
+	if (need_to_lockdown_read)
Lee, Chun-Yi 251570
+		kdb_cmd_enabled &= ~read_flags;
Lee, Chun-Yi 251570
+}
Lee, Chun-Yi 251570
+
Lee, Chun-Yi 251570
+/*
Lee, Chun-Yi 251570
+ * Check whether the flags of the current command, the permissions of the kdb
Lee, Chun-Yi 251570
+ * console and the lockdown state allow a command to be run.
Lee, Chun-Yi 251570
  */
Lee, Chun-Yi 251570
-static inline bool kdb_check_flags(kdb_cmdflags_t flags, int permissions,
Lee, Chun-Yi 251570
+static bool kdb_check_flags(kdb_cmdflags_t flags, int permissions,
Lee, Chun-Yi 251570
 				   bool no_args)
Lee, Chun-Yi 251570
 {
Lee, Chun-Yi 251570
 	/* permissions comes from userspace so needs massaging slightly */
Lee, Chun-Yi 251570
@@ -1175,6 +1228,9 @@ static int kdb_local(kdb_reason_t reason
Lee, Chun-Yi 251570
 		kdb_curr_task(raw_smp_processor_id());
Lee, Chun-Yi 251570
 
Lee, Chun-Yi 251570
 	KDB_DEBUG_STATE("kdb_local 1", reason);
Lee, Chun-Yi 251570
+
Lee, Chun-Yi 251570
+	kdb_check_for_lockdown();
Lee, Chun-Yi 251570
+
Lee, Chun-Yi 251570
 	kdb_go_count = 0;
Lee, Chun-Yi 251570
 	if (reason == KDB_REASON_DEBUG) {
Lee, Chun-Yi 251570
 		/* special case below */
Lee, Chun-Yi 251570
--- a/security/security.c
Lee, Chun-Yi 251570
+++ b/security/security.c
Lee, Chun-Yi 251570
@@ -59,10 +59,12 @@ const char *const lockdown_reasons[LOCKD
Lee, Chun-Yi 251570
 	[LOCKDOWN_DEBUGFS] = "debugfs access",
Lee, Chun-Yi 251570
 	[LOCKDOWN_XMON_WR] = "xmon write access",
Lee, Chun-Yi 251570
 	[LOCKDOWN_BPF_WRITE_USER] = "use of bpf to write user RAM",
Lee, Chun-Yi 251570
+	[LOCKDOWN_DBG_WRITE_KERNEL] = "use of kgdb/kdb to write kernel RAM",
Lee, Chun-Yi 251570
 	[LOCKDOWN_INTEGRITY_MAX] = "integrity",
Lee, Chun-Yi 251570
 	[LOCKDOWN_KCORE] = "/proc/kcore access",
Lee, Chun-Yi 251570
 	[LOCKDOWN_KPROBES] = "use of kprobes",
Lee, Chun-Yi 251570
 	[LOCKDOWN_BPF_READ_KERNEL] = "use of bpf to read kernel RAM",
Lee, Chun-Yi 251570
+	[LOCKDOWN_DBG_READ_KERNEL] = "use of kgdb/kdb to read kernel RAM",
Lee, Chun-Yi 251570
 	[LOCKDOWN_PERF] = "unsafe use of perf",
Lee, Chun-Yi 251570
 	[LOCKDOWN_TRACEFS] = "use of tracefs",
Lee, Chun-Yi 251570
 	[LOCKDOWN_XMON_RW] = "xmon read and write access",