Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Sumit Garg <sumit.garg@linaro.org>
To:     kgdb-bugreport@lists.sourceforge.net
Cc:     daniel.thompson@linaro.org, jason.wessel@windriver.com,
        dianders@chromium.org, peterz@infradead.org,
        stefan.saecherl@fau.de, qy15sije@cip.cs.fau.de,
        linux-kernel@vger.kernel.org, Sumit Garg <sumit.garg@linaro.org>
Subject: [PATCH] kernel: debug: Handle breakpoints in kernel .init.text section
Date:   Fri, 19 Feb 2021 13:31:34 +0530
Message-Id: <1613721694-16418-1-git-send-email-sumit.garg@linaro.org>
Precedence: bulk

Currently breakpoints in kernel .init.text section are not handled
correctly while allowing to remove them even after corresponding pages
have been freed.

In order to keep track of .init.text section breakpoints, add another
breakpoint state as BP_ACTIVE_INIT and don't try to free these
breakpoints once the system is in running state.

To be clear there is still a very small window between call to
free_initmem() and system_state = SYSTEM_RUNNING which can lead to
removal of freed .init.text section breakpoints but I think we can live
with that.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 include/linux/kgdb.h      |  3 ++-
 kernel/debug/debug_core.c | 17 +++++++++++++----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 0d6cf64..57b8885 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -71,7 +71,8 @@ enum kgdb_bpstate {
 	BP_UNDEFINED = 0,
 	BP_REMOVED,
 	BP_SET,
-	BP_ACTIVE
+	BP_ACTIVE_INIT,
+	BP_ACTIVE,
 };
 
 struct kgdb_bkpt {
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index af6e8b4f..229dd11 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -324,7 +324,11 @@ int dbg_activate_sw_breakpoints(void)
 		}
 
 		kgdb_flush_swbreak_addr(kgdb_break[i].bpt_addr);
-		kgdb_break[i].state = BP_ACTIVE;
+		if (system_state >= SYSTEM_RUNNING ||
+		    !init_section_contains((void *)kgdb_break[i].bpt_addr, 0))
+			kgdb_break[i].state = BP_ACTIVE;
+		else
+			kgdb_break[i].state = BP_ACTIVE_INIT;
 	}
 	return ret;
 }
@@ -378,8 +382,13 @@ int dbg_deactivate_sw_breakpoints(void)
 	int i;
 
 	for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
-		if (kgdb_break[i].state != BP_ACTIVE)
+		if (kgdb_break[i].state < BP_ACTIVE_INIT)
+			continue;
+		if (system_state >= SYSTEM_RUNNING &&
+		    kgdb_break[i].state == BP_ACTIVE_INIT) {
+			kgdb_break[i].state = BP_UNDEFINED;
 			continue;
+		}
 		error = kgdb_arch_remove_breakpoint(&kgdb_break[i]);
 		if (error) {
 			pr_info("BP remove failed: %lx\n",
@@ -425,7 +434,7 @@ int kgdb_has_hit_break(unsigned long addr)
 	int i;
 
 	for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
-		if (kgdb_break[i].state == BP_ACTIVE &&
+		if (kgdb_break[i].state >= BP_ACTIVE_INIT &&
 		    kgdb_break[i].bpt_addr == addr)
 			return 1;
 	}
@@ -439,7 +448,7 @@ int dbg_remove_all_break(void)
 
 	/* Clear memory breakpoints. */
 	for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
-		if (kgdb_break[i].state != BP_ACTIVE)
+		if (kgdb_break[i].state < BP_ACTIVE_INIT)
 			goto setundefined;
 		error = kgdb_arch_remove_breakpoint(&kgdb_break[i]);
 		if (error)
-- 
2.7.4