[PATCH v3 0/3] ARM: kprobes: introduces instruction checker.

linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v3 0/3] ARM: kprobes: introduces instruction checker.
@ 2014-11-21  6:35 Wang Nan
  2014-11-21  6:35 ` [PATCH v3 1/3] ARM: kprobes: introduces checker Wang Nan
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Wang Nan @ 2014-11-21  6:35 UTC (permalink / raw)
  To: linux-arm-kernel

This the third version (and the final version I hope) of instruction
checker patch. It is a part of version 10 of ARM OPTPROBE patch series.
Previous discussion can be found from:

https://lkml.org/lkml/2014/11/18/26
https://lkml.org/lkml/2014/10/25/48
https://lkml.org/lkml/2014/10/22/254
https://lkml.org/lkml/2014/8/27/255
https://lkml.org/lkml/2014/8/12/12
https://lkml.org/lkml/2014/8/8/992
https://lkml.org/lkml/2014/8/8/5
https://lkml.org/lkml/2014/8/5/63

Except fixing an error found by Tixy, the main changes in this series
are mainly small code cleanup and commit message cleanup.

Wang Nan (3):
  ARM: kprobes: introduces checker
  ARM: kprobes: collects stack consumption for store instructions
  ARM: kprobes: disallow probing stack consuming instructions

 arch/arm/include/asm/kprobes.h           |   1 -
 arch/arm/include/asm/probes.h            |  13 ++++
 arch/arm/kernel/Makefile                 |   6 +-
 arch/arm/kernel/entry-armv.S             |   3 +-
 arch/arm/kernel/kprobes-arm.c            |   3 +
 arch/arm/kernel/kprobes-test-arm.c       |  16 +++--
 arch/arm/kernel/kprobes-thumb.c          |   4 ++
 arch/arm/kernel/kprobes.c                |  15 ++++-
 arch/arm/kernel/kprobes.h                |   7 +-
 arch/arm/kernel/probes-arm.c             |   5 +-
 arch/arm/kernel/probes-arm.h             |   3 +-
 arch/arm/kernel/probes-checkers-arm.c    |  99 +++++++++++++++++++++++++++++
 arch/arm/kernel/probes-checkers-common.c |  87 +++++++++++++++++++++++++
 arch/arm/kernel/probes-checkers-thumb.c  | 106 +++++++++++++++++++++++++++++++
 arch/arm/kernel/probes-checkers.h        |  53 ++++++++++++++++
 arch/arm/kernel/probes-thumb.c           |  10 +--
 arch/arm/kernel/probes-thumb.h           |   6 +-
 arch/arm/kernel/probes.c                 |  70 ++++++++++++++++++--
 arch/arm/kernel/probes.h                 |  11 +++-
 arch/arm/kernel/uprobes.c                |   2 +-
 20 files changed, 489 insertions(+), 31 deletions(-)
 create mode 100644 arch/arm/kernel/probes-checkers-arm.c
 create mode 100644 arch/arm/kernel/probes-checkers-common.c
 create mode 100644 arch/arm/kernel/probes-checkers-thumb.c
 create mode 100644 arch/arm/kernel/probes-checkers.h

-- 
1.8.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/3] ARM: kprobes: introduces checker
  2014-11-21  6:35 [PATCH v3 0/3] ARM: kprobes: introduces instruction checker Wang Nan
@ 2014-11-21  6:35 ` Wang Nan
  2014-11-21  6:35 ` [PATCH v3 2/3] ARM: kprobes: collects stack consumption for store instructions Wang Nan
  2014-11-21  6:35 ` [PATCH v3 3/3] ARM: kprobes: disallow probing stack consuming instructions Wang Nan
  2 siblings, 0 replies; 7+ messages in thread
From: Wang Nan @ 2014-11-21  6:35 UTC (permalink / raw)
  To: linux-arm-kernel

This patch introdces 'checker' to decoding phase, and calls checkers
when instruction decoding. This allows further decoding for specific
instructions.  This patch introduces a stub call of checkers in kprobe
arch_prepare_kprobe() as an example and for further expansion.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Reviewed-by: Jon Medhurst <tixy@linaro.org>
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

---

v1 -> v2:
 - kprobe checker stubs are introduced in this patch.

v2 -> v3:
 - Code cleanups following Masami Hiramatsu and Tixy's advises.
 - Commit message improvements.
---
 arch/arm/kernel/kprobes-arm.c   |  2 ++
 arch/arm/kernel/kprobes-thumb.c |  3 +++
 arch/arm/kernel/kprobes.c       |  6 ++++-
 arch/arm/kernel/kprobes.h       |  7 +++--
 arch/arm/kernel/probes-arm.c    |  5 ++--
 arch/arm/kernel/probes-arm.h    |  3 ++-
 arch/arm/kernel/probes-thumb.c  | 10 ++++---
 arch/arm/kernel/probes-thumb.h  |  6 +++--
 arch/arm/kernel/probes.c        | 60 ++++++++++++++++++++++++++++++++++++-----
 arch/arm/kernel/probes.h        | 11 +++++++-
 arch/arm/kernel/uprobes.c       |  2 +-
 11 files changed, 95 insertions(+), 20 deletions(-)

diff --git a/arch/arm/kernel/kprobes-arm.c b/arch/arm/kernel/kprobes-arm.c
index ac300c6..5a81275 100644
--- a/arch/arm/kernel/kprobes-arm.c
+++ b/arch/arm/kernel/kprobes-arm.c
@@ -341,3 +341,5 @@ const union decode_action kprobes_arm_actions[NUM_PROBES_ARM_ACTIONS] = {
 	[PROBES_BRANCH] = {.handler = simulate_bbl},
 	[PROBES_LDMSTM] = {.decoder = kprobe_decode_ldmstm}
 };
+
+const struct decode_checker *kprobes_arm_checkers[] = {NULL};
diff --git a/arch/arm/kernel/kprobes-thumb.c b/arch/arm/kernel/kprobes-thumb.c
index 9495d7f..b8ba7d2 100644
--- a/arch/arm/kernel/kprobes-thumb.c
+++ b/arch/arm/kernel/kprobes-thumb.c
@@ -664,3 +664,6 @@ const union decode_action kprobes_t32_actions[NUM_PROBES_T32_ACTIONS] = {
 	[PROBES_T32_MUL_ADD_LONG] = {
 		.handler = t32_emulate_rdlo12rdhi8rn16rm0_noflags},
 };
+
+const struct decode_checker *kprobes_t32_checkers[] = {NULL};
+const struct decode_checker *kprobes_t16_checkers[] = {NULL};
diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
index 6d64420..d7bee4b 100644
--- a/arch/arm/kernel/kprobes.c
+++ b/arch/arm/kernel/kprobes.c
@@ -61,6 +61,7 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
 	kprobe_decode_insn_t *decode_insn;
 	const union decode_action *actions;
 	int is;
+	const struct decode_checker **checkers;
 
 	if (in_exception_text(addr))
 		return -EINVAL;
@@ -74,9 +75,11 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
 		insn = __opcode_thumb32_compose(insn, inst2);
 		decode_insn = thumb32_probes_decode_insn;
 		actions = kprobes_t32_actions;
+		checkers = kprobes_t32_checkers;
 	} else {
 		decode_insn = thumb16_probes_decode_insn;
 		actions = kprobes_t16_actions;
+		checkers = kprobes_t16_checkers;
 	}
 #else /* !CONFIG_THUMB2_KERNEL */
 	thumb = false;
@@ -85,12 +88,13 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
 	insn = __mem_to_opcode_arm(*p->addr);
 	decode_insn = arm_probes_decode_insn;
 	actions = kprobes_arm_actions;
+	checkers = kprobes_arm_checkers;
 #endif
 
 	p->opcode = insn;
 	p->ainsn.insn = tmp_insn;
 
-	switch ((*decode_insn)(insn, &p->ainsn, true, actions)) {
+	switch ((*decode_insn)(insn, &p->ainsn, true, actions, checkers)) {
 	case INSN_REJECTED:	/* not supported */
 		return -EINVAL;
 
diff --git a/arch/arm/kernel/kprobes.h b/arch/arm/kernel/kprobes.h
index 9a2712e..05f3b40 100644
--- a/arch/arm/kernel/kprobes.h
+++ b/arch/arm/kernel/kprobes.h
@@ -36,16 +36,19 @@ kprobe_decode_ldmstm(kprobe_opcode_t insn, struct arch_probes_insn *asi,
 typedef enum probes_insn (kprobe_decode_insn_t)(probes_opcode_t,
 						struct arch_probes_insn *,
 						bool,
-						const union decode_action *);
+						const union decode_action *,
+						const struct decode_checker *[*]);
 
 #ifdef CONFIG_THUMB2_KERNEL
 
 extern const union decode_action kprobes_t32_actions[];
 extern const union decode_action kprobes_t16_actions[];
-
+extern const struct decode_checker *kprobes_t32_checkers[];
+extern const struct decode_checker *kprobes_t16_checkers[];
 #else /* !CONFIG_THUMB2_KERNEL */
 
 extern const union decode_action kprobes_arm_actions[];
+extern const struct decode_checker *kprobes_arm_checkers[];
 
 #endif
 
diff --git a/arch/arm/kernel/probes-arm.c b/arch/arm/kernel/probes-arm.c
index 8eaef81..125feda 100644
--- a/arch/arm/kernel/probes-arm.c
+++ b/arch/arm/kernel/probes-arm.c
@@ -725,10 +725,11 @@ static void __kprobes arm_singlestep(probes_opcode_t insn,
  */
 enum probes_insn __kprobes
 arm_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
-		       bool emulate, const union decode_action *actions)
+		       bool emulate, const union decode_action *actions,
+		       const struct decode_checker *checkers[])
 {
 	asi->insn_singlestep = arm_singlestep;
 	asi->insn_check_cc = probes_condition_checks[insn>>28];
 	return probes_decode_insn(insn, asi, probes_decode_arm_table, false,
-				  emulate, actions);
+				  emulate, actions, checkers);
 }
diff --git a/arch/arm/kernel/probes-arm.h b/arch/arm/kernel/probes-arm.h
index ace6572..8b11ec4 100644
--- a/arch/arm/kernel/probes-arm.h
+++ b/arch/arm/kernel/probes-arm.h
@@ -68,6 +68,7 @@ extern const union decode_item probes_decode_arm_table[];
 
 enum probes_insn arm_probes_decode_insn(probes_opcode_t,
 		struct arch_probes_insn *, bool emulate,
-		const union decode_action *actions);
+		const union decode_action *actions,
+		const struct decode_checker *checkers[]);
 
 #endif
diff --git a/arch/arm/kernel/probes-thumb.c b/arch/arm/kernel/probes-thumb.c
index 4131351..a5022b4 100644
--- a/arch/arm/kernel/probes-thumb.c
+++ b/arch/arm/kernel/probes-thumb.c
@@ -863,20 +863,22 @@ static void __kprobes thumb32_singlestep(probes_opcode_t opcode,
 
 enum probes_insn __kprobes
 thumb16_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
-			   bool emulate, const union decode_action *actions)
+			   bool emulate, const union decode_action *actions,
+			   const struct decode_checker *checkers[])
 {
 	asi->insn_singlestep = thumb16_singlestep;
 	asi->insn_check_cc = thumb_check_cc;
 	return probes_decode_insn(insn, asi, probes_decode_thumb16_table, true,
-				  emulate, actions);
+				  emulate, actions, checkers);
 }
 
 enum probes_insn __kprobes
 thumb32_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
-			   bool emulate, const union decode_action *actions)
+			   bool emulate, const union decode_action *actions,
+			   const struct decode_checker *checkers[])
 {
 	asi->insn_singlestep = thumb32_singlestep;
 	asi->insn_check_cc = thumb_check_cc;
 	return probes_decode_insn(insn, asi, probes_decode_thumb32_table, true,
-				  emulate, actions);
+				  emulate, actions, checkers);
 }
diff --git a/arch/arm/kernel/probes-thumb.h b/arch/arm/kernel/probes-thumb.h
index 7c6f6eb..ccfe3e4 100644
--- a/arch/arm/kernel/probes-thumb.h
+++ b/arch/arm/kernel/probes-thumb.h
@@ -89,9 +89,11 @@ extern const union decode_item probes_decode_thumb16_table[];
 
 enum probes_insn __kprobes
 thumb16_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
-		bool emulate, const union decode_action *actions);
+		bool emulate, const union decode_action *actions,
+		const struct decode_checker *checkers[]);
 enum probes_insn __kprobes
 thumb32_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
-		bool emulate, const union decode_action *actions);
+		bool emulate, const union decode_action *actions,
+		const struct decode_checker *checkers[]);
 
 #endif
diff --git a/arch/arm/kernel/probes.c b/arch/arm/kernel/probes.c
index a8ab540..6a7d92e 100644
--- a/arch/arm/kernel/probes.c
+++ b/arch/arm/kernel/probes.c
@@ -342,6 +342,31 @@ static const int decode_struct_sizes[NUM_DECODE_TYPES] = {
 	[DECODE_TYPE_REJECT]	= sizeof(struct decode_reject)
 };
 
+static int run_checkers(const struct decode_checker *checkers[],
+		int action, probes_opcode_t insn,
+		struct arch_probes_insn *asi,
+		const struct decode_header *h)
+{
+	const struct decode_checker **p;
+
+	if (!checkers)
+		return INSN_GOOD;
+
+	p = checkers;
+	while (*p != NULL) {
+		int retval;
+		probes_check_t *checker_func = (*p)[action].checker;
+
+		retval = INSN_GOOD;
+		if (checker_func)
+			retval = checker_func(insn, asi, h);
+		if (retval == INSN_REJECTED)
+			return retval;
+		p++;
+	}
+	return INSN_GOOD;
+}
+
 /*
  * probes_decode_insn operates on data tables in order to decode an ARM
  * architecture instruction onto which a kprobe has been placed.
@@ -388,11 +413,17 @@ static const int decode_struct_sizes[NUM_DECODE_TYPES] = {
 int __kprobes
 probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
 		   const union decode_item *table, bool thumb,
-		   bool emulate, const union decode_action *actions)
+		   bool emulate, const union decode_action *actions,
+		   const struct decode_checker *checkers[])
 {
 	const struct decode_header *h = (struct decode_header *)table;
 	const struct decode_header *next;
 	bool matched = false;
+	/*
+	 * @insn can be modified by decode_regs. Save its original
+	 * value for checkers.
+	 */
+	probes_opcode_t origin_insn = insn;
 
 	if (emulate)
 		insn = prepare_emulated_insn(insn, asi, thumb);
@@ -422,24 +453,41 @@ probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
 		}
 
 		case DECODE_TYPE_CUSTOM: {
+			int err;
 			struct decode_custom *d = (struct decode_custom *)h;
-			return actions[d->decoder.action].decoder(insn, asi, h);
+			int action = d->decoder.action;
+
+			err = run_checkers(checkers, action, origin_insn, asi, h);
+			if (err == INSN_REJECTED)
+				return INSN_REJECTED;
+			return actions[action].decoder(insn, asi, h);
 		}
 
 		case DECODE_TYPE_SIMULATE: {
+			int err;
 			struct decode_simulate *d = (struct decode_simulate *)h;
-			asi->insn_handler = actions[d->handler.action].handler;
+			int action = d->handler.action;
+
+			err = run_checkers(checkers, action, origin_insn, asi, h);
+			if (err == INSN_REJECTED)
+				return INSN_REJECTED;
+			asi->insn_handler = actions[action].handler;
 			return INSN_GOOD_NO_SLOT;
 		}
 
 		case DECODE_TYPE_EMULATE: {
+			int err;
 			struct decode_emulate *d = (struct decode_emulate *)h;
+			int action = d->handler.action;
+
+			err = run_checkers(checkers, action, origin_insn, asi, h);
+			if (err == INSN_REJECTED)
+				return INSN_REJECTED;
 
 			if (!emulate)
-				return actions[d->handler.action].decoder(insn,
-					asi, h);
+				return actions[action].decoder(insn, asi, h);
 
-			asi->insn_handler = actions[d->handler.action].handler;
+			asi->insn_handler = actions[action].handler;
 			set_emulated_insn(insn, asi, thumb);
 			return INSN_GOOD;
 		}
diff --git a/arch/arm/kernel/probes.h b/arch/arm/kernel/probes.h
index dba9f24..b4bf1f5 100644
--- a/arch/arm/kernel/probes.h
+++ b/arch/arm/kernel/probes.h
@@ -314,6 +314,14 @@ union decode_action {
 	probes_custom_decode_t	*decoder;
 };
 
+typedef enum probes_insn (probes_check_t)(probes_opcode_t,
+					   struct arch_probes_insn *,
+					   const struct decode_header *);
+
+struct decode_checker {
+	probes_check_t	*checker;
+};
+
 #define DECODE_END			\
 	{.bits = DECODE_TYPE_END}
 
@@ -402,6 +410,7 @@ probes_insn_handler_t probes_emulate_none;
 int __kprobes
 probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
 		const union decode_item *table, bool thumb, bool emulate,
-		const union decode_action *actions);
+		const union decode_action *actions,
+		const struct decode_checker **checkers);
 
 #endif
diff --git a/arch/arm/kernel/uprobes.c b/arch/arm/kernel/uprobes.c
index 56adf9c..372585a 100644
--- a/arch/arm/kernel/uprobes.c
+++ b/arch/arm/kernel/uprobes.c
@@ -88,7 +88,7 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
 	auprobe->ixol[1] = __opcode_to_mem_arm(UPROBE_SS_ARM_INSN);
 
 	ret = arm_probes_decode_insn(insn, &auprobe->asi, false,
-				     uprobes_probes_actions);
+				     uprobes_probes_actions, NULL);
 	switch (ret) {
 	case INSN_REJECTED:
 		return -EINVAL;
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/3] ARM: kprobes: collects stack consumption for store instructions
  2014-11-21  6:35 [PATCH v3 0/3] ARM: kprobes: introduces instruction checker Wang Nan
  2014-11-21  6:35 ` [PATCH v3 1/3] ARM: kprobes: introduces checker Wang Nan
@ 2014-11-21  6:35 ` Wang Nan
  2014-11-21 17:18   ` Jon Medhurst (Tixy)
  2014-11-21  6:35 ` [PATCH v3 3/3] ARM: kprobes: disallow probing stack consuming instructions Wang Nan
  2 siblings, 1 reply; 7+ messages in thread
From: Wang Nan @ 2014-11-21  6:35 UTC (permalink / raw)
  To: linux-arm-kernel

This patch uses the previously introduced checker functionality on
store instructions to record their stack consumption information to
arch_probes_insn.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Jon Medhurst <tixy@linaro.org>
Reviewed-by: Jon Medhurst <tixy@linaro.org>

---

v1 -> v2:
 - Bugfix and code improvements following Tixy's suggestion. See:
   http://www.spinics.net/lists/arm-kernel/msg372912.html

v2 -> v3:
 - Totaly reconstructed following Tixy' instruction. See:
   https://lkml.org/lkml/2014/10/27/662 .
   Add his SOB.

v3 -> v4:
 - Commit message improvements.
 - Comments improvements and code cleanup.
 - A bug is found and fixed in decode table in arm_check_stack().
---
 arch/arm/include/asm/probes.h            |   1 +
 arch/arm/kernel/Makefile                 |   6 +-
 arch/arm/kernel/kprobes-arm.c            |   3 +-
 arch/arm/kernel/kprobes-thumb.c          |   5 +-
 arch/arm/kernel/probes-checkers-arm.c    |  99 +++++++++++++++++++++++++++++
 arch/arm/kernel/probes-checkers-common.c |  87 +++++++++++++++++++++++++
 arch/arm/kernel/probes-checkers-thumb.c  | 106 +++++++++++++++++++++++++++++++
 arch/arm/kernel/probes-checkers.h        |  53 ++++++++++++++++
 arch/arm/kernel/probes.c                 |  10 +++
 9 files changed, 364 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/kernel/probes-checkers-arm.c
 create mode 100644 arch/arm/kernel/probes-checkers-common.c
 create mode 100644 arch/arm/kernel/probes-checkers-thumb.c
 create mode 100644 arch/arm/kernel/probes-checkers.h

diff --git a/arch/arm/include/asm/probes.h b/arch/arm/include/asm/probes.h
index 806cfe6..ccf9af3 100644
--- a/arch/arm/include/asm/probes.h
+++ b/arch/arm/include/asm/probes.h
@@ -38,6 +38,7 @@ struct arch_probes_insn {
 	probes_check_cc			*insn_check_cc;
 	probes_insn_singlestep_t	*insn_singlestep;
 	probes_insn_fn_t		*insn_fn;
+	int stack_space;
 };
 
 #endif
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 38ddd9f..45aed4b 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -52,11 +52,11 @@ obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= ftrace.o insn.o
 obj-$(CONFIG_JUMP_LABEL)	+= jump_label.o insn.o patch.o
 obj-$(CONFIG_KEXEC)		+= machine_kexec.o relocate_kernel.o
 obj-$(CONFIG_UPROBES)		+= probes.o probes-arm.o uprobes.o uprobes-arm.o
-obj-$(CONFIG_KPROBES)		+= probes.o kprobes.o kprobes-common.o patch.o
+obj-$(CONFIG_KPROBES)		+= probes.o kprobes.o kprobes-common.o patch.o probes-checkers-common.o
 ifdef CONFIG_THUMB2_KERNEL
-obj-$(CONFIG_KPROBES)		+= kprobes-thumb.o probes-thumb.o
+obj-$(CONFIG_KPROBES)		+= kprobes-thumb.o probes-thumb.o probes-checkers-thumb.o
 else
-obj-$(CONFIG_KPROBES)		+= kprobes-arm.o probes-arm.o
+obj-$(CONFIG_KPROBES)		+= kprobes-arm.o probes-arm.o probes-checkers-arm.o
 endif
 obj-$(CONFIG_ARM_KPROBES_TEST)	+= test-kprobes.o
 test-kprobes-objs		:= kprobes-test.o
diff --git a/arch/arm/kernel/kprobes-arm.c b/arch/arm/kernel/kprobes-arm.c
index 5a81275..56f78ce 100644
--- a/arch/arm/kernel/kprobes-arm.c
+++ b/arch/arm/kernel/kprobes-arm.c
@@ -64,6 +64,7 @@
 
 #include "kprobes.h"
 #include "probes-arm.h"
+#include "probes-checkers.h"
 
 #if  __LINUX_ARM_ARCH__ >= 6
 #define BLX(reg)	"blx	"reg"		\n\t"
@@ -342,4 +343,4 @@ const union decode_action kprobes_arm_actions[NUM_PROBES_ARM_ACTIONS] = {
 	[PROBES_LDMSTM] = {.decoder = kprobe_decode_ldmstm}
 };
 
-const struct decode_checker *kprobes_arm_checkers[] = {NULL};
+const struct decode_checker *kprobes_arm_checkers[] = {arm_stack_checker, NULL};
diff --git a/arch/arm/kernel/kprobes-thumb.c b/arch/arm/kernel/kprobes-thumb.c
index b8ba7d2..dbfd3ca 100644
--- a/arch/arm/kernel/kprobes-thumb.c
+++ b/arch/arm/kernel/kprobes-thumb.c
@@ -15,6 +15,7 @@
 
 #include "kprobes.h"
 #include "probes-thumb.h"
+#include "probes-checkers.h"
 
 /* These emulation encodings are functionally equivalent... */
 #define t32_emulate_rd8rn16rm0ra12_noflags \
@@ -665,5 +666,5 @@ const union decode_action kprobes_t32_actions[NUM_PROBES_T32_ACTIONS] = {
 		.handler = t32_emulate_rdlo12rdhi8rn16rm0_noflags},
 };
 
-const struct decode_checker *kprobes_t32_checkers[] = {NULL};
-const struct decode_checker *kprobes_t16_checkers[] = {NULL};
+const struct decode_checker *kprobes_t32_checkers[] = {t32_stack_checker, NULL};
+const struct decode_checker *kprobes_t16_checkers[] = {t16_stack_checker, NULL};
diff --git a/arch/arm/kernel/probes-checkers-arm.c b/arch/arm/kernel/probes-checkers-arm.c
new file mode 100644
index 0000000..53f6736
--- /dev/null
+++ b/arch/arm/kernel/probes-checkers-arm.c
@@ -0,0 +1,99 @@
+/*
+ * arch/arm/kernel/probes-checkers-arm.c
+ *
+ * Copyright (C) 2014 Huawei Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include "probes.h"
+#include "probes-arm.h"
+#include "probes-checkers.h"
+
+static enum probes_insn __kprobes arm_check_stack(probes_opcode_t insn,
+		struct arch_probes_insn *asi,
+		const struct decode_header *h)
+{
+	/*
+	 * PROBES_LDRSTRD, PROBES_LDMSTM, PROBES_STORE,
+	 * PROBES_STORE_EXTRA may get here. Simply mark all normal
+	 * insns as STACK_USE_NONE.
+	 */
+	static const union decode_item table[] = {
+		/*
+		 * 'STR{,D,B,H}, Rt, [Rn, Rm]' should be marked as UNKNOWN
+		 * if Rn or Rm is SP.
+		 *                                 x
+		 * STR (register)	cccc 011x x0x0 xxxx xxxx xxxx xxxx xxxx
+		 * STRB (register)	cccc 011x x1x0 xxxx xxxx xxxx xxxx xxxx
+		 */
+		DECODE_OR	(0x0e10000f, 0x0600000d),
+		DECODE_OR	(0x0e1f0000, 0x060d0000),
+
+		/*
+		 *                                                     x
+		 * STRD (register)	cccc 000x x0x0 xxxx xxxx xxxx 1111 xxxx
+		 * STRH (register)	cccc 000x x0x0 xxxx xxxx xxxx 1011 xxxx
+		 */
+		DECODE_OR	(0x0e5000bf, 0x000000bd),
+		DECODE_CUSTOM	(0x0e5f00b0, 0x000d00b0, STACK_USE_UNKNOWN),
+
+		/*
+		 * For PROBES_LDMSTM, only stmdx sp, [...] need to examine
+		 *
+		 * Bit B/A (bit 24) encodes arithmetic operation order. 1 means
+		 * before, 0 means after.
+		 * Bit I/D (bit 23) encodes arithmetic operation. 1 means
+		 * increment, 0 means decrement.
+		 *
+		 * So:
+		 *                              B I
+		 *                              / /
+		 *                              A D   | Rn |
+		 * STMDX SP, [...]	cccc 100x 00x0 xxxx xxxx xxxx xxxx xxxx
+		 */
+		DECODE_CUSTOM	(0x0edf0000, 0x080d0000, STACK_USE_STMDX),
+
+		/*                              P U W | Rn | Rt |     imm12    |*/
+		/* STR (immediate)	cccc 010x x0x0 1101 xxxx xxxx xxxx xxxx */
+		/* STRB (immediate)	cccc 010x x1x0 1101 xxxx xxxx xxxx xxxx */
+		/*                              P U W | Rn | Rt |imm4|    |imm4|*/
+		/* STRD (immediate)	cccc 000x x1x0 1101 xxxx xxxx 1111 xxxx */
+		/* STRH (immediate)	cccc 000x x1x0 1101 xxxx xxxx 1011 xxxx */
+		/*
+		 * index = (P == '1'); add = (U == '1').
+		 * Above insns with:
+		 *    index == 0 (str{,d,h} rx, [sp], #+/-imm) or
+		 *    add == 1 (str{,d,h} rx, [sp, #+<imm>])
+		 * should be STACK_USE_NONE.
+		 * Only str{,b,d,h} rx,[sp,#-n] (P == 1 and U == 0) are
+		 * required to be examined.
+		 */
+		/* STR{,B} Rt,[SP,#-n]	cccc 0101 0xx0 1101 xxxx xxxx xxxx xxxx */
+		DECODE_CUSTOM	(0x0f9f0000, 0x050d0000, STACK_USE_FIXED_XXX),
+
+		/* STR{D,H} Rt,[SP,#-n]	cccc 0001 01x0 1101 xxxx xxxx 1x11 xxxx */
+		DECODE_CUSTOM	(0x0fdf00b0, 0x014d00b0, STACK_USE_FIXED_X0X),
+
+		/* fall through */
+		DECODE_CUSTOM	(0, 0, STACK_USE_NONE),
+		DECODE_END
+	};
+
+	return probes_decode_insn(insn, asi, table, false, false, stack_check_actions, NULL);
+}
+
+const struct decode_checker arm_stack_checker[NUM_PROBES_ARM_ACTIONS] = {
+	[PROBES_LDRSTRD] = {.checker = arm_check_stack},
+	[PROBES_STORE_EXTRA] = {.checker = arm_check_stack},
+	[PROBES_STORE] = {.checker = arm_check_stack},
+	[PROBES_LDMSTM] = {.checker = arm_check_stack},
+};
diff --git a/arch/arm/kernel/probes-checkers-common.c b/arch/arm/kernel/probes-checkers-common.c
new file mode 100644
index 0000000..e46f844
--- /dev/null
+++ b/arch/arm/kernel/probes-checkers-common.c
@@ -0,0 +1,87 @@
+/*
+ * arch/arm/kernel/probes-checkers-common.c
+ *
+ * Copyright (C) 2014 Huawei Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include "probes.h"
+#include "probes-arm.h"
+#include "probes-checkers.h"
+
+enum probes_insn checker_stack_use_none(probes_opcode_t insn,
+		struct arch_probes_insn *asi,
+		const struct decode_header *h)
+{
+	asi->stack_space = 0;
+	return INSN_GOOD_NO_SLOT;
+}
+
+enum probes_insn checker_stack_use_unknown(probes_opcode_t insn,
+		struct arch_probes_insn *asi,
+		const struct decode_header *h)
+{
+	asi->stack_space = -1;
+	return INSN_GOOD_NO_SLOT;
+}
+
+#ifdef CONFIG_THUMB2_KERNEL
+enum probes_insn checker_stack_use_imm_0xx(probes_opcode_t insn,
+		struct arch_probes_insn *asi,
+		const struct decode_header *h)
+{
+	int imm = insn & 0xff;
+	asi->stack_space = imm;
+	return INSN_GOOD_NO_SLOT;
+}
+#else
+enum probes_insn checker_stack_use_imm_x0x(probes_opcode_t insn,
+		struct arch_probes_insn *asi,
+		const struct decode_header *h)
+{
+	int imm = ((insn & 0xf00) >> 4) + (insn & 0xf);
+	asi->stack_space = imm;
+	return INSN_GOOD_NO_SLOT;
+}
+#endif
+
+enum probes_insn checker_stack_use_imm_xxx(probes_opcode_t insn,
+		struct arch_probes_insn *asi,
+		const struct decode_header *h)
+{
+	int imm = insn & 0xfff;
+	asi->stack_space = imm;
+	return INSN_GOOD_NO_SLOT;
+}
+
+enum probes_insn checker_stack_use_stmdx(probes_opcode_t insn,
+		struct arch_probes_insn *asi,
+		const struct decode_header *h)
+{
+	unsigned int reglist = insn & 0xffff;
+	int pbit = insn & (1 << 24);
+	asi->stack_space = (hweight32(reglist) - (!pbit ? 1 : 0)) * 4;
+
+	return INSN_GOOD_NO_SLOT;
+}
+
+const union decode_action stack_check_actions[] = {
+	[STACK_USE_NONE] = {.decoder = checker_stack_use_none},
+	[STACK_USE_UNKNOWN] = {.decoder = checker_stack_use_unknown},
+#ifdef CONFIG_THUMB2_KERNEL
+	[STACK_USE_FIXED_0XX] = {.decoder = checker_stack_use_imm_0xx},
+#else
+	[STACK_USE_FIXED_X0X] = {.decoder = checker_stack_use_imm_x0x},
+#endif
+	[STACK_USE_FIXED_XXX] = {.decoder = checker_stack_use_imm_xxx},
+	[STACK_USE_STMDX] = {.decoder = checker_stack_use_stmdx},
+};
diff --git a/arch/arm/kernel/probes-checkers-thumb.c b/arch/arm/kernel/probes-checkers-thumb.c
new file mode 100644
index 0000000..2976e5c
--- /dev/null
+++ b/arch/arm/kernel/probes-checkers-thumb.c
@@ -0,0 +1,106 @@
+/*
+ * arch/arm/kernel/probes-checkers-arm.c
+ *
+ * Copyright (C) 2014 Huawei Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include "probes.h"
+#include "probes-thumb.h"
+#include "probes-checkers.h"
+
+static enum probes_insn __kprobes t32_check_stack(probes_opcode_t insn,
+		struct arch_probes_insn *asi,
+		const struct decode_header *h)
+{
+	/*
+	 * PROBES_T32_LDMSTM, PROBES_T32_LDRDSTRD and PROBES_T32_LDRSTR
+	 * may get here. Simply mark all normal insns as STACK_USE_NONE.
+	 */
+	static const union decode_item table[] = {
+
+		/*
+		 * First, filter out all ldr insns to make our life easier.
+		 * Following load insns may come here:
+		 * LDM, LDRD, LDR.
+		 * In T32 encoding, bit 20 is enough for distinguishing
+		 * load and store. All load insns have this bit set, when
+		 * all store insns have this bit clear.
+		 */
+		DECODE_CUSTOM	(0x00100000, 0x00100000, STACK_USE_NONE),
+
+		/*
+		 * Mark all 'STR{,B,H}, Rt, [Rn, Rm]' as STACK_USE_UNKNOWN
+		 * if Rn or Rm is SP. T32 doesn't encode STRD.
+		 */
+		/*                                    | Rn | Rt |         | Rm |*/
+		/* STR (register)	1111 1000 0100 xxxx xxxx 0000 00xx xxxx */
+		/* STRB (register)	1111 1000 0000 xxxx xxxx 0000 00xx xxxx */
+		/* STRH (register)	1111 1000 0010 xxxx xxxx 0000 00xx xxxx */
+		/* INVALID INSN		1111 1000 0110 xxxx xxxx 0000 00xx xxxx */
+		/* By Introducing INVALID INSN, bit 21 and 22 can be ignored. */
+		DECODE_OR	(0xff9f0fc0, 0xf80d0000),
+		DECODE_CUSTOM	(0xff900fcf, 0xf800000d, STACK_USE_UNKNOWN),
+
+
+		/*                                    | Rn | Rt | PUW|   imm8  |*/
+		/* STR (imm 8)		1111 1000 0100 xxxx xxxx 110x xxxx xxxx */
+		/* STRB (imm 8)		1111 1000 0000 xxxx xxxx 110x xxxx xxxx */
+		/* STRH (imm 8)		1111 1000 0010 xxxx xxxx 110x xxxx xxxx */
+		/* INVALID INSN		1111 1000 0110 xxxx xxxx 110x xxxx xxxx */
+		/* Only consider U == 0 and P == 1: strx rx, [sp, #-<imm>] */
+		DECODE_CUSTOM	(0xff9f0e00, 0xf80d0c00, STACK_USE_FIXED_0XX),
+
+		/* For STR{,B,H} (imm 12), offset is always positive, so ignore them. */
+
+		/*                              P U W | Rn | Rt | Rt2|   imm8  |*/
+		/* STRD (immediate)	1110 1001 01x0 1101 xxxx xxxx xxxx xxxx */
+		/* Only consider U == 0 and P == 1 */
+		DECODE_CUSTOM	(0xffdf0000, 0xe94d0000, STACK_USE_FIXED_0XX),
+
+		/*                                    | Rn | */
+		/* STMDB		1110 1001 00x0 1101 xxxx xxxx xxxx xxxx */
+		DECODE_CUSTOM	(0xffdf0000, 0xe94d0000, STACK_USE_STMDX),
+
+		/* fall through */
+		DECODE_CUSTOM	(0, 0, STACK_USE_NONE),
+		DECODE_END
+	};
+
+	return probes_decode_insn(insn, asi, table, false, false, stack_check_actions, NULL);
+}
+
+const struct decode_checker t32_stack_checker[NUM_PROBES_T32_ACTIONS] = {
+	[PROBES_T32_LDMSTM] = {.checker = t32_check_stack},
+	[PROBES_T32_LDRDSTRD] = {.checker = t32_check_stack},
+	[PROBES_T32_LDRSTR] = {.checker = t32_check_stack},
+};
+
+/*
+ * See following comments. This insn must be 'push'.
+ */
+static enum probes_insn __kprobes t16_check_stack(probes_opcode_t insn,
+		struct arch_probes_insn *asi,
+		const struct decode_header *h)
+{
+	unsigned int reglist = insn & 0x1ff;
+	asi->stack_space = hweight32(reglist) * 4;
+	return INSN_GOOD;
+}
+
+/*
+ * T16 encoding is simple: only the 'push' insn can need extra stack space.
+ * Other insns, like str, can only use r0-r7 as Rn.
+ */
+const struct decode_checker t16_stack_checker[NUM_PROBES_T16_ACTIONS] = {
+	[PROBES_T16_PUSH] = {.checker = t16_check_stack},
+};
diff --git a/arch/arm/kernel/probes-checkers.h b/arch/arm/kernel/probes-checkers.h
new file mode 100644
index 0000000..07825ab
--- /dev/null
+++ b/arch/arm/kernel/probes-checkers.h
@@ -0,0 +1,53 @@
+/*
+ * arch/arm/kernel/probes-checkers-common.c
+ *
+ * Copyright (C) 2014 Huawei Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+#ifndef _ARM_KERNEL_PROBES_CHECKERS_H
+#define _ARM_KERNEL_PROBES_CHECKERS_H
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include "probes.h"
+
+extern probes_check_t checker_stack_use_none;
+extern probes_check_t checker_stack_use_unknown;
+#ifdef CONFIG_THUMB2_KERNEL
+extern probes_check_t checker_stack_use_imm_0xx;
+#else
+extern probes_check_t checker_stack_use_imm_x0x;
+#endif
+extern probes_check_t checker_stack_use_imm_xxx;
+extern probes_check_t checker_stack_use_stmdx;
+
+enum {
+	STACK_USE_NONE,
+	STACK_USE_UNKNOWN,
+#ifdef CONFIG_THUMB2_KERNEL
+	STACK_USE_FIXED_0XX,
+#else
+	STACK_USE_FIXED_X0X,
+#endif
+	STACK_USE_FIXED_XXX,
+	STACK_USE_STMDX,
+	NUM_STACK_USE_TYPES
+};
+
+extern const union decode_action stack_check_actions[];
+
+#ifndef CONFIG_THUMB2_KERNEL
+extern const struct decode_checker arm_stack_checker[];
+#else
+#endif
+extern const struct decode_checker t32_stack_checker[];
+extern const struct decode_checker t16_stack_checker[];
+#endif
diff --git a/arch/arm/kernel/probes.c b/arch/arm/kernel/probes.c
index 6a7d92e..cc68986 100644
--- a/arch/arm/kernel/probes.c
+++ b/arch/arm/kernel/probes.c
@@ -425,6 +425,16 @@ probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
 	 */
 	probes_opcode_t origin_insn = insn;
 
+	/*
+	 * stack_space is initialized to 0 here. Checker functions
+	 * should update is value if they find this is a stack store
+	 * instruction: positive value means bytes of stack usage,
+	 * negitive value means unable to determine stack usage
+	 * statically. For instruction doesn't store to stack, checker
+	 * do nothing with it.
+	 */
+	asi->stack_space = 0;
+
 	if (emulate)
 		insn = prepare_emulated_insn(insn, asi, thumb);
 
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/3] ARM: kprobes: collects stack consumption for store instructions
  2014-11-21  6:35 ` [PATCH v3 2/3] ARM: kprobes: collects stack consumption for store instructions Wang Nan
@ 2014-11-21 17:18   ` Jon Medhurst (Tixy)
  0 siblings, 0 replies; 7+ messages in thread
From: Jon Medhurst (Tixy) @ 2014-11-21 17:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 2014-11-21 at 14:35 +0800, Wang Nan wrote:
> This patch uses the previously introduced checker functionality on
> store instructions to record their stack consumption information to
> arch_probes_insn.
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Signed-off-by: Jon Medhurst <tixy@linaro.org>
> Reviewed-by: Jon Medhurst <tixy@linaro.org>
> 
> ---

During testing of these patches I found a couple of bugs in the 32-bit
thumb instruction decoding...

[...]
> +static enum probes_insn __kprobes t32_check_stack(probes_opcode_t insn,
> +		struct arch_probes_insn *asi,
> +		const struct decode_header *h)
> +{
> +	/*
> +	 * PROBES_T32_LDMSTM, PROBES_T32_LDRDSTRD and PROBES_T32_LDRSTR
> +	 * may get here. Simply mark all normal insns as STACK_USE_NONE.
> +	 */
> +	static const union decode_item table[] = {
> +
> +		/*
> +		 * First, filter out all ldr insns to make our life easier.
> +		 * Following load insns may come here:
> +		 * LDM, LDRD, LDR.
> +		 * In T32 encoding, bit 20 is enough for distinguishing
> +		 * load and store. All load insns have this bit set, when
> +		 * all store insns have this bit clear.
> +		 */
> +		DECODE_CUSTOM	(0x00100000, 0x00100000, STACK_USE_NONE),
> +
> +		/*
> +		 * Mark all 'STR{,B,H}, Rt, [Rn, Rm]' as STACK_USE_UNKNOWN
> +		 * if Rn or Rm is SP. T32 doesn't encode STRD.
> +		 */
> +		/*                                    | Rn | Rt |         | Rm |*/
> +		/* STR (register)	1111 1000 0100 xxxx xxxx 0000 00xx xxxx */
> +		/* STRB (register)	1111 1000 0000 xxxx xxxx 0000 00xx xxxx */
> +		/* STRH (register)	1111 1000 0010 xxxx xxxx 0000 00xx xxxx */
> +		/* INVALID INSN		1111 1000 0110 xxxx xxxx 0000 00xx xxxx */
> +		/* By Introducing INVALID INSN, bit 21 and 22 can be ignored. */
> +		DECODE_OR	(0xff9f0fc0, 0xf80d0000),
> +		DECODE_CUSTOM	(0xff900fcf, 0xf800000d, STACK_USE_UNKNOWN),
> +
> +
> +		/*                                    | Rn | Rt | PUW|   imm8  |*/
> +		/* STR (imm 8)		1111 1000 0100 xxxx xxxx 110x xxxx xxxx */
> +		/* STRB (imm 8)		1111 1000 0000 xxxx xxxx 110x xxxx xxxx */
> +		/* STRH (imm 8)		1111 1000 0010 xxxx xxxx 110x xxxx xxxx */
> +		/* INVALID INSN		1111 1000 0110 xxxx xxxx 110x xxxx xxxx */
> +		/* Only consider U == 0 and P == 1: strx rx, [sp, #-<imm>] */
> +		DECODE_CUSTOM	(0xff9f0e00, 0xf80d0c00, STACK_USE_FIXED_0XX),
> +
> +		/* For STR{,B,H} (imm 12), offset is always positive, so ignore them. */
> +
> +		/*                              P U W | Rn | Rt | Rt2|   imm8  |*/
> +		/* STRD (immediate)	1110 1001 01x0 1101 xxxx xxxx xxxx xxxx */
> +		/* Only consider U == 0 and P == 1 */
> +		DECODE_CUSTOM	(0xffdf0000, 0xe94d0000, STACK_USE_FIXED_0XX),

For the encoding of LDRD, the 8 bit immediate value is the number of
words not bytes and so needs multiplying by 4. This will need an
additional enum and function to handle that.


> +
> +		/*                                    | Rn | */
> +		/* STMDB		1110 1001 00x0 1101 xxxx xxxx xxxx xxxx */
> +		DECODE_CUSTOM	(0xffdf0000, 0xe94d0000, STACK_USE_STMDX),

The match value is incorrect (the same as used for LDRD), this should
be 

		DECODE_CUSTOM	(0xffdf0000, 0xe90d0000, STACK_USE_STMDX),


-- 
Tixy

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 3/3] ARM: kprobes: disallow probing stack consuming instructions
  2014-11-21  6:35 [PATCH v3 0/3] ARM: kprobes: introduces instruction checker Wang Nan
  2014-11-21  6:35 ` [PATCH v3 1/3] ARM: kprobes: introduces checker Wang Nan
  2014-11-21  6:35 ` [PATCH v3 2/3] ARM: kprobes: collects stack consumption for store instructions Wang Nan
@ 2014-11-21  6:35 ` Wang Nan
  2014-11-21 17:59   ` Jon Medhurst (Tixy)
  2 siblings, 1 reply; 7+ messages in thread
From: Wang Nan @ 2014-11-21  6:35 UTC (permalink / raw)
  To: linux-arm-kernel

This patch prohibit probing instructions for which the stack
requirement are unable to be determined statically. Some test cases
are found not work again after the modification, this patch also
removes them.

Signed-off-by: Wang Nan <wangnan0@huawei.com>

---

v1 -> v2:
 - Use MAX_STACK_SIZE macro instead of hard coded stack size.

v2 -> v3:
 - Commit message improvements.
---
 arch/arm/include/asm/kprobes.h     |  1 -
 arch/arm/include/asm/probes.h      | 12 ++++++++++++
 arch/arm/kernel/entry-armv.S       |  3 ++-
 arch/arm/kernel/kprobes-test-arm.c | 16 ++++++++++------
 arch/arm/kernel/kprobes.c          |  9 +++++++++
 5 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/arch/arm/include/asm/kprobes.h b/arch/arm/include/asm/kprobes.h
index 49fa0df..56f9ac6 100644
--- a/arch/arm/include/asm/kprobes.h
+++ b/arch/arm/include/asm/kprobes.h
@@ -22,7 +22,6 @@
 
 #define __ARCH_WANT_KPROBES_INSN_SLOT
 #define MAX_INSN_SIZE			2
-#define MAX_STACK_SIZE			64	/* 32 would probably be OK */
 
 #define flush_insn_slot(p)		do { } while (0)
 #define kretprobe_blacklist_size	0
diff --git a/arch/arm/include/asm/probes.h b/arch/arm/include/asm/probes.h
index ccf9af3..f0a1ee8 100644
--- a/arch/arm/include/asm/probes.h
+++ b/arch/arm/include/asm/probes.h
@@ -19,6 +19,8 @@
 #ifndef _ASM_PROBES_H
 #define _ASM_PROBES_H
 
+#ifndef __ASSEMBLY__
+
 typedef u32 probes_opcode_t;
 
 struct arch_probes_insn;
@@ -41,4 +43,14 @@ struct arch_probes_insn {
 	int stack_space;
 };
 
+#endif /* __ASSEMBLY__ */
+
+/*
+ * We assume one instruction can consume at most 64 bytes stack, which is
+ * 'push {r0-r15}'. Instructions consume more or unknown stack space like
+ * 'str r0, [sp, #-80]' and 'str r0, [sp, r1]' should be prohibit to probe.
+ * Both kprobe and jprobe use this macro.
+ */
+#define MAX_STACK_SIZE			64
+
 #endif
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 2f5555d..672b219 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -31,6 +31,7 @@
 
 #include "entry-header.S"
 #include <asm/entry-macro-multi.S>
+#include <asm/probes.h>
 
 /*
  * Interrupt handling.
@@ -249,7 +250,7 @@ __und_svc:
 	@ If a kprobe is about to simulate a "stmdb sp..." instruction,
 	@ it obviously needs free stack space which then will belong to
 	@ the saved context.
-	svc_entry 64
+	svc_entry MAX_STACK_SIZE
 #else
 	svc_entry
 #endif
diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
index cb14242..d8ad5bb 100644
--- a/arch/arm/kernel/kprobes-test-arm.c
+++ b/arch/arm/kernel/kprobes-test-arm.c
@@ -476,7 +476,8 @@ void kprobe_arm_test_cases(void)
 	TEST_GROUP("Extra load/store instructions")
 
 	TEST_RPR(  "strh	r",0, VAL1,", [r",1, 48,", -r",2, 24,"]")
-	TEST_RPR(  "streqh	r",14,VAL2,", [r",13,0, ", r",12, 48,"]")
+	TEST_RPR(  "streqh	r",14,VAL2,", [r",11,0, ", r",12, 48,"]")
+	TEST_UNSUPPORTED(  "streqh	r14, [r13, r12]")
 	TEST_RPR(  "strh	r",1, VAL1,", [r",2, 24,", r",3,  48,"]!")
 	TEST_RPR(  "strneh	r",12,VAL2,", [r",11,48,", -r",10,24,"]!")
 	TEST_RPR(  "strh	r",2, VAL1,", [r",3, 24,"], r",4, 48,"")
@@ -565,7 +566,8 @@ void kprobe_arm_test_cases(void)
 
 #if __LINUX_ARM_ARCH__ >= 5
 	TEST_RPR(  "strd	r",0, VAL1,", [r",1, 48,", -r",2,24,"]")
-	TEST_RPR(  "strccd	r",8, VAL2,", [r",13,0, ", r",12,48,"]")
+	TEST_RPR(  "strccd	r",8, VAL2,", [r",11,0, ", r",12,48,"]")
+	TEST_UNSUPPORTED(  "strccd r8, [r13, r12]")
 	TEST_RPR(  "strd	r",4, VAL1,", [r",2, 24,", r",3, 48,"]!")
 	TEST_RPR(  "strcsd	r",12,VAL2,", [r",11,48,", -r",10,24,"]!")
 	TEST_RPR(  "strd	r",2, VAL1,", [r",5, 24,"], r",4,48,"")
@@ -638,13 +640,15 @@ void kprobe_arm_test_cases(void)
 	TEST_RP( "str"byte"	r",2, VAL1,", [r",3, 24,"], #48")		\
 	TEST_RP( "str"byte"	r",10,VAL2,", [r",9, 64,"], #-48")		\
 	TEST_RPR("str"byte"	r",0, VAL1,", [r",1, 48,", -r",2, 24,"]")	\
-	TEST_RPR("str"byte"	r",14,VAL2,", [r",13,0, ", r",12, 48,"]")	\
+	TEST_RPR("str"byte"	r",14,VAL2,", [r",11,0, ", r",12, 48,"]")	\
+	TEST_UNSUPPORTED("str"byte" r14, [r13, r12]")	\
 	TEST_RPR("str"byte"	r",1, VAL1,", [r",2, 24,", r",3,  48,"]!")	\
 	TEST_RPR("str"byte"	r",12,VAL2,", [r",11,48,", -r",10,24,"]!")	\
 	TEST_RPR("str"byte"	r",2, VAL1,", [r",3, 24,"], r",4, 48,"")	\
 	TEST_RPR("str"byte"	r",10,VAL2,", [r",9, 48,"], -r",11,24,"")	\
 	TEST_RPR("str"byte"	r",0, VAL1,", [r",1, 24,", r",2,  32,", asl #1]")\
-	TEST_RPR("str"byte"	r",14,VAL2,", [r",13,0, ", r",12, 32,", lsr #2]")\
+	TEST_RPR("str"byte"	r",14,VAL2,", [r",11,0, ", r",12, 32,", lsr #2]")\
+	TEST_UNSUPPORTED("str"byte"	r14, [r13, r12, lsr #2]")\
 	TEST_RPR("str"byte"	r",1, VAL1,", [r",2, 24,", r",3,  32,", asr #3]!")\
 	TEST_RPR("str"byte"	r",12,VAL2,", [r",11,24,", r",10, 4,", ror #31]!")\
 	TEST_P(  "ldr"byte"	r0, [r",0,  24,", #-2]")			\
@@ -668,12 +672,12 @@ void kprobe_arm_test_cases(void)
 
 	LOAD_STORE("")
 	TEST_P(   "str	pc, [r",0,0,", #15*4]")
-	TEST_R(   "str	pc, [sp, r",2,15*4,"]")
+	TEST_UNSUPPORTED(   "str	pc, [sp, r2]")
 	TEST_BF(  "ldr	pc, [sp, #15*4]")
 	TEST_BF_R("ldr	pc, [sp, r",2,15*4,"]")
 
 	TEST_P(   "str	sp, [r",0,0,", #13*4]")
-	TEST_R(   "str	sp, [sp, r",2,13*4,"]")
+	TEST_UNSUPPORTED(   "str	sp, [sp, r2]")
 	TEST_BF(  "ldr	sp, [sp, #13*4]")
 	TEST_BF_R("ldr	sp, [sp, r",2,13*4,"]")
 
diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
index d7bee4b..30498436 100644
--- a/arch/arm/kernel/kprobes.c
+++ b/arch/arm/kernel/kprobes.c
@@ -115,6 +115,15 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
 		break;
 	}
 
+	/*
+	 * Never instrument insn like 'str r0, [sp, +/-r1]'. Also, insn likes
+	 * 'str r0, [sp, #-68]' should also be prohibited.
+	 * See __und_svc.
+	 */
+	if ((p->ainsn.stack_space < 0) ||
+			(p->ainsn.stack_space > MAX_STACK_SIZE))
+		return -EINVAL;
+
 	return 0;
 }
 
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 3/3] ARM: kprobes: disallow probing stack consuming instructions
  2014-11-21  6:35 ` [PATCH v3 3/3] ARM: kprobes: disallow probing stack consuming instructions Wang Nan
@ 2014-11-21 17:59   ` Jon Medhurst (Tixy)
  2014-11-21 20:11     ` [PATCH] ARM: kprobes: Add test cases for " Jon Medhurst (Tixy)
  0 siblings, 1 reply; 7+ messages in thread
From: Jon Medhurst (Tixy) @ 2014-11-21 17:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 2014-11-21 at 14:35 +0800, Wang Nan wrote:
> This patch prohibit probing instructions for which the stack

Needs an 's' on the end of 'prohibit'

> requirement are unable to be determined statically. Some test cases

and another 's' on the end of 'requirement'

> are found not work again after the modification, this patch also
> removes them.
> 
> Signed-off-by: Wang Nan <wangnan0@huawei.com>

Reviewed-by: Jon Medhurst <tixy@linaro.org>

I think we also need to add new test cases to cover the stack store
instructions. I have already produced these (which is how I found the
Thumb decoding bugs) so I will follow up with a separate patch with
those. It would be good if you could that it to the end of your series.

Thanks

-- 
Tixy

> ---
> 
> v1 -> v2:
>  - Use MAX_STACK_SIZE macro instead of hard coded stack size.
> 
> v2 -> v3:
>  - Commit message improvements.
> ---
>  arch/arm/include/asm/kprobes.h     |  1 -
>  arch/arm/include/asm/probes.h      | 12 ++++++++++++
>  arch/arm/kernel/entry-armv.S       |  3 ++-
>  arch/arm/kernel/kprobes-test-arm.c | 16 ++++++++++------
>  arch/arm/kernel/kprobes.c          |  9 +++++++++
>  5 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kprobes.h b/arch/arm/include/asm/kprobes.h
> index 49fa0df..56f9ac6 100644
> --- a/arch/arm/include/asm/kprobes.h
> +++ b/arch/arm/include/asm/kprobes.h
> @@ -22,7 +22,6 @@
>  
>  #define __ARCH_WANT_KPROBES_INSN_SLOT
>  #define MAX_INSN_SIZE			2
> -#define MAX_STACK_SIZE			64	/* 32 would probably be OK */
>  
>  #define flush_insn_slot(p)		do { } while (0)
>  #define kretprobe_blacklist_size	0
> diff --git a/arch/arm/include/asm/probes.h b/arch/arm/include/asm/probes.h
> index ccf9af3..f0a1ee8 100644
> --- a/arch/arm/include/asm/probes.h
> +++ b/arch/arm/include/asm/probes.h
> @@ -19,6 +19,8 @@
>  #ifndef _ASM_PROBES_H
>  #define _ASM_PROBES_H
>  
> +#ifndef __ASSEMBLY__
> +
>  typedef u32 probes_opcode_t;
>  
>  struct arch_probes_insn;
> @@ -41,4 +43,14 @@ struct arch_probes_insn {
>  	int stack_space;
>  };
>  
> +#endif /* __ASSEMBLY__ */
> +
> +/*
> + * We assume one instruction can consume at most 64 bytes stack, which is
> + * 'push {r0-r15}'. Instructions consume more or unknown stack space like
> + * 'str r0, [sp, #-80]' and 'str r0, [sp, r1]' should be prohibit to probe.
> + * Both kprobe and jprobe use this macro.
> + */
> +#define MAX_STACK_SIZE			64
> +
>  #endif
> diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
> index 2f5555d..672b219 100644
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -31,6 +31,7 @@
>  
>  #include "entry-header.S"
>  #include <asm/entry-macro-multi.S>
> +#include <asm/probes.h>
>  
>  /*
>   * Interrupt handling.
> @@ -249,7 +250,7 @@ __und_svc:
>  	@ If a kprobe is about to simulate a "stmdb sp..." instruction,
>  	@ it obviously needs free stack space which then will belong to
>  	@ the saved context.
> -	svc_entry 64
> +	svc_entry MAX_STACK_SIZE
>  #else
>  	svc_entry
>  #endif
> diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
> index cb14242..d8ad5bb 100644
> --- a/arch/arm/kernel/kprobes-test-arm.c
> +++ b/arch/arm/kernel/kprobes-test-arm.c
> @@ -476,7 +476,8 @@ void kprobe_arm_test_cases(void)
>  	TEST_GROUP("Extra load/store instructions")
>  
>  	TEST_RPR(  "strh	r",0, VAL1,", [r",1, 48,", -r",2, 24,"]")
> -	TEST_RPR(  "streqh	r",14,VAL2,", [r",13,0, ", r",12, 48,"]")
> +	TEST_RPR(  "streqh	r",14,VAL2,", [r",11,0, ", r",12, 48,"]")
> +	TEST_UNSUPPORTED(  "streqh	r14, [r13, r12]")
>  	TEST_RPR(  "strh	r",1, VAL1,", [r",2, 24,", r",3,  48,"]!")
>  	TEST_RPR(  "strneh	r",12,VAL2,", [r",11,48,", -r",10,24,"]!")
>  	TEST_RPR(  "strh	r",2, VAL1,", [r",3, 24,"], r",4, 48,"")
> @@ -565,7 +566,8 @@ void kprobe_arm_test_cases(void)
>  
>  #if __LINUX_ARM_ARCH__ >= 5
>  	TEST_RPR(  "strd	r",0, VAL1,", [r",1, 48,", -r",2,24,"]")
> -	TEST_RPR(  "strccd	r",8, VAL2,", [r",13,0, ", r",12,48,"]")
> +	TEST_RPR(  "strccd	r",8, VAL2,", [r",11,0, ", r",12,48,"]")
> +	TEST_UNSUPPORTED(  "strccd r8, [r13, r12]")
>  	TEST_RPR(  "strd	r",4, VAL1,", [r",2, 24,", r",3, 48,"]!")
>  	TEST_RPR(  "strcsd	r",12,VAL2,", [r",11,48,", -r",10,24,"]!")
>  	TEST_RPR(  "strd	r",2, VAL1,", [r",5, 24,"], r",4,48,"")
> @@ -638,13 +640,15 @@ void kprobe_arm_test_cases(void)
>  	TEST_RP( "str"byte"	r",2, VAL1,", [r",3, 24,"], #48")		\
>  	TEST_RP( "str"byte"	r",10,VAL2,", [r",9, 64,"], #-48")		\
>  	TEST_RPR("str"byte"	r",0, VAL1,", [r",1, 48,", -r",2, 24,"]")	\
> -	TEST_RPR("str"byte"	r",14,VAL2,", [r",13,0, ", r",12, 48,"]")	\
> +	TEST_RPR("str"byte"	r",14,VAL2,", [r",11,0, ", r",12, 48,"]")	\
> +	TEST_UNSUPPORTED("str"byte" r14, [r13, r12]")	\
>  	TEST_RPR("str"byte"	r",1, VAL1,", [r",2, 24,", r",3,  48,"]!")	\
>  	TEST_RPR("str"byte"	r",12,VAL2,", [r",11,48,", -r",10,24,"]!")	\
>  	TEST_RPR("str"byte"	r",2, VAL1,", [r",3, 24,"], r",4, 48,"")	\
>  	TEST_RPR("str"byte"	r",10,VAL2,", [r",9, 48,"], -r",11,24,"")	\
>  	TEST_RPR("str"byte"	r",0, VAL1,", [r",1, 24,", r",2,  32,", asl #1]")\
> -	TEST_RPR("str"byte"	r",14,VAL2,", [r",13,0, ", r",12, 32,", lsr #2]")\
> +	TEST_RPR("str"byte"	r",14,VAL2,", [r",11,0, ", r",12, 32,", lsr #2]")\
> +	TEST_UNSUPPORTED("str"byte"	r14, [r13, r12, lsr #2]")\
>  	TEST_RPR("str"byte"	r",1, VAL1,", [r",2, 24,", r",3,  32,", asr #3]!")\
>  	TEST_RPR("str"byte"	r",12,VAL2,", [r",11,24,", r",10, 4,", ror #31]!")\
>  	TEST_P(  "ldr"byte"	r0, [r",0,  24,", #-2]")			\
> @@ -668,12 +672,12 @@ void kprobe_arm_test_cases(void)
>  
>  	LOAD_STORE("")
>  	TEST_P(   "str	pc, [r",0,0,", #15*4]")
> -	TEST_R(   "str	pc, [sp, r",2,15*4,"]")
> +	TEST_UNSUPPORTED(   "str	pc, [sp, r2]")
>  	TEST_BF(  "ldr	pc, [sp, #15*4]")
>  	TEST_BF_R("ldr	pc, [sp, r",2,15*4,"]")
>  
>  	TEST_P(   "str	sp, [r",0,0,", #13*4]")
> -	TEST_R(   "str	sp, [sp, r",2,13*4,"]")
> +	TEST_UNSUPPORTED(   "str	sp, [sp, r2]")
>  	TEST_BF(  "ldr	sp, [sp, #13*4]")
>  	TEST_BF_R("ldr	sp, [sp, r",2,13*4,"]")
>  
> diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
> index d7bee4b..30498436 100644
> --- a/arch/arm/kernel/kprobes.c
> +++ b/arch/arm/kernel/kprobes.c
> @@ -115,6 +115,15 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
>  		break;
>  	}
>  
> +	/*
> +	 * Never instrument insn like 'str r0, [sp, +/-r1]'. Also, insn likes
> +	 * 'str r0, [sp, #-68]' should also be prohibited.
> +	 * See __und_svc.
> +	 */
> +	if ((p->ainsn.stack_space < 0) ||
> +			(p->ainsn.stack_space > MAX_STACK_SIZE))
> +		return -EINVAL;
> +
>  	return 0;
>  }
>  

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] ARM: kprobes: Add test cases for stack consuming instructions
  2014-11-21 17:59   ` Jon Medhurst (Tixy)
@ 2014-11-21 20:11     ` Jon Medhurst (Tixy)
  0 siblings, 0 replies; 7+ messages in thread
From: Jon Medhurst (Tixy) @ 2014-11-21 20:11 UTC (permalink / raw)
  To: linux-arm-kernel

From: Jon Medhurst <tixy@linaro.org>

These have extra 'checker' functions associated with them so
lets make sure those get covered by testing.

Signed-off-by: Jon Medhurst <tixy@linaro.org>
---

Wang Nan, as I promised in my reply to your patch, here are the
additional test cases for the stack store instructions. Would be good to
add this patch to the series. Let me know if you think I've missed
something...


 arch/arm/kernel/kprobes-test-arm.c   | 17 +++++++++++++++--
 arch/arm/kernel/kprobes-test-thumb.c | 12 ++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
index d8ad5bb..7aa96b6 100644
--- a/arch/arm/kernel/kprobes-test-arm.c
+++ b/arch/arm/kernel/kprobes-test-arm.c
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <asm/system_info.h>
 #include <asm/opcodes.h>
+#include <asm/probes.h>
 
 #include "kprobes-test.h"
 
@@ -478,6 +479,7 @@ void kprobe_arm_test_cases(void)
 	TEST_RPR(  "strh	r",0, VAL1,", [r",1, 48,", -r",2, 24,"]")
 	TEST_RPR(  "streqh	r",14,VAL2,", [r",11,0, ", r",12, 48,"]")
 	TEST_UNSUPPORTED(  "streqh	r14, [r13, r12]")
+	TEST_UNSUPPORTED(  "streqh	r14, [r12, r13]")
 	TEST_RPR(  "strh	r",1, VAL1,", [r",2, 24,", r",3,  48,"]!")
 	TEST_RPR(  "strneh	r",12,VAL2,", [r",11,48,", -r",10,24,"]!")
 	TEST_RPR(  "strh	r",2, VAL1,", [r",3, 24,"], r",4, 48,"")
@@ -502,6 +504,9 @@ void kprobe_arm_test_cases(void)
 	TEST_RP(   "strplh	r",12,VAL2,", [r",11,24,", #-4]!")
 	TEST_RP(   "strh	r",2, VAL1,", [r",3, 24,"], #48")
 	TEST_RP(   "strh	r",10,VAL2,", [r",9, 64,"], #-48")
+	TEST_RP(   "strh	r",3, VAL1,", [r",13,TEST_MEMORY_SIZE,", #-"__stringify(MAX_STACK_SIZE)"]!")
+	TEST_UNSUPPORTED("strh r3, [r13, #-"__stringify(MAX_STACK_SIZE)"-8]!")
+	TEST_RP(   "strh	r",4, VAL1,", [r",14,TEST_MEMORY_SIZE,", #-"__stringify(MAX_STACK_SIZE)"-8]!")
 	TEST_UNSUPPORTED(__inst_arm(0xe1efc3b0) "	@ strh r12, [pc, #48]!")
 	TEST_UNSUPPORTED(__inst_arm(0xe0c9f3b0) "	@ strh pc, [r9], #48")
 
@@ -568,6 +573,7 @@ void kprobe_arm_test_cases(void)
 	TEST_RPR(  "strd	r",0, VAL1,", [r",1, 48,", -r",2,24,"]")
 	TEST_RPR(  "strccd	r",8, VAL2,", [r",11,0, ", r",12,48,"]")
 	TEST_UNSUPPORTED(  "strccd r8, [r13, r12]")
+	TEST_UNSUPPORTED(  "strccd r8, [r12, r13]")
 	TEST_RPR(  "strd	r",4, VAL1,", [r",2, 24,", r",3, 48,"]!")
 	TEST_RPR(  "strcsd	r",12,VAL2,", [r",11,48,", -r",10,24,"]!")
 	TEST_RPR(  "strd	r",2, VAL1,", [r",5, 24,"], r",4,48,"")
@@ -591,6 +597,9 @@ void kprobe_arm_test_cases(void)
 	TEST_RP(   "strvcd	r",12,VAL2,", [r",11,24,", #-16]!")
 	TEST_RP(   "strd	r",2, VAL1,", [r",4, 24,"], #48")
 	TEST_RP(   "strd	r",10,VAL2,", [r",9, 64,"], #-48")
+	TEST_RP(   "strd	r",6, VAL1,", [r",13,TEST_MEMORY_SIZE,", #-"__stringify(MAX_STACK_SIZE)"]!")
+	TEST_UNSUPPORTED("strd r6, [r13, #-"__stringify(MAX_STACK_SIZE)"-8]!")
+	TEST_RP(   "strd	r",4, VAL1,", [r",12,TEST_MEMORY_SIZE,", #-"__stringify(MAX_STACK_SIZE)"-8]!")
 	TEST_UNSUPPORTED(__inst_arm(0xe1efc3f0) "	@ strd r12, [pc, #48]!")
 
 	TEST_P(	   "ldrd	r0, [r",0, 24,", #-8]")
@@ -639,16 +648,20 @@ void kprobe_arm_test_cases(void)
 	TEST_RP( "str"byte"	r",12,VAL2,", [r",11,24,", #-4]!")		\
 	TEST_RP( "str"byte"	r",2, VAL1,", [r",3, 24,"], #48")		\
 	TEST_RP( "str"byte"	r",10,VAL2,", [r",9, 64,"], #-48")		\
+	TEST_RP( "str"byte"	r",3, VAL1,", [r",13,TEST_MEMORY_SIZE,", #-"__stringify(MAX_STACK_SIZE)"]!") \
+	TEST_UNSUPPORTED("str"byte" r3, [r13, #-"__stringify(MAX_STACK_SIZE)"-8]!")				\
+	TEST_RP( "str"byte"	r",4, VAL1,", [r",10,TEST_MEMORY_SIZE,", #-"__stringify(MAX_STACK_SIZE)"-8]!") \
 	TEST_RPR("str"byte"	r",0, VAL1,", [r",1, 48,", -r",2, 24,"]")	\
 	TEST_RPR("str"byte"	r",14,VAL2,", [r",11,0, ", r",12, 48,"]")	\
-	TEST_UNSUPPORTED("str"byte" r14, [r13, r12]")	\
+	TEST_UNSUPPORTED("str"byte" r14, [r13, r12]")				\
+	TEST_UNSUPPORTED("str"byte" r14, [r12, r13]")				\
 	TEST_RPR("str"byte"	r",1, VAL1,", [r",2, 24,", r",3,  48,"]!")	\
 	TEST_RPR("str"byte"	r",12,VAL2,", [r",11,48,", -r",10,24,"]!")	\
 	TEST_RPR("str"byte"	r",2, VAL1,", [r",3, 24,"], r",4, 48,"")	\
 	TEST_RPR("str"byte"	r",10,VAL2,", [r",9, 48,"], -r",11,24,"")	\
 	TEST_RPR("str"byte"	r",0, VAL1,", [r",1, 24,", r",2,  32,", asl #1]")\
 	TEST_RPR("str"byte"	r",14,VAL2,", [r",11,0, ", r",12, 32,", lsr #2]")\
-	TEST_UNSUPPORTED("str"byte"	r14, [r13, r12, lsr #2]")\
+	TEST_UNSUPPORTED("str"byte"	r14, [r13, r12, lsr #2]")		\
 	TEST_RPR("str"byte"	r",1, VAL1,", [r",2, 24,", r",3,  32,", asr #3]!")\
 	TEST_RPR("str"byte"	r",12,VAL2,", [r",11,24,", r",10, 4,", ror #31]!")\
 	TEST_P(  "ldr"byte"	r0, [r",0,  24,", #-2]")			\
diff --git a/arch/arm/kernel/kprobes-test-thumb.c b/arch/arm/kernel/kprobes-test-thumb.c
index 844dd10..45b19d2 100644
--- a/arch/arm/kernel/kprobes-test-thumb.c
+++ b/arch/arm/kernel/kprobes-test-thumb.c
@@ -11,6 +11,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <asm/opcodes.h>
+#include <asm/probes.h>
 
 #include "kprobes-test.h"
 
@@ -416,6 +417,9 @@ void kprobe_thumb32_test_cases(void)
 	TEST_RR( "strd	r",14,VAL2,", r",12,VAL1,", [sp, #16]!")
 	TEST_RRP("strd	r",1, VAL1,", r",0, VAL2,", [r",7, 24,"], #16")
 	TEST_RR( "strd	r",7, VAL2,", r",8, VAL1,", [sp], #-16")
+	TEST_RRP("strd	r",6, VAL1,", r",7, VAL2,", [r",13, TEST_MEMORY_SIZE,", #-"__stringify(MAX_STACK_SIZE)"]!")
+	TEST_UNSUPPORTED("strd r6, r7, [r13, #-"__stringify(MAX_STACK_SIZE)"-8]!")
+	TEST_RRP("strd	r",4, VAL1,", r",5, VAL2,", [r",14, TEST_MEMORY_SIZE,", #-"__stringify(MAX_STACK_SIZE)"-8]!")
 	TEST_UNSUPPORTED(__inst_thumb32(0xe9efec04) "	@ strd	r14, r12, [pc, #16]!")
 	TEST_UNSUPPORTED(__inst_thumb32(0xe8efec04) "	@ strd	r14, r12, [pc], #16")
 
@@ -821,14 +825,22 @@ CONDITION_INSTRUCTIONS(22,
 	TEST_RP( "str"size"	r",14,VAL2,", [r",1, 256,  ", #-128]!")		\
 	TEST_RPR("str"size".w	r",0, VAL1,", [r",1, 0,", r",2, 4,"]")		\
 	TEST_RPR("str"size"	r",14,VAL2,", [r",10,0,", r",11,4,", lsl #1]")	\
+	TEST_UNSUPPORTED("str"size"	r0, [r13, r1]")				\
 	TEST_R(  "str"size".w	r",7, VAL1,", [sp, #24]")			\
 	TEST_RP( "str"size".w	r",0, VAL2,", [r",0,0, "]")			\
+	TEST_RP( "str"size"	r",6, VAL1,", [r",13, TEST_MEMORY_SIZE,", #-"__stringify(MAX_STACK_SIZE)"]!") \
+	TEST_UNSUPPORTED("str"size"	r6, [r13, #-"__stringify(MAX_STACK_SIZE)"-8]!")			\
+	TEST_RP( "str"size"	r",4, VAL2,", [r",12, TEST_MEMORY_SIZE,", #-"__stringify(MAX_STACK_SIZE)"-8]!") \
 	TEST_UNSUPPORTED("str"size"t	r0, [r1, #4]")
 
 	SINGLE_STORE("b")
 	SINGLE_STORE("h")
 	SINGLE_STORE("")
 
+	TEST_UNSUPPORTED(__inst_thumb32(0xf801000d) "	@ strb	r0, [r1, r13]")	
+	TEST_UNSUPPORTED(__inst_thumb32(0xf821000d) "	@ strh	r0, [r1, r13]")	
+	TEST_UNSUPPORTED(__inst_thumb32(0xf841000d) "	@ str	r0, [r1, r13]")	
+
 	TEST("str	sp, [sp]")
 	TEST_UNSUPPORTED(__inst_thumb32(0xf8cfe000) "	@ str	r14, [pc]")
 	TEST_UNSUPPORTED(__inst_thumb32(0xf8cef000) "	@ str	pc, [r14]")
-- 
2.1.3

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-11-21 20:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-21  6:35 [PATCH v3 0/3] ARM: kprobes: introduces instruction checker Wang Nan
2014-11-21  6:35 ` [PATCH v3 1/3] ARM: kprobes: introduces checker Wang Nan
2014-11-21  6:35 ` [PATCH v3 2/3] ARM: kprobes: collects stack consumption for store instructions Wang Nan
2014-11-21 17:18   ` Jon Medhurst (Tixy)
2014-11-21  6:35 ` [PATCH v3 3/3] ARM: kprobes: disallow probing stack consuming instructions Wang Nan
2014-11-21 17:59   ` Jon Medhurst (Tixy)
2014-11-21 20:11     ` [PATCH] ARM: kprobes: Add test cases for " Jon Medhurst (Tixy)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).