All of lore.kernel.org
 help / color / mirror / Atom feed
From: osandov@osandov.com (Omar Sandoval)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM/ARM64: don't enter kgdb when userspace executes a kgdb break instruction.
Date: Wed, 30 Jul 2014 00:12:45 -0700	[thread overview]
Message-ID: <20140730071245.GA8954@mew.web-pass.com> (raw)

The kgdb breakpoint hooks (kgdb_brk_fn and kgdb_compiled_brk_fn) should only be
entered when a kgdb break instruction is executed from the kernel. Otherwise,
if kgdb is enabled, a userspace program can cause the kernel to drop into the
debugger by executing either KGDB_BREAKINST or KGDB_COMPILED_BREAK on ARM, or
brk #KGDB_{DYN,COMPILED}_DGB_BRK_IMM on ARM64.

Signed-off-by: Omar Sandoval <osandov@osandov.com>
---
The following program reproduces the fixed problem on ARM:
.globl _start
_start:
	udf	#65006	@ KGDB_BREAKINST

And on ARM64:
.globl _start
_start:
        brk     #0x400	@ KGDB_DYN_DGB_BRK_IMM

 arch/arm/kernel/kgdb.c                  | 4 ++++
 arch/arm64/include/asm/debug-monitors.h | 4 +++-
 arch/arm64/kernel/debug-monitors.c      | 3 ++-
 arch/arm64/kernel/kgdb.c                | 4 ++++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c
index 778c2f7..a74b53c 100644
--- a/arch/arm/kernel/kgdb.c
+++ b/arch/arm/kernel/kgdb.c
@@ -160,12 +160,16 @@ static int kgdb_compiled_brk_fn(struct pt_regs *regs, unsigned int instr)
 static struct undef_hook kgdb_brkpt_hook = {
 	.instr_mask		= 0xffffffff,
 	.instr_val		= KGDB_BREAKINST,
+	.cpsr_mask		= MODE_MASK,
+	.cpsr_val		= SVC_MODE,
 	.fn			= kgdb_brk_fn
 };
 
 static struct undef_hook kgdb_compiled_brkpt_hook = {
 	.instr_mask		= 0xffffffff,
 	.instr_val		= KGDB_COMPILED_BREAK,
+	.cpsr_mask		= MODE_MASK,
+	.cpsr_val		= SVC_MODE,
 	.fn			= kgdb_compiled_brk_fn
 };
 
diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index 6e9b5b3..e1d27ce 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -105,8 +105,10 @@ void unregister_step_hook(struct step_hook *hook);
 
 struct break_hook {
 	struct list_head node;
-	u32 esr_val;
 	u32 esr_mask;
+	u32 esr_val;
+	u64 pstate_mask;
+	u64 pstate_val;
 	int (*fn)(struct pt_regs *regs, unsigned int esr);
 };
 
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index a7fb874..594fac4 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -303,7 +303,8 @@ static int call_break_hook(struct pt_regs *regs, unsigned int esr)
 
 	read_lock(&break_hook_lock);
 	list_for_each_entry(hook, &break_hook, node)
-		if ((esr & hook->esr_mask) == hook->esr_val)
+		if ((esr & hook->esr_mask) == hook->esr_val &&
+		    (regs->pstate & hook->pstate_mask) == hook->pstate_val)
 			fn = hook->fn;
 	read_unlock(&break_hook_lock);
 
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index 75c9cf1..80eb035 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -236,12 +236,16 @@ static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned int esr)
 static struct break_hook kgdb_brkpt_hook = {
 	.esr_mask	= 0xffffffff,
 	.esr_val	= DBG_ESR_VAL_BRK(KGDB_DYN_DGB_BRK_IMM),
+	.pstate_mask	= PSR_MODE_MASK,
+	.pstate_val	= PSR_MODE_EL1h,
 	.fn		= kgdb_brk_fn
 };
 
 static struct break_hook kgdb_compiled_brkpt_hook = {
 	.esr_mask	= 0xffffffff,
 	.esr_val	= DBG_ESR_VAL_BRK(KDBG_COMPILED_DBG_BRK_IMM),
+	.pstate_mask	= PSR_MODE_MASK,
+	.pstate_val	= PSR_MODE_EL1h,
 	.fn		= kgdb_compiled_brk_fn
 };
 
-- 
2.0.3

WARNING: multiple messages have this Message-ID (diff)
From: Omar Sandoval <osandov@osandov.com>
To: linux-arm-kernel@lists.infradead.org, linux@arm.linux.org.uk,
	catalin.marinas@arm.com, will.deacon@arm.com
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH] ARM/ARM64: don't enter kgdb when userspace executes a kgdb break instruction.
Date: Wed, 30 Jul 2014 00:12:45 -0700	[thread overview]
Message-ID: <20140730071245.GA8954@mew.web-pass.com> (raw)

The kgdb breakpoint hooks (kgdb_brk_fn and kgdb_compiled_brk_fn) should only be
entered when a kgdb break instruction is executed from the kernel. Otherwise,
if kgdb is enabled, a userspace program can cause the kernel to drop into the
debugger by executing either KGDB_BREAKINST or KGDB_COMPILED_BREAK on ARM, or
brk #KGDB_{DYN,COMPILED}_DGB_BRK_IMM on ARM64.

Signed-off-by: Omar Sandoval <osandov@osandov.com>
---
The following program reproduces the fixed problem on ARM:
.globl _start
_start:
	udf	#65006	@ KGDB_BREAKINST

And on ARM64:
.globl _start
_start:
        brk     #0x400	@ KGDB_DYN_DGB_BRK_IMM

 arch/arm/kernel/kgdb.c                  | 4 ++++
 arch/arm64/include/asm/debug-monitors.h | 4 +++-
 arch/arm64/kernel/debug-monitors.c      | 3 ++-
 arch/arm64/kernel/kgdb.c                | 4 ++++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c
index 778c2f7..a74b53c 100644
--- a/arch/arm/kernel/kgdb.c
+++ b/arch/arm/kernel/kgdb.c
@@ -160,12 +160,16 @@ static int kgdb_compiled_brk_fn(struct pt_regs *regs, unsigned int instr)
 static struct undef_hook kgdb_brkpt_hook = {
 	.instr_mask		= 0xffffffff,
 	.instr_val		= KGDB_BREAKINST,
+	.cpsr_mask		= MODE_MASK,
+	.cpsr_val		= SVC_MODE,
 	.fn			= kgdb_brk_fn
 };
 
 static struct undef_hook kgdb_compiled_brkpt_hook = {
 	.instr_mask		= 0xffffffff,
 	.instr_val		= KGDB_COMPILED_BREAK,
+	.cpsr_mask		= MODE_MASK,
+	.cpsr_val		= SVC_MODE,
 	.fn			= kgdb_compiled_brk_fn
 };
 
diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index 6e9b5b3..e1d27ce 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -105,8 +105,10 @@ void unregister_step_hook(struct step_hook *hook);
 
 struct break_hook {
 	struct list_head node;
-	u32 esr_val;
 	u32 esr_mask;
+	u32 esr_val;
+	u64 pstate_mask;
+	u64 pstate_val;
 	int (*fn)(struct pt_regs *regs, unsigned int esr);
 };
 
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index a7fb874..594fac4 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -303,7 +303,8 @@ static int call_break_hook(struct pt_regs *regs, unsigned int esr)
 
 	read_lock(&break_hook_lock);
 	list_for_each_entry(hook, &break_hook, node)
-		if ((esr & hook->esr_mask) == hook->esr_val)
+		if ((esr & hook->esr_mask) == hook->esr_val &&
+		    (regs->pstate & hook->pstate_mask) == hook->pstate_val)
 			fn = hook->fn;
 	read_unlock(&break_hook_lock);
 
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index 75c9cf1..80eb035 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -236,12 +236,16 @@ static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned int esr)
 static struct break_hook kgdb_brkpt_hook = {
 	.esr_mask	= 0xffffffff,
 	.esr_val	= DBG_ESR_VAL_BRK(KGDB_DYN_DGB_BRK_IMM),
+	.pstate_mask	= PSR_MODE_MASK,
+	.pstate_val	= PSR_MODE_EL1h,
 	.fn		= kgdb_brk_fn
 };
 
 static struct break_hook kgdb_compiled_brkpt_hook = {
 	.esr_mask	= 0xffffffff,
 	.esr_val	= DBG_ESR_VAL_BRK(KDBG_COMPILED_DBG_BRK_IMM),
+	.pstate_mask	= PSR_MODE_MASK,
+	.pstate_val	= PSR_MODE_EL1h,
 	.fn		= kgdb_compiled_brk_fn
 };
 
-- 
2.0.3


             reply	other threads:[~2014-07-30  7:12 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-30  7:12 Omar Sandoval [this message]
2014-07-30  7:12 ` [PATCH] ARM/ARM64: don't enter kgdb when userspace executes a kgdb break instruction Omar Sandoval
2014-07-30  7:20 ` Omar Sandoval
2014-07-30 11:24 ` Will Deacon
2014-07-30 11:24   ` Will Deacon
2014-07-31  5:33   ` Omar Sandoval
2014-07-31  5:33     ` Omar Sandoval
2014-07-31 10:46     ` Will Deacon
2014-07-31 10:46       ` Will Deacon
2014-08-01  3:07       ` Omar Sandoval
2014-08-01  3:07         ` Omar Sandoval

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140730071245.GA8954@mew.web-pass.com \
    --to=osandov@osandov.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.