linux-kernel - [PATCH] kgdb: Resolve races during kgdb_io_register/unregister

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20200629171529.558003-1-daniel.thompson@linaro.org>
Date:   Mon, 29 Jun 2020 18:15:29 +0100
From:   Daniel Thompson <daniel.thompson@...aro.org>
To:     Jason Wessel <jason.wessel@...driver.com>,
        Douglas Anderson <dianders@...omium.org>
Cc:     Daniel Thompson <daniel.thompson@...aro.org>, pmladek@...e.com,
        kgdb-bugreport@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
        patches@...aro.org
Subject: [PATCH] kgdb: Resolve races during kgdb_io_register/unregister_module

Currently kgdb_register_callbacks() and kgdb_unregister_callbacks()
are called outside the scope of the kgdb_registration_lock. This
allows them to race with each other. This could do all sorts of crazy
things up to and including dbg_io_ops becoming NULL partway through the
execution of the kgdb trap handler (which isn't allowed and would be
fatal).

Fix this by bringing the trap handler setup and teardown into the scope
of the registration lock.

Signed-off-by: Daniel Thompson <daniel.thompson@...aro.org>
---
 kernel/debug/debug_core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 9e5934780f41..9799f2c6dc94 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -1117,9 +1117,8 @@ int kgdb_register_io_module(struct kgdb_io *new_dbg_io_ops)

 	dbg_io_ops = new_dbg_io_ops;

-	spin_unlock(&kgdb_registration_lock);
-
 	if (old_dbg_io_ops) {
+		spin_unlock(&kgdb_registration_lock);
 		old_dbg_io_ops->deinit();
 		return 0;
 	}
@@ -1129,6 +1128,8 @@ int kgdb_register_io_module(struct kgdb_io *new_dbg_io_ops)
 	/* Arm KGDB now. */
 	kgdb_register_callbacks();

+	spin_unlock(&kgdb_registration_lock);
+
 	if (kgdb_break_asap &&
 	    (!dbg_is_early || IS_ENABLED(CONFIG_ARCH_HAS_EARLY_DEBUG)))
 		kgdb_initial_breakpoint();
@@ -1147,13 +1148,14 @@ void kgdb_unregister_io_module(struct kgdb_io *old_dbg_io_ops)
 {
 	BUG_ON(kgdb_connected);

+	spin_lock(&kgdb_registration_lock);
+
 	/*
 	 * KGDB is no longer able to communicate out, so
 	 * unregister our callbacks and reset state.
 	 */
 	kgdb_unregister_callbacks();

-	spin_lock(&kgdb_registration_lock);

 	WARN_ON_ONCE(dbg_io_ops != old_dbg_io_ops);
 	dbg_io_ops = NULL;

base-commit: 9ebcfadb0610322ac537dd7aa5d9cbc2b2894c68
--
2.25.4