arm64: kgdb: handle read-only text / modules

PATCH New AKASHI Takahiro 2 participants 4 messages Sep 23, 2016 – Sep 25, 2016

Subsystems (2)

Stable branch arm64

lore

Share & tools

lore

AKASHI Takahiro

author

2016-09-23 07:35 UTC #0 lore raw

PATCH 1/1 arm64: kgdb: handle read-only text / modules

Handle read-only cases (CONFIG_DEBUG_RODATA/CONFIG_DEBUG_SET_MODULE_RONX) by using aarch64_insn_write() instead of probe_kernel_write(). See how this works:

    commit 2f896d586610 ("arm64: use fixmap for text patching")

Signed-off-by AKASHI Takahiro <[email protected]>

Reviewed-by Mark Rutland <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Jason Wessel <[email protected]>

Cc <[email protected]> # 3.18-3.19: 2f896d5: arm64: use fixmap

Cc <[email protected]> # 3.18-3.19: f6242ca: arm64: Fix text

Cc <[email protected]> # 4.0-

---
 arch/arm64/include/asm/debug-monitors.h |  2 --
 arch/arm64/kernel/kgdb.c                | 36 ++++++++++++++++++++++-----------
 2 files changed, 24 insertions(+), 14 deletions(-)

arch/arm64/include/asm/debug-monitors.h +0 -2

index 4b6b3f7..b71420a 100644
@@ -61,8 +61,6 @@
 
 #define AARCH64_BREAK_KGDB_DYN_DBG	\
 	(AARCH64_BREAK_MON | (KGDB_DYN_DBG_BRK_IMM << 5))
-#define KGDB_DYN_BRK_INS_BYTE(x)	\
-	((AARCH64_BREAK_KGDB_DYN_DBG >> (8 * (x))) & 0xff)
 
 #define CACHE_FLUSH_IS_SAFE		1

arch/arm64/kernel/kgdb.c +24 ~~-12~~

index 6732a27..b06a7a2 100644
@@ -19,6 +19,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/bug.h>
 #include <linux/cpumask.h>
 #include <linux/irq.h>
 #include <linux/irq_work.h>

@@ -26,6 +27,8 @@
 #include <linux/kgdb.h>
 #include <linux/kprobes.h>
 #include <linux/percpu.h>
+#include <asm/debug-monitors.h>
+#include <asm/insn.h>
 #include <asm/ptrace.h>
 #include <asm/traps.h>

@@ -370,15 +373,24 @@ void kgdb_arch_exit(void)
 	unregister_die_notifier(&kgdb_notifier);
 }
 
-/*
- * ARM instructions are always in LE.
- * Break instruction is encoded in LE format
- */
-struct kgdb_arch arch_kgdb_ops = {
-	.gdb_bpt_instr = {
-		KGDB_DYN_BRK_INS_BYTE(0),
-		KGDB_DYN_BRK_INS_BYTE(1),
-		KGDB_DYN_BRK_INS_BYTE(2),
-		KGDB_DYN_BRK_INS_BYTE(3),
-	}
-};
+struct kgdb_arch arch_kgdb_ops;
+
+int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
+{
+	int err;
+
+	BUILD_BUG_ON(AARCH64_INSN_SIZE != BREAK_INSTR_SIZE);
+
+	err = aarch64_insn_read((void *)bpt->bpt_addr, (u32 *)bpt->saved_instr);
+	if (err)
+		return err;
+
+	return aarch64_insn_write((void *)bpt->bpt_addr,
+			(u32)AARCH64_BREAK_KGDB_DYN_DBG);
+}
+
+int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
+{
+	return aarch64_insn_write((void *)bpt->bpt_addr,
+			*(u32 *)bpt->saved_instr);
+}

Signature · 1 line

2.10.0

Catalin Marinas

maintainer

2016-09-23 09:49 UTC #1 lore raw

PATCH 1/1 arm64: kgdb: handle read-only text / modules

Quoted 5 lines · AKASHI Takahiro

Handle read-only cases (CONFIG_DEBUG_RODATA/CONFIG_DEBUG_SET_MODULE_RONX) by using aarch64_insn_write() instead of probe_kernel_write(). See how this works:
    commit 2f896d586610 ("arm64: use fixmap for text patching")

Quoted 8 lines

Signed-off-by AKASHI Takahiro <[email protected]>
Reviewed-by Mark Rutland <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Jason Wessel <[email protected]>
Cc <[email protected]> # 3.18-3.19: 2f896d5: arm64: use fixmap
Cc <[email protected]> # 3.18-3.19: f6242ca: arm64: Fix text
Cc <[email protected]> # 4.0-

Queued for 4.8 with a slight change in the last Cc: tag above:

Cc <[email protected]> # 3.18.x-

Thanks.

Signature · 1 line

Catalin

Catalin Marinas

maintainer

2016-09-23 10:23 UTC #2 lore raw

PATCH 1/1 arm64: kgdb: handle read-only text / modules

Quoted 15 lines · Catalin Marinas

Quoted 5 lines · AKASHI Takahiro
Handle read-only cases (CONFIG_DEBUG_RODATA/CONFIG_DEBUG_SET_MODULE_RONX) by using aarch64_insn_write() instead of probe_kernel_write(). See how this works:
    commit 2f896d586610 ("arm64: use fixmap for text patching")
Quoted 8 lines
Signed-off-by AKASHI Takahiro <[email protected]>
Reviewed-by Mark Rutland <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Jason Wessel <[email protected]>
Cc <[email protected]> # 3.18-3.19: 2f896d5: arm64: use fixmap
Cc <[email protected]> # 3.18-3.19: f6242ca: arm64: Fix text
Cc <[email protected]> # 4.0-

Quoted 3 lines

Queued for 4.8 with a slight change in the last Cc: tag above:
Cc <[email protected]> # 3.18.x-

I tried to apply this patch to 3.18, 4.1, 4.4, 4.7. It fails on all of them with smaller or slightly larger conflicts.

So, I'll merge it in 4.8 without any "Cc: stable" tags, just a "Fixes:" one for the commit introducing CONFIG_DEBUG_SET_MODULE_RONX (3.18). Once the patch reaches mainline, could you please back-port and test it on the stable kernel versions and send separate patches to stable at vger.kernel.org?

Thanks.

Signature · 1 line

Catalin

AKASHI Takahiro

author

2016-09-25 00:29 UTC #3 lore raw

PATCH 1/1 arm64: kgdb: handle read-only text / modules

Hi Catalin,

Quoted 16 lines · Catalin Marinas

Quoted 15 lines · Catalin Marinas
Quoted 13 lines · AKASHI Takahiro
Handle read-only cases (CONFIG_DEBUG_RODATA/CONFIG_DEBUG_SET_MODULE_RONX) by using aarch64_insn_write() instead of probe_kernel_write(). See how this works:
    commit 2f896d586610 ("arm64: use fixmap for text patching")
Signed-off-by AKASHI Takahiro <[email protected]>
Reviewed-by Mark Rutland <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Jason Wessel <[email protected]>
Cc <[email protected]> # 3.18-3.19: 2f896d5: arm64: use fixmap
Cc <[email protected]> # 3.18-3.19: f6242ca: arm64: Fix text
Cc <[email protected]> # 4.0-

Quoted 6 lines

Quoted 3 lines
Queued for 4.8 with a slight change in the last Cc: tag above:
Cc <[email protected]> # 3.18.x-
I tried to apply this patch to 3.18, 4.1, 4.4, 4.7. It fails on all of them with smaller or slightly larger conflicts.

Oh, too bad.
I guest we'd better revert the following patches as well:
c696b93 arm64/debug: Simplify BRK insn opcode declarations c172d99 arm64/debug: More consistent naming for the BRK ESR template macro
but I will check anyway.

Quoted 5 lines

So, I'll merge it in 4.8 without any "Cc: stable" tags, just a "Fixes:" one for the commit introducing CONFIG_DEBUG_SET_MODULE_RONX (3.18). Once the patch reaches mainline, could you please back-port and test it on the stable kernel versions and send separate patches to stable at vger.kernel.org?

Thanks,
-Takahiro AKASHI

Quoted 4 lines

Thanks.
Signature · 1 line
Catalin