* [PATCH 1/4] ARM: kprobes: seprates load and store actions
2014-10-25 6:42 [PATCH 0/4] ARM: kprobes: introduces instruction checker Wang Nan
@ 2014-10-25 6:42 ` Wang Nan
2014-10-25 6:42 ` [PATCH 2/4] ARM: kprobes: introduces checker Wang Nan
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Wang Nan @ 2014-10-25 6:42 UTC (permalink / raw)
To: linux-arm-kernel
This patch seprates actions for load and store. Following patches will
check store instructions for more informations.
Coverage test complains register test coverage missing after this
sepration. This patch introduces one testcase for it.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
arch/arm/kernel/kprobes-arm.c | 6 ++-
arch/arm/kernel/kprobes-test-arm.c | 1 +
arch/arm/kernel/kprobes-test-thumb.c | 13 ++++++
arch/arm/kernel/kprobes-thumb.c | 18 ++++++---
arch/arm/kernel/probes-arm.c | 11 +++--
arch/arm/kernel/probes-arm.h | 6 ++-
arch/arm/kernel/probes-thumb.c | 78 +++++++++++++++++++++++++++---------
arch/arm/kernel/probes-thumb.h | 18 ++++++---
arch/arm/kernel/uprobes-arm.c | 6 ++-
9 files changed, 116 insertions(+), 41 deletions(-)
diff --git a/arch/arm/kernel/kprobes-arm.c b/arch/arm/kernel/kprobes-arm.c
index ac300c6..d5a0ad5 100644
--- a/arch/arm/kernel/kprobes-arm.c
+++ b/arch/arm/kernel/kprobes-arm.c
@@ -315,7 +315,8 @@ const union decode_action kprobes_arm_actions[NUM_PROBES_ARM_ACTIONS] = {
[PROBES_MUL1] = {.handler = emulate_rdlo12rdhi16rn0rm8_rwflags_nopc},
[PROBES_MUL2] = {.handler = emulate_rd16rn12rm0rs8_rwflags_nopc},
[PROBES_SWP] = {.handler = emulate_rd12rn16rm0_rwflags_nopc},
- [PROBES_LDRSTRD] = {.handler = emulate_ldrdstrd},
+ [PROBES_LDRD] = {.handler = emulate_ldrdstrd},
+ [PROBES_STRD] = {.handler = emulate_ldrdstrd},
[PROBES_LOAD_EXTRA] = {.handler = emulate_ldr},
[PROBES_LOAD] = {.handler = emulate_ldr},
[PROBES_STORE_EXTRA] = {.handler = emulate_str},
@@ -339,5 +340,6 @@ const union decode_action kprobes_arm_actions[NUM_PROBES_ARM_ACTIONS] = {
[PROBES_MUL_ADD] = {.handler = emulate_rd16rn12rm0rs8_rwflags_nopc},
[PROBES_BITFIELD] = {.handler = emulate_rd12rm0_noflags_nopc},
[PROBES_BRANCH] = {.handler = simulate_bbl},
- [PROBES_LDMSTM] = {.decoder = kprobe_decode_ldmstm}
+ [PROBES_LDM] = {.decoder = kprobe_decode_ldmstm},
+ [PROBES_STM] = {.decoder = kprobe_decode_ldmstm},
};
diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
index cb14242..264c064 100644
--- a/arch/arm/kernel/kprobes-test-arm.c
+++ b/arch/arm/kernel/kprobes-test-arm.c
@@ -571,6 +571,7 @@ void kprobe_arm_test_cases(void)
TEST_RPR( "strd r",2, VAL1,", [r",5, 24,"], r",4,48,"")
TEST_RPR( "strd r",10,VAL2,", [r",9, 48,"], -r",7,24,"")
TEST_UNSUPPORTED(__inst_arm(0xe1afc0fa) " @ strd r12, [pc, r10]!")
+ TEST_UNSUPPORTED(__inst_arm(0xe1aac0ff) " @ strd r12, [r10, pc]!")
TEST_PR( "ldrd r0, [r",0, 48,", -r",2,24,"]")
TEST_PR( "ldrmid r8, [r",13,0, ", r",12,48,"]")
diff --git a/arch/arm/kernel/kprobes-test-thumb.c b/arch/arm/kernel/kprobes-test-thumb.c
index 844dd10..ed863c4 100644
--- a/arch/arm/kernel/kprobes-test-thumb.c
+++ b/arch/arm/kernel/kprobes-test-thumb.c
@@ -410,6 +410,13 @@ void kprobe_thumb32_test_cases(void)
TEST_UNSUPPORTED(__inst_thumb32(0xe9d47d00) " @ ldrd r7, sp, [r4]")
TEST_UNSUPPORTED(__inst_thumb32(0xe9d47f00) " @ ldrd r7, pc, [r4]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xe9efec04) " @ strd r14, r12, [pc, #16]!")
+ TEST_UNSUPPORTED(__inst_thumb32(0xe8efec04) " @ strd r14, r12, [pc], #16")
+ TEST_UNSUPPORTED(__inst_thumb32(0xe9c4d800) " @ strd sp, r8, [r4]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xe9c4f800) " @ strd pc, r8, [r4]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xe9c47d00) " @ strd r7, sp, [r4]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xe9c47f00) " @ strd r7, pc, [r4]")
+
TEST_RRP("strd r",0, VAL1,", r",1, VAL2,", [r",1, 24,", #-16]")
TEST_RR( "strd r",12,VAL2,", r",14,VAL1,", [sp, #16]")
TEST_RRP("strd r",1, VAL1,", r",0, VAL2,", [r",7, 24,", #-16]!")
@@ -832,6 +839,12 @@ CONDITION_INSTRUCTIONS(22,
TEST("str sp, [sp]")
TEST_UNSUPPORTED(__inst_thumb32(0xf8cfe000) " @ str r14, [pc]")
TEST_UNSUPPORTED(__inst_thumb32(0xf8cef000) " @ str pc, [r14]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xf841100f) " @ str r1, [r1, pc]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xf841100d) " @ str r1, [r1, sp]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xf8a1d000) " @ strh sp, [r1]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xf821d002) " @ strh sp, [r1, r2]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xf822100f) " @ strh r1, [r2, pc]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xf822100d) " @ strh r1, [r2, sp]")
TEST_GROUP("Advanced SIMD element or structure load/store instructions")
diff --git a/arch/arm/kernel/kprobes-thumb.c b/arch/arm/kernel/kprobes-thumb.c
index 9495d7f..e0ade52 100644
--- a/arch/arm/kernel/kprobes-thumb.c
+++ b/arch/arm/kernel/kprobes-thumb.c
@@ -626,17 +626,22 @@ const union decode_action kprobes_t16_actions[NUM_PROBES_T16_ACTIONS] = {
[PROBES_T16_LDR_LIT] = {.handler = t16_simulate_ldr_literal},
[PROBES_T16_BLX] = {.handler = t16_simulate_bxblx},
[PROBES_T16_HIREGOPS] = {.decoder = t16_decode_hiregs},
- [PROBES_T16_LDRHSTRH] = {.handler = t16_emulate_loregs_rwflags},
- [PROBES_T16_LDRSTR] = {.handler = t16_simulate_ldrstr_sp_relative},
+ [PROBES_T16_LDRH] = {.handler = t16_emulate_loregs_rwflags},
+ [PROBES_T16_STRH] = {.handler = t16_emulate_loregs_rwflags},
+ [PROBES_T16_LDR] = {.handler = t16_simulate_ldrstr_sp_relative},
+ [PROBES_T16_STR] = {.handler = t16_simulate_ldrstr_sp_relative},
[PROBES_T16_ADR] = {.handler = t16_simulate_reladr},
- [PROBES_T16_LDMSTM] = {.handler = t16_emulate_loregs_rwflags},
+ [PROBES_T16_LDM] = {.handler = t16_emulate_loregs_rwflags},
+ [PROBES_T16_STM] = {.handler = t16_emulate_loregs_rwflags},
[PROBES_T16_BRANCH_COND] = {.decoder = t16_decode_cond_branch},
[PROBES_T16_BRANCH] = {.handler = t16_simulate_branch},
};
const union decode_action kprobes_t32_actions[NUM_PROBES_T32_ACTIONS] = {
- [PROBES_T32_LDMSTM] = {.decoder = t32_decode_ldmstm},
- [PROBES_T32_LDRDSTRD] = {.handler = t32_emulate_ldrdstrd},
+ [PROBES_T32_LDM] = {.decoder = t32_decode_ldmstm},
+ [PROBES_T32_STM] = {.decoder = t32_decode_ldmstm},
+ [PROBES_T32_LDRD] = {.handler = t32_emulate_ldrdstrd},
+ [PROBES_T32_STRD] = {.handler = t32_emulate_ldrdstrd},
[PROBES_T32_TABLE_BRANCH] = {.handler = t32_simulate_table_branch},
[PROBES_T32_TST] = {.handler = t32_emulate_rd8rn16rm0_rwflags},
[PROBES_T32_MOV] = {.handler = t32_emulate_rd8rn16rm0_rwflags},
@@ -655,7 +660,8 @@ const union decode_action kprobes_t32_actions[NUM_PROBES_T32_ACTIONS] = {
[PROBES_T32_BRANCH] = {.handler = t32_simulate_branch},
[PROBES_T32_PLDI] = {.handler = probes_simulate_nop},
[PROBES_T32_LDR_LIT] = {.handler = t32_simulate_ldr_literal},
- [PROBES_T32_LDRSTR] = {.handler = t32_emulate_ldrstr},
+ [PROBES_T32_LDR] = {.handler = t32_emulate_ldrstr},
+ [PROBES_T32_STR] = {.handler = t32_emulate_ldrstr},
[PROBES_T32_SIGN_EXTEND] = {.handler = t32_emulate_rd8rn16rm0_rwflags},
[PROBES_T32_MEDIA] = {.handler = t32_emulate_rd8rn16rm0_rwflags},
[PROBES_T32_REVERSE] = {.handler = t32_emulate_rd8rn16_noflags},
diff --git a/arch/arm/kernel/probes-arm.c b/arch/arm/kernel/probes-arm.c
index 8eaef81..a17899f 100644
--- a/arch/arm/kernel/probes-arm.c
+++ b/arch/arm/kernel/probes-arm.c
@@ -270,13 +270,17 @@ static const union decode_item arm_cccc_000x_____1xx1_table[] = {
DECODE_REJECT (0x0e10e0d0, 0x0000e0d0),
/* LDRD (register) cccc 000x x0x0 xxxx xxxx xxxx 1101 xxxx */
+ DECODE_EMULATEX (0x0e5000f0, 0x000000d0, PROBES_LDRD,
+ REGS(NOPCWB, NOPCX, 0, 0, NOPC)),
/* STRD (register) cccc 000x x0x0 xxxx xxxx xxxx 1111 xxxx */
- DECODE_EMULATEX (0x0e5000d0, 0x000000d0, PROBES_LDRSTRD,
+ DECODE_EMULATEX (0x0e5000f0, 0x000000f0, PROBES_STRD,
REGS(NOPCWB, NOPCX, 0, 0, NOPC)),
/* LDRD (immediate) cccc 000x x1x0 xxxx xxxx xxxx 1101 xxxx */
+ DECODE_EMULATEX (0x0e5000f0, 0x004000d0, PROBES_LDRD,
+ REGS(NOPCWB, NOPCX, 0, 0, 0)),
/* STRD (immediate) cccc 000x x1x0 xxxx xxxx xxxx 1111 xxxx */
- DECODE_EMULATEX (0x0e5000d0, 0x004000d0, PROBES_LDRSTRD,
+ DECODE_EMULATEX (0x0e5000f0, 0x004000f0, PROBES_STRD,
REGS(NOPCWB, NOPCX, 0, 0, 0)),
/* STRH (register) cccc 000x x0x0 xxxx xxxx xxxx 1011 xxxx */
@@ -601,8 +605,9 @@ static const union decode_item arm_cccc_100x_table[] = {
/* Block data transfer instructions */
/* LDM cccc 100x x0x1 xxxx xxxx xxxx xxxx xxxx */
+ DECODE_CUSTOM (0x0e500000, 0x08100000, PROBES_LDM),
/* STM cccc 100x x0x0 xxxx xxxx xxxx xxxx xxxx */
- DECODE_CUSTOM (0x0e400000, 0x08000000, PROBES_LDMSTM),
+ DECODE_CUSTOM (0x0e500000, 0x08000000, PROBES_STM),
/* STM (user registers) cccc 100x x1x0 xxxx xxxx xxxx xxxx xxxx */
/* LDM (user registers) cccc 100x x1x1 xxxx 0xxx xxxx xxxx xxxx */
diff --git a/arch/arm/kernel/probes-arm.h b/arch/arm/kernel/probes-arm.h
index ace6572..74c7f97 100644
--- a/arch/arm/kernel/probes-arm.h
+++ b/arch/arm/kernel/probes-arm.h
@@ -28,7 +28,8 @@ enum probes_arm_action {
PROBES_MUL1,
PROBES_MUL2,
PROBES_SWP,
- PROBES_LDRSTRD,
+ PROBES_LDRD,
+ PROBES_STRD,
PROBES_LOAD,
PROBES_STORE,
PROBES_LOAD_EXTRA,
@@ -49,7 +50,8 @@ enum probes_arm_action {
PROBES_MUL_ADD,
PROBES_BITFIELD,
PROBES_BRANCH,
- PROBES_LDMSTM,
+ PROBES_LDM,
+ PROBES_STM,
NUM_PROBES_ARM_ACTIONS
};
diff --git a/arch/arm/kernel/probes-thumb.c b/arch/arm/kernel/probes-thumb.c
index 4131351..ac27d63 100644
--- a/arch/arm/kernel/probes-thumb.c
+++ b/arch/arm/kernel/probes-thumb.c
@@ -37,10 +37,11 @@ static const union decode_item t32_table_1110_100x_x0xx[] = {
DECODE_REJECT (0xfe402000, 0xe8002000),
/* STMIA 1110 1000 10x0 xxxx xxxx xxxx xxxx xxxx */
- /* LDMIA 1110 1000 10x1 xxxx xxxx xxxx xxxx xxxx */
/* STMDB 1110 1001 00x0 xxxx xxxx xxxx xxxx xxxx */
+ DECODE_CUSTOM (0xfe500000, 0xe8000000, PROBES_T32_STM),
+ /* LDMIA 1110 1000 10x1 xxxx xxxx xxxx xxxx xxxx */
/* LDMDB 1110 1001 00x1 xxxx xxxx xxxx xxxx xxxx */
- DECODE_CUSTOM (0xfe400000, 0xe8000000, PROBES_T32_LDMSTM),
+ DECODE_CUSTOM (0xfe500000, 0xe8100000, PROBES_T32_LDM),
DECODE_END
};
@@ -49,11 +50,15 @@ static const union decode_item t32_table_1110_100x_x1xx[] = {
/* Load/store dual, load/store exclusive, table branch */
/* STRD (immediate) 1110 1000 x110 xxxx xxxx xxxx xxxx xxxx */
- /* LDRD (immediate) 1110 1000 x111 xxxx xxxx xxxx xxxx xxxx */
- DECODE_OR (0xff600000, 0xe8600000),
+ DECODE_OR (0xff700000, 0xe8600000),
/* STRD (immediate) 1110 1001 x1x0 xxxx xxxx xxxx xxxx xxxx */
+ DECODE_EMULATEX (0xff500000, 0xe9400000, PROBES_T32_STRD,
+ REGS(NOPCWB, NOSPPC, NOSPPC, 0, 0)),
+
+ /* LDRD (immediate) 1110 1000 x111 xxxx xxxx xxxx xxxx xxxx */
+ DECODE_OR (0xff700000, 0xe8700000),
/* LDRD (immediate) 1110 1001 x1x1 xxxx xxxx xxxx xxxx xxxx */
- DECODE_EMULATEX (0xff400000, 0xe9400000, PROBES_T32_LDRDSTRD,
+ DECODE_EMULATEX (0xff500000, 0xe9500000, PROBES_T32_LDRD,
REGS(NOPCWB, NOSPPC, NOSPPC, 0, 0)),
/* TBB 1110 1000 1101 xxxx xxxx xxxx 0000 xxxx */
@@ -340,16 +345,29 @@ static const union decode_item t32_table_1111_100x[] = {
REGS(PC, ANY, 0, 0, 0)),
/* STR (immediate) 1111 1000 0100 xxxx xxxx 1xxx xxxx xxxx */
- /* LDR (immediate) 1111 1000 0101 xxxx xxxx 1xxx xxxx xxxx */
- DECODE_OR (0xffe00800, 0xf8400800),
+ DECODE_OR (0xfff00800, 0xf8400800),
/* STR (immediate) 1111 1000 1100 xxxx xxxx xxxx xxxx xxxx */
+ /*
+ * Reject PC for Rt. PC has already rejected by
+ * 0xff1f0000, 0xf80f0000 and 0xff10f000, 0xf800f000.
+ * Suppress complain on coverage in test code.
+ */
+ DECODE_EMULATEX (0xfff00000, 0xf8c00000, PROBES_T32_STR,
+ REGS(NOPCX, NOPCX, 0, 0, 0)),
+ /* LDR (immediate) 1111 1000 0101 xxxx xxxx 1xxx xxxx xxxx */
+ DECODE_OR (0xfff00800, 0xf8500800),
/* LDR (immediate) 1111 1000 1101 xxxx xxxx xxxx xxxx xxxx */
- DECODE_EMULATEX (0xffe00000, 0xf8c00000, PROBES_T32_LDRSTR,
+ DECODE_EMULATEX (0xfff00000, 0xf8d00000, PROBES_T32_LDR,
REGS(NOPCX, ANY, 0, 0, 0)),
-
/* STR (register) 1111 1000 0100 xxxx xxxx 0000 00xx xxxx */
+ /*
+ * Rt == PC and Rn == PC have already been rejected by
+ * 0xff1f0000, 0xf80f0000 and 0xff10f000, 0xf800f000
+ */
+ DECODE_EMULATEX (0xfff00fc0, 0xf8400000, PROBES_T32_STR,
+ REGS(NOPCX, NOPCX, 0, 0, NOSPPC)),
/* LDR (register) 1111 1000 0101 xxxx xxxx 0000 00xx xxxx */
- DECODE_EMULATEX (0xffe00fc0, 0xf8400000, PROBES_T32_LDRSTR,
+ DECODE_EMULATEX (0xfff00fc0, 0xf8500000, PROBES_T32_LDR,
REGS(NOPCX, ANY, 0, 0, NOSPPC)),
/* LDRB (literal) 1111 1000 x001 1111 xxxx xxxx xxxx xxxx */
@@ -361,27 +379,35 @@ static const union decode_item t32_table_1111_100x[] = {
/* STRB (immediate) 1111 1000 0000 xxxx xxxx 1xxx xxxx xxxx */
/* STRH (immediate) 1111 1000 0010 xxxx xxxx 1xxx xxxx xxxx */
+ DECODE_OR (0xffd00800, 0xf8000800),
+ /* STRB (immediate) 1111 1000 1000 xxxx xxxx xxxx xxxx xxxx */
+ /* STRH (immediate) 1111 1000 1010 xxxx xxxx xxxx xxxx xxxx */
+ DECODE_EMULATEX (0xffd00000, 0xf8800000, PROBES_T32_STR,
+ REGS(NOPCX, NOSPPCX, 0, 0, 0)),
+
/* LDRB (immediate) 1111 1000 0001 xxxx xxxx 1xxx xxxx xxxx */
/* LDRSB (immediate) 1111 1001 0001 xxxx xxxx 1xxx xxxx xxxx */
/* LDRH (immediate) 1111 1000 0011 xxxx xxxx 1xxx xxxx xxxx */
/* LDRSH (immediate) 1111 1001 0011 xxxx xxxx 1xxx xxxx xxxx */
- DECODE_OR (0xfec00800, 0xf8000800),
- /* STRB (immediate) 1111 1000 1000 xxxx xxxx xxxx xxxx xxxx */
- /* STRH (immediate) 1111 1000 1010 xxxx xxxx xxxx xxxx xxxx */
+ DECODE_OR (0xfed00800, 0xf8100800),
+
/* LDRB (immediate) 1111 1000 1001 xxxx xxxx xxxx xxxx xxxx */
/* LDRSB (immediate) 1111 1001 1001 xxxx xxxx xxxx xxxx xxxx */
/* LDRH (immediate) 1111 1000 1011 xxxx xxxx xxxx xxxx xxxx */
/* LDRSH (immediate) 1111 1001 1011 xxxx xxxx xxxx xxxx xxxx */
- DECODE_EMULATEX (0xfec00000, 0xf8800000, PROBES_T32_LDRSTR,
+ DECODE_EMULATEX (0xfed00000, 0xf8900000, PROBES_T32_LDR,
REGS(NOPCX, NOSPPCX, 0, 0, 0)),
/* STRB (register) 1111 1000 0000 xxxx xxxx 0000 00xx xxxx */
/* STRH (register) 1111 1000 0010 xxxx xxxx 0000 00xx xxxx */
+ DECODE_EMULATEX (0xffd00fc0, 0xf8000000, PROBES_T32_STR,
+ REGS(NOPCX, NOSPPCX, 0, 0, NOSPPC)),
+
/* LDRB (register) 1111 1000 0001 xxxx xxxx 0000 00xx xxxx */
/* LDRSB (register) 1111 1001 0001 xxxx xxxx 0000 00xx xxxx */
/* LDRH (register) 1111 1000 0011 xxxx xxxx 0000 00xx xxxx */
/* LDRSH (register) 1111 1001 0011 xxxx xxxx 0000 00xx xxxx */
- DECODE_EMULATEX (0xfe800fc0, 0xf8000000, PROBES_T32_LDRSTR,
+ DECODE_EMULATEX (0xfed00fc0, 0xf8100000, PROBES_T32_LDR,
REGS(NOPCX, NOSPPCX, 0, 0, NOSPPC)),
/* Other unallocated instructions... */
@@ -778,23 +804,34 @@ const union decode_item probes_decode_thumb16_table[] = {
/* STR (register) 0101 000x xxxx xxxx */
/* STRH (register) 0101 001x xxxx xxxx */
+ DECODE_EMULATE (0xfc00, 0x5000, PROBES_T16_STRH),
/* STRB (register) 0101 010x xxxx xxxx */
+ DECODE_EMULATE (0xfe00, 0x5400, PROBES_T16_STRH),
/* LDRSB (register) 0101 011x xxxx xxxx */
+ DECODE_EMULATE (0xfe00, 0x5600, PROBES_T16_LDRH),
+
/* LDR (register) 0101 100x xxxx xxxx */
/* LDRH (register) 0101 101x xxxx xxxx */
/* LDRB (register) 0101 110x xxxx xxxx */
/* LDRSH (register) 0101 111x xxxx xxxx */
+ DECODE_EMULATE (0xf800, 0x5800, PROBES_T16_LDRH),
+
/* STR (immediate, Thumb) 0110 0xxx xxxx xxxx */
- /* LDR (immediate, Thumb) 0110 1xxx xxxx xxxx */
/* STRB (immediate, Thumb) 0111 0xxx xxxx xxxx */
+ DECODE_EMULATE (0xe800, 0x6000, PROBES_T16_STRH),
+
+ /* LDR (immediate, Thumb) 0110 1xxx xxxx xxxx */
/* LDRB (immediate, Thumb) 0111 1xxx xxxx xxxx */
- DECODE_EMULATE (0xc000, 0x4000, PROBES_T16_LDRHSTRH),
+ DECODE_EMULATE (0xe800, 0x6800, PROBES_T16_LDRH),
+
/* STRH (immediate, Thumb) 1000 0xxx xxxx xxxx */
+ DECODE_EMULATE (0xf800, 0x8000, PROBES_T16_STRH),
/* LDRH (immediate, Thumb) 1000 1xxx xxxx xxxx */
- DECODE_EMULATE (0xf000, 0x8000, PROBES_T16_LDRHSTRH),
+ DECODE_EMULATE (0xf800, 0x8800, PROBES_T16_LDRH),
/* STR (immediate, Thumb) 1001 0xxx xxxx xxxx */
+ DECODE_SIMULATE (0xf800, 0x9000, PROBES_T16_STR),
/* LDR (immediate, Thumb) 1001 1xxx xxxx xxxx */
- DECODE_SIMULATE (0xf000, 0x9000, PROBES_T16_LDRSTR),
+ DECODE_SIMULATE (0xf800, 0x9800, PROBES_T16_LDR),
/*
* Generate PC-/SP-relative address
@@ -810,8 +847,9 @@ const union decode_item probes_decode_thumb16_table[] = {
DECODE_TABLE (0xf000, 0xb000, t16_table_1011),
/* STM 1100 0xxx xxxx xxxx */
+ DECODE_EMULATE (0xf800, 0xc000, PROBES_T16_STM),
/* LDM 1100 1xxx xxxx xxxx */
- DECODE_EMULATE (0xf000, 0xc000, PROBES_T16_LDMSTM),
+ DECODE_EMULATE (0xf800, 0xc800, PROBES_T16_LDM),
/*
* Conditional branch, and Supervisor Call
diff --git a/arch/arm/kernel/probes-thumb.h b/arch/arm/kernel/probes-thumb.h
index 7c6f6eb..f36aa21 100644
--- a/arch/arm/kernel/probes-thumb.h
+++ b/arch/arm/kernel/probes-thumb.h
@@ -30,8 +30,10 @@
enum probes_t32_action {
PROBES_T32_EMULATE_NONE,
PROBES_T32_SIMULATE_NOP,
- PROBES_T32_LDMSTM,
- PROBES_T32_LDRDSTRD,
+ PROBES_T32_LDM,
+ PROBES_T32_STM,
+ PROBES_T32_LDRD,
+ PROBES_T32_STRD,
PROBES_T32_TABLE_BRANCH,
PROBES_T32_TST,
PROBES_T32_CMP,
@@ -50,7 +52,8 @@ enum probes_t32_action {
PROBES_T32_BRANCH,
PROBES_T32_PLDI,
PROBES_T32_LDR_LIT,
- PROBES_T32_LDRSTR,
+ PROBES_T32_LDR,
+ PROBES_T32_STR,
PROBES_T32_SIGN_EXTEND,
PROBES_T32_MEDIA,
PROBES_T32_REVERSE,
@@ -75,10 +78,13 @@ enum probes_t16_action {
PROBES_T16_BLX,
PROBES_T16_HIREGOPS,
PROBES_T16_LDR_LIT,
- PROBES_T16_LDRHSTRH,
- PROBES_T16_LDRSTR,
+ PROBES_T16_LDRH,
+ PROBES_T16_STRH,
+ PROBES_T16_LDR,
+ PROBES_T16_STR,
PROBES_T16_ADR,
- PROBES_T16_LDMSTM,
+ PROBES_T16_LDM,
+ PROBES_T16_STM,
PROBES_T16_BRANCH_COND,
PROBES_T16_BRANCH,
NUM_PROBES_T16_ACTIONS
diff --git a/arch/arm/kernel/uprobes-arm.c b/arch/arm/kernel/uprobes-arm.c
index d3b655f..929f9ab 100644
--- a/arch/arm/kernel/uprobes-arm.c
+++ b/arch/arm/kernel/uprobes-arm.c
@@ -207,7 +207,8 @@ const union decode_action uprobes_probes_actions[] = {
[PROBES_MUL1] = {.handler = probes_simulate_nop},
[PROBES_MUL2] = {.handler = probes_simulate_nop},
[PROBES_SWP] = {.handler = probes_simulate_nop},
- [PROBES_LDRSTRD] = {.decoder = decode_pc_ro},
+ [PROBES_LDRD] = {.decoder = decode_pc_ro},
+ [PROBES_STRD] = {.decoder = decode_pc_ro},
[PROBES_LOAD_EXTRA] = {.decoder = decode_pc_ro},
[PROBES_LOAD] = {.decoder = decode_ldr},
[PROBES_STORE_EXTRA] = {.decoder = decode_pc_ro},
@@ -230,5 +231,6 @@ const union decode_action uprobes_probes_actions[] = {
[PROBES_MUL_ADD] = {.handler = probes_simulate_nop},
[PROBES_BITFIELD] = {.handler = probes_simulate_nop},
[PROBES_BRANCH] = {.handler = simulate_bbl},
- [PROBES_LDMSTM] = {.decoder = uprobe_decode_ldmstm}
+ [PROBES_LDM] = {.decoder = uprobe_decode_ldmstm},
+ [PROBES_STM] = {.decoder = uprobe_decode_ldmstm}
};
--
1.8.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/4] ARM: kprobes: introduces checker
2014-10-25 6:42 [PATCH 0/4] ARM: kprobes: introduces instruction checker Wang Nan
2014-10-25 6:42 ` [PATCH 1/4] ARM: kprobes: seprates load and store actions Wang Nan
@ 2014-10-25 6:42 ` Wang Nan
2014-10-25 6:42 ` [PATCH 3/4] ARM: kprobes: collects stack consumption for store instructions Wang Nan
2014-10-25 6:42 ` [PATCH 4/4] ARM: kprobes: disallow probing stack consuming instructions Wang Nan
3 siblings, 0 replies; 7+ messages in thread
From: Wang Nan @ 2014-10-25 6:42 UTC (permalink / raw)
To: linux-arm-kernel
This patch introdces 'checker' to decoding phase, and calls checkers
when instruction decoding. This allows further analysis for specific
instructions.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
arch/arm/kernel/kprobes.c | 2 +-
arch/arm/kernel/kprobes.h | 3 ++-
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 | 51 +++++++++++++++++++++++++++++++++++++++++-
arch/arm/kernel/probes.h | 11 ++++++++-
arch/arm/kernel/uprobes.c | 2 +-
9 files changed, 79 insertions(+), 14 deletions(-)
diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
index 6d64420..3302983 100644
--- a/arch/arm/kernel/kprobes.c
+++ b/arch/arm/kernel/kprobes.c
@@ -90,7 +90,7 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
p->opcode = insn;
p->ainsn.insn = tmp_insn;
- switch ((*decode_insn)(insn, &p->ainsn, true, actions)) {
+ switch ((*decode_insn)(insn, &p->ainsn, true, actions, NULL)) {
case INSN_REJECTED: /* not supported */
return -EINVAL;
diff --git a/arch/arm/kernel/kprobes.h b/arch/arm/kernel/kprobes.h
index 9a2712e..632fe0b 100644
--- a/arch/arm/kernel/kprobes.h
+++ b/arch/arm/kernel/kprobes.h
@@ -36,7 +36,8 @@ 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
diff --git a/arch/arm/kernel/probes-arm.c b/arch/arm/kernel/probes-arm.c
index a17899f..d280e825 100644
--- a/arch/arm/kernel/probes-arm.c
+++ b/arch/arm/kernel/probes-arm.c
@@ -730,10 +730,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 74c7f97..185adaf 100644
--- a/arch/arm/kernel/probes-arm.h
+++ b/arch/arm/kernel/probes-arm.h
@@ -70,6 +70,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 ac27d63..56925e4 100644
--- a/arch/arm/kernel/probes-thumb.c
+++ b/arch/arm/kernel/probes-thumb.c
@@ -901,20 +901,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 f36aa21..2277744 100644
--- a/arch/arm/kernel/probes-thumb.h
+++ b/arch/arm/kernel/probes-thumb.h
@@ -95,9 +95,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..02598da 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,18 +453,36 @@ 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;
+ int action = d->decoder.action;
+
+ err = run_checkers(checkers, action, origin_insn, asi, h);
+ if (err == INSN_REJECTED)
+ return INSN_REJECTED;
return actions[d->decoder.action].decoder(insn, asi, h);
}
case DECODE_TYPE_SIMULATE: {
+ int err;
struct decode_simulate *d = (struct decode_simulate *)h;
+ 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[d->handler.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,
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 3/4] ARM: kprobes: collects stack consumption for store instructions
2014-10-25 6:42 [PATCH 0/4] ARM: kprobes: introduces instruction checker Wang Nan
2014-10-25 6:42 ` [PATCH 1/4] ARM: kprobes: seprates load and store actions Wang Nan
2014-10-25 6:42 ` [PATCH 2/4] ARM: kprobes: introduces checker Wang Nan
@ 2014-10-25 6:42 ` Wang Nan
2014-10-28 16:46 ` Jon Medhurst (Tixy)
2014-10-25 6:42 ` [PATCH 4/4] ARM: kprobes: disallow probing stack consuming instructions Wang Nan
3 siblings, 1 reply; 7+ messages in thread
From: Wang Nan @ 2014-10-25 6:42 UTC (permalink / raw)
To: linux-arm-kernel
This patch use previous introduced checker on store instructions,
record stack consumption informations to arch_probes_insn. With such
information, kprobe opt can decide how much stack needs to be
protected.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
arch/arm/include/asm/probes.h | 1 +
arch/arm/kernel/kprobes.c | 15 +++++++-
arch/arm/kernel/probes-arm.c | 25 +++++++++++++
arch/arm/kernel/probes-arm.h | 1 +
arch/arm/kernel/probes-thumb.c | 80 ++++++++++++++++++++++++++++++++++++++++++
arch/arm/kernel/probes-thumb.h | 2 ++
arch/arm/kernel/probes.c | 64 +++++++++++++++++++++++++++++++++
arch/arm/kernel/probes.h | 13 +++++++
8 files changed, 200 insertions(+), 1 deletion(-)
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/kprobes.c b/arch/arm/kernel/kprobes.c
index 3302983..618531d 100644
--- a/arch/arm/kernel/kprobes.c
+++ b/arch/arm/kernel/kprobes.c
@@ -61,6 +61,16 @@ 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;
+#ifdef CONFIG_THUMB2_KERNEL
+ const struct decode_checker *t32_checkers[] =
+ {t32_stack_checker, NULL};
+ const struct decode_checker *t16_checkers[] =
+ {t16_stack_checker, NULL};
+#else
+ const struct decode_checker *arm_checkers[] =
+ {arm_stack_checker, NULL};
+#endif
if (in_exception_text(addr))
return -EINVAL;
@@ -74,9 +84,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 = t32_checkers;
} else {
decode_insn = thumb16_probes_decode_insn;
actions = kprobes_t16_actions;
+ checkers = t16_checkers;
}
#else /* !CONFIG_THUMB2_KERNEL */
thumb = false;
@@ -85,12 +97,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 = arm_checkers;
#endif
p->opcode = insn;
p->ainsn.insn = tmp_insn;
- switch ((*decode_insn)(insn, &p->ainsn, true, actions, NULL)) {
+ switch ((*decode_insn)(insn, &p->ainsn, true, actions, checkers)) {
case INSN_REJECTED: /* not supported */
return -EINVAL;
diff --git a/arch/arm/kernel/probes-arm.c b/arch/arm/kernel/probes-arm.c
index d280e825..20e95c0 100644
--- a/arch/arm/kernel/probes-arm.c
+++ b/arch/arm/kernel/probes-arm.c
@@ -109,6 +109,31 @@ void __kprobes simulate_mov_ipsp(probes_opcode_t insn,
regs->uregs[12] = regs->uregs[13];
}
+enum probes_insn __kprobes chk_stack_arm_store(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h)
+{
+ int imm = insn & 0xfff;
+ check_insn_stack_regs(insn, asi, h, imm);
+ return INSN_GOOD;
+}
+
+enum probes_insn __kprobes chk_stack_arm_store_extra(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h)
+{
+ int imm = ((insn & 0xf00) >> 4) + (insn & 0xf);
+ check_insn_stack_regs(insn, asi, h, imm);
+ return INSN_GOOD;
+}
+
+const struct decode_checker arm_stack_checker[NUM_PROBES_ARM_ACTIONS] = {
+ [PROBES_STRD] = {.checker = chk_stack_arm_store_extra},
+ [PROBES_STORE_EXTRA] = {.checker = chk_stack_arm_store_extra},
+ [PROBES_STRD] = {.checker = chk_stack_arm_store},
+ [PROBES_STM] = {.checker = chk_stack_stm},
+};
+
/*
* For the instruction masking and comparisons in all the "space_*"
* functions below, Do _not_ rearrange the order of tests unless
diff --git a/arch/arm/kernel/probes-arm.h b/arch/arm/kernel/probes-arm.h
index 185adaf..4d63cf8 100644
--- a/arch/arm/kernel/probes-arm.h
+++ b/arch/arm/kernel/probes-arm.h
@@ -73,4 +73,5 @@ enum probes_insn arm_probes_decode_insn(probes_opcode_t,
const union decode_action *actions,
const struct decode_checker *checkers[]);
+extern const struct decode_checker arm_stack_checker[];
#endif
diff --git a/arch/arm/kernel/probes-thumb.c b/arch/arm/kernel/probes-thumb.c
index 56925e4..8e7c5be 100644
--- a/arch/arm/kernel/probes-thumb.c
+++ b/arch/arm/kernel/probes-thumb.c
@@ -15,6 +15,86 @@
#include "probes.h"
#include "probes-thumb.h"
+enum probes_insn __kprobes chk_stack_t32_strd(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h)
+{
+ int imm = insn & 0xff;
+ check_insn_stack_regs(insn, asi, h, imm);
+ return INSN_GOOD;
+}
+
+/*
+ * Note: This function doesn't process PROBES_T32_STRD.
+ */
+enum probes_insn __kprobes chk_stack_t32_check_str(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h)
+{
+ int rn = -1, rm = -1;
+ u32 regs = h->type_regs.bits >> DECODE_TYPE_BITS;
+ int index, add;
+
+ /* Rn is used in every cases */
+ BUG_ON((regs & 0xf0000) == 0);
+ rn = (insn & 0xf0000) >> 16;
+ if ((regs & 0xf) != 0)
+ rm = insn & 0xf;
+
+ /*
+ * Rn is not SP. Rm can't be sp in any case.
+ * So it is not a stack store.
+ */
+ if (rn != 0xd)
+ return INSN_GOOD;
+
+ /*
+ * For 'str? rx, [sp, ry]', ry can be negative. In addition,
+ * index is true in every cases, so unable to determine stack
+ * consumption.
+ */
+ if (rm != -1) {
+ asi->stack_space = -1;
+ return INSN_GOOD;
+ }
+
+ /*
+ * For 'str? rx, [sp, #+/-<imm>]', if bit 23 is set, index
+ * and add are both set. Else, index and add are determined
+ * by P bit and U bit (bit 10, 9)
+ */
+ if (insn & 0x800000)
+ index = add = 1;
+ else {
+ index = (insn & (1 << 10));
+ add = (insn &(1 << 9));
+ }
+
+ if (!index || add)
+ return INSN_GOOD;
+
+ asi->stack_space = insn & 0xff;
+ return INSN_GOOD;
+}
+
+enum probes_insn __kprobes chk_stack_t16_push(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;
+}
+
+const struct decode_checker t32_stack_checker[NUM_PROBES_T32_ACTIONS] = {
+ [PROBES_T32_STM] = {.checker = chk_stack_stm},
+ [PROBES_T32_STRD] = {.checker = chk_stack_t32_strd},
+ [PROBES_T32_STR] = {.checker = chk_stack_t32_check_str},
+};
+
+const struct decode_checker t16_stack_checker[NUM_PROBES_T16_ACTIONS] = {
+ [PROBES_T16_PUSH] = {.checker = chk_stack_t16_push},
+};
static const union decode_item t32_table_1110_100x_x0xx[] = {
/* Load/store multiple instructions */
diff --git a/arch/arm/kernel/probes-thumb.h b/arch/arm/kernel/probes-thumb.h
index 2277744..a5783d0 100644
--- a/arch/arm/kernel/probes-thumb.h
+++ b/arch/arm/kernel/probes-thumb.h
@@ -102,4 +102,6 @@ thumb32_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
bool emulate, const union decode_action *actions,
const struct decode_checker *checkers[]);
+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 02598da..4ef4087 100644
--- a/arch/arm/kernel/probes.c
+++ b/arch/arm/kernel/probes.c
@@ -188,6 +188,25 @@ void __kprobes probes_emulate_none(probes_opcode_t opcode,
asi->insn_fn();
}
+/* ARM and Thumb can share this checker */
+enum probes_insn __kprobes chk_stack_stm(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h)
+{
+ unsigned int reglist = insn & 0xffff;
+ int ubit = insn & (1 << 23);
+ int pbit = insn & (1 << 24);
+ int rn = (insn >> 16) & 0xf;
+
+ /* This is stmi?, doesn't require extra stack */
+ if (ubit)
+ return INSN_GOOD;
+ /* If pbit == ubit (== 0), this is stmda, one dword is saved */
+ asi->stack_space = (rn == 0xd) ?
+ (hweight32(reglist) - ((!pbit == !ubit) ? 1 : 0)) * 4 : 0;
+ return INSN_GOOD;
+}
+
/*
* Prepare an instruction slot to receive an instruction for emulating.
* This is done by placing a subroutine return after the location where the
@@ -425,6 +444,8 @@ probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
*/
probes_opcode_t origin_insn = insn;
+ asi->stack_space = 0;
+
if (emulate)
insn = prepare_emulated_insn(insn, asi, thumb);
@@ -503,3 +524,46 @@ probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
}
}
}
+
+int __kprobes check_insn_stack_regs(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h,
+ int imm)
+{
+ u32 regs = h->type_regs.bits >> DECODE_TYPE_BITS;
+ int rn = -1, rm = -1, index, add;
+ asi->stack_space = 0;
+
+ if (((regs >> 16) & 0xf) != REG_TYPE_NONE)
+ rn = (insn >> 16) & 0xf;
+
+ if ((regs & 0xf) != REG_TYPE_NONE)
+ rm = insn & 0xf;
+
+ if ((rn != 13) && (rm != 13))
+ return NOT_STACK_STORE;
+
+ index = insn & (1 << 24);
+ add = insn & (1 << 23);
+
+ if (!index)
+ return NOT_STACK_STORE;
+
+ /*
+ * Even if insn is 'str r0, [sp], +<Rm>', Rm may less than 0.
+ * Therefore if both Rn and Rm are registers and !index,
+ * We are unable to determine whether it is a stack store.
+ */
+ if ((rn != -1) && (rm != -1)) {
+ asi->stack_space = -1;
+ return STACK_REG;
+ }
+
+ /* 'str(d/h) r0, [sp], #+/-<imm>' */
+ /* or 'str(d/h) r0, [sp, #+<imm>'] */
+ if (add)
+ return NOT_STACK_STORE;
+
+ asi->stack_space = imm;
+ return STACK_IMM;
+}
diff --git a/arch/arm/kernel/probes.h b/arch/arm/kernel/probes.h
index b4bf1f5..b52629c 100644
--- a/arch/arm/kernel/probes.h
+++ b/arch/arm/kernel/probes.h
@@ -413,4 +413,17 @@ probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
const union decode_action *actions,
const struct decode_checker **checkers);
+enum probes_insn __kprobes chk_stack_stm(probes_opcode_t,
+ struct arch_probes_insn *,
+ const struct decode_header *);
+
+enum {
+ NOT_STACK_STORE,
+ STACK_REG,
+ STACK_IMM,
+};
+int __kprobes check_insn_stack_regs(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h,
+ int imm);
#endif
--
1.8.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/4] ARM: kprobes: collects stack consumption for store instructions
2014-10-25 6:42 ` [PATCH 3/4] ARM: kprobes: collects stack consumption for store instructions Wang Nan
@ 2014-10-28 16:46 ` Jon Medhurst (Tixy)
0 siblings, 0 replies; 7+ messages in thread
From: Jon Medhurst (Tixy) @ 2014-10-28 16:46 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, 2014-10-25 at 14:42 +0800, Wang Nan wrote:
> This patch use previous introduced checker on store instructions,
> record stack consumption informations to arch_probes_insn. With such
> information, kprobe opt can decide how much stack needs to be
> protected.
>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
My comments below are mostly minor nit-picks, but there is one bug and
some code clarity issues.
> ---
> arch/arm/include/asm/probes.h | 1 +
> arch/arm/kernel/kprobes.c | 15 +++++++-
> arch/arm/kernel/probes-arm.c | 25 +++++++++++++
> arch/arm/kernel/probes-arm.h | 1 +
> arch/arm/kernel/probes-thumb.c | 80 ++++++++++++++++++++++++++++++++++++++++++
> arch/arm/kernel/probes-thumb.h | 2 ++
> arch/arm/kernel/probes.c | 64 +++++++++++++++++++++++++++++++++
> arch/arm/kernel/probes.h | 13 +++++++
> 8 files changed, 200 insertions(+), 1 deletion(-)
>
> 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/kprobes.c b/arch/arm/kernel/kprobes.c
> index 3302983..618531d 100644
> --- a/arch/arm/kernel/kprobes.c
> +++ b/arch/arm/kernel/kprobes.c
> @@ -61,6 +61,16 @@ 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;
> +#ifdef CONFIG_THUMB2_KERNEL
> + const struct decode_checker *t32_checkers[] =
> + {t32_stack_checker, NULL};
> + const struct decode_checker *t16_checkers[] =
> + {t16_stack_checker, NULL};
> +#else
> + const struct decode_checker *arm_checkers[] =
> + {arm_stack_checker, NULL};
> +#endif
Wouldn't it be cleaner, i.e. avoid this extra #ifdef, to define the the
kprobe checker tables in kprobes-{arm,thumb}.c and corresponding
headers? That is the same as the action tables are.
> if (in_exception_text(addr))
> return -EINVAL;
> @@ -74,9 +84,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 = t32_checkers;
> } else {
> decode_insn = thumb16_probes_decode_insn;
> actions = kprobes_t16_actions;
> + checkers = t16_checkers;
> }
> #else /* !CONFIG_THUMB2_KERNEL */
> thumb = false;
> @@ -85,12 +97,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 = arm_checkers;
> #endif
>
> p->opcode = insn;
> p->ainsn.insn = tmp_insn;
>
> - switch ((*decode_insn)(insn, &p->ainsn, true, actions, NULL)) {
> + switch ((*decode_insn)(insn, &p->ainsn, true, actions, checkers)) {
> case INSN_REJECTED: /* not supported */
> return -EINVAL;
>
> diff --git a/arch/arm/kernel/probes-arm.c b/arch/arm/kernel/probes-arm.c
> index d280e825..20e95c0 100644
> --- a/arch/arm/kernel/probes-arm.c
> +++ b/arch/arm/kernel/probes-arm.c
> @@ -109,6 +109,31 @@ void __kprobes simulate_mov_ipsp(probes_opcode_t insn,
> regs->uregs[12] = regs->uregs[13];
> }
>
> +enum probes_insn __kprobes chk_stack_arm_store(probes_opcode_t insn,
> + struct arch_probes_insn *asi,
> + const struct decode_header *h)
> +{
> + int imm = insn & 0xfff;
> + check_insn_stack_regs(insn, asi, h, imm);
> + return INSN_GOOD;
> +}
> +
> +enum probes_insn __kprobes chk_stack_arm_store_extra(probes_opcode_t insn,
> + struct arch_probes_insn *asi,
> + const struct decode_header *h)
> +{
> + int imm = ((insn & 0xf00) >> 4) + (insn & 0xf);
> + check_insn_stack_regs(insn, asi, h, imm);
> + return INSN_GOOD;
> +}
> +
> +const struct decode_checker arm_stack_checker[NUM_PROBES_ARM_ACTIONS] = {
> + [PROBES_STRD] = {.checker = chk_stack_arm_store_extra},
> + [PROBES_STORE_EXTRA] = {.checker = chk_stack_arm_store_extra},
> + [PROBES_STRD] = {.checker = chk_stack_arm_store},
The above PROBES_STRD should be PROBES_STORE.
> + [PROBES_STM] = {.checker = chk_stack_stm},
> +};
> +
> /*
> * For the instruction masking and comparisons in all the "space_*"
> * functions below, Do _not_ rearrange the order of tests unless
> diff --git a/arch/arm/kernel/probes-arm.h b/arch/arm/kernel/probes-arm.h
> index 185adaf..4d63cf8 100644
> --- a/arch/arm/kernel/probes-arm.h
> +++ b/arch/arm/kernel/probes-arm.h
> @@ -73,4 +73,5 @@ enum probes_insn arm_probes_decode_insn(probes_opcode_t,
> const union decode_action *actions,
> const struct decode_checker *checkers[]);
>
> +extern const struct decode_checker arm_stack_checker[];
> #endif
> diff --git a/arch/arm/kernel/probes-thumb.c b/arch/arm/kernel/probes-thumb.c
> index 56925e4..8e7c5be 100644
> --- a/arch/arm/kernel/probes-thumb.c
> +++ b/arch/arm/kernel/probes-thumb.c
> @@ -15,6 +15,86 @@
> #include "probes.h"
> #include "probes-thumb.h"
>
> +enum probes_insn __kprobes chk_stack_t32_strd(probes_opcode_t insn,
> + struct arch_probes_insn *asi,
> + const struct decode_header *h)
> +{
> + int imm = insn & 0xff;
> + check_insn_stack_regs(insn, asi, h, imm);
> + return INSN_GOOD;
> +}
> +
> +/*
> + * Note: This function doesn't process PROBES_T32_STRD.
> + */
> +enum probes_insn __kprobes chk_stack_t32_check_str(probes_opcode_t insn,
Should the function name not be chk_stack_t32_str? The use of 'check' in
the name isn't consistent with the other functions and seems redundant
considering that's what the 'chk' at the start means.
> + struct arch_probes_insn *asi,
> + const struct decode_header *h)
> +{
> + int rn = -1, rm = -1;
> + u32 regs = h->type_regs.bits >> DECODE_TYPE_BITS;
> + int index, add;
> +
> + /* Rn is used in every cases */
> + BUG_ON((regs & 0xf0000) == 0);
Best use REG_TYPE_NONE rather than 0
> + rn = (insn & 0xf0000) >> 16;
> + if ((regs & 0xf) != 0)
Again, REG_TYPE_NONE rather than 0
> + rm = insn & 0xf;
> +
> + /*
> + * Rn is not SP. Rm can't be sp in any case.
> + * So it is not a stack store.
> + */
> + if (rn != 0xd)
The latest ARM ARM says "ARMv8-A removes UNPREDICTABLE for R13"
so I think it would be safest to also include a test for rm != 0xd. Even
though at the moment the decode table prevents this situation occurring,
I can imaging it being something that could get overlooked in any later
changes to support ARMv8.
> + return INSN_GOOD;
> +
> + /*
> + * For 'str? rx, [sp, ry]', ry can be negative. In addition,
> + * index is true in every cases, so unable to determine stack
> + * consumption.
> + */
> + if (rm != -1) {
> + asi->stack_space = -1;
> + return INSN_GOOD;
> + }
> +
> + /*
> + * For 'str? rx, [sp, #+/-<imm>]', if bit 23 is set, index
> + * and add are both set. Else, index and add are determined
> + * by P bit and U bit (bit 10, 9)
> + */
> + if (insn & 0x800000)
> + index = add = 1;
> + else {
> + index = (insn & (1 << 10));
> + add = (insn &(1 << 9));
> + }
> +
> + if (!index || add)
> + return INSN_GOOD;
> +
> + asi->stack_space = insn & 0xff;
> + return INSN_GOOD;
> +}
> +
> +enum probes_insn __kprobes chk_stack_t16_push(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;
> +}
> +
> +const struct decode_checker t32_stack_checker[NUM_PROBES_T32_ACTIONS] = {
> + [PROBES_T32_STM] = {.checker = chk_stack_stm},
> + [PROBES_T32_STRD] = {.checker = chk_stack_t32_strd},
> + [PROBES_T32_STR] = {.checker = chk_stack_t32_check_str},
> +};
> +
> +const struct decode_checker t16_stack_checker[NUM_PROBES_T16_ACTIONS] = {
> + [PROBES_T16_PUSH] = {.checker = chk_stack_t16_push},
> +};
>
> static const union decode_item t32_table_1110_100x_x0xx[] = {
> /* Load/store multiple instructions */
> diff --git a/arch/arm/kernel/probes-thumb.h b/arch/arm/kernel/probes-thumb.h
> index 2277744..a5783d0 100644
> --- a/arch/arm/kernel/probes-thumb.h
> +++ b/arch/arm/kernel/probes-thumb.h
> @@ -102,4 +102,6 @@ thumb32_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
> bool emulate, const union decode_action *actions,
> const struct decode_checker *checkers[]);
>
> +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 02598da..4ef4087 100644
> --- a/arch/arm/kernel/probes.c
> +++ b/arch/arm/kernel/probes.c
> @@ -188,6 +188,25 @@ void __kprobes probes_emulate_none(probes_opcode_t opcode,
> asi->insn_fn();
> }
>
> +/* ARM and Thumb can share this checker */
> +enum probes_insn __kprobes chk_stack_stm(probes_opcode_t insn,
> + struct arch_probes_insn *asi,
> + const struct decode_header *h)
> +{
> + unsigned int reglist = insn & 0xffff;
> + int ubit = insn & (1 << 23);
> + int pbit = insn & (1 << 24);
> + int rn = (insn >> 16) & 0xf;
> +
> + /* This is stmi?, doesn't require extra stack */
> + if (ubit)
> + return INSN_GOOD;
> + /* If pbit == ubit (== 0), this is stmda, one dword is saved */
> + asi->stack_space = (rn == 0xd) ?
> + (hweight32(reglist) - ((!pbit == !ubit) ? 1 : 0)) * 4 : 0;
Why test ubit? We (and the compiler) know it's zero. Also, we can avoid
an extra ? operator if we return for rn == 0xd, so how about the above
function more simply written like...
/* If stmi? or not using SP then doesn't require extra stack */
if (ubit || rn != 0xd)
return INSN_GOOD;
/* If pbit == 0, this is stmda, one word is saved */
asi->stack_space = (hweight32(reglist) - !pbit) * 4
Note, I also changed 'dword' to 'word' as this is ARM not X86 :-)
> + return INSN_GOOD;
> +}
> +
> /*
> * Prepare an instruction slot to receive an instruction for emulating.
> * This is done by placing a subroutine return after the location where the
> @@ -425,6 +444,8 @@ probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
> */
> probes_opcode_t origin_insn = insn;
>
> + asi->stack_space = 0;
I'm wondering what the convention for setting stack_space is. If we
initialise it to zero here, I guess it means that checker functions
don't need to change it unless they detect stack use? In which case,
check_insn_stack_regs doesn't need to zero it at it's start. However, if
we think that it is best for stack checker functions to always set
stack_space (not sure I do) then this is missing from chk_stack_stm and
chk_stack_t32_check_str.
> +
> if (emulate)
> insn = prepare_emulated_insn(insn, asi, thumb);
>
> @@ -503,3 +524,46 @@ probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
> }
> }
> }
> +
> +int __kprobes check_insn_stack_regs(probes_opcode_t insn,
I can't work out why the name ends with '_regs' ?
Would something like check_insn_stack_use be better?
> + struct arch_probes_insn *asi,
> + const struct decode_header *h,
> + int imm)
> +{
> + u32 regs = h->type_regs.bits >> DECODE_TYPE_BITS;
> + int rn = -1, rm = -1, index, add;
> + asi->stack_space = 0;
> +
> + if (((regs >> 16) & 0xf) != REG_TYPE_NONE)
> + rn = (insn >> 16) & 0xf;
> +
> + if ((regs & 0xf) != REG_TYPE_NONE)
> + rm = insn & 0xf;
> +
> + if ((rn != 13) && (rm != 13))
> + return NOT_STACK_STORE;
> +
> + index = insn & (1 << 24);
> + add = insn & (1 << 23);
> +
> + if (!index)
> + return NOT_STACK_STORE;
> +
> + /*
> + * Even if insn is 'str r0, [sp], +<Rm>', Rm may less than 0.
The word 'be' is missing from last sentence, it should read "Rm may be
less than 0"
> + * Therefore if both Rn and Rm are registers and !index,
> + * We are unable to determine whether it is a stack store.
> + */
> + if ((rn != -1) && (rm != -1)) {
> + asi->stack_space = -1;
> + return STACK_REG;
> + }
> +
> + /* 'str(d/h) r0, [sp], #+/-<imm>' */
The above instruction type can't cause us to get here because we earlier
return for !index (and if it could get here, the test below for 'add'
doesn't catch the -<imm> case).
> + /* or 'str(d/h) r0, [sp, #+<imm>'] */
> + if (add)
> + return NOT_STACK_STORE;
> +
> + asi->stack_space = imm;
> + return STACK_IMM;
> +}
> diff --git a/arch/arm/kernel/probes.h b/arch/arm/kernel/probes.h
> index b4bf1f5..b52629c 100644
> --- a/arch/arm/kernel/probes.h
> +++ b/arch/arm/kernel/probes.h
> @@ -413,4 +413,17 @@ probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
> const union decode_action *actions,
> const struct decode_checker **checkers);
>
> +enum probes_insn __kprobes chk_stack_stm(probes_opcode_t,
> + struct arch_probes_insn *,
> + const struct decode_header *);
> +
> +enum {
> + NOT_STACK_STORE,
I think the word 'STORE' can be misleading, "str r0, [sp,#4]" could be
regarded as a stack store, or if what we mean is update SP, then "sub
sp,sp,#N" does that and allocates stack space, neither of those is what
we're testing for. How about using the term 'stack use'? So the enum
names could become
STACK_USE_NONE, /* or NO_STACK_USE if preferred */
STACK_USE_REG,
STACK_USE_IMM,
> + STACK_REG,
> + STACK_IMM,
> +};
> +int __kprobes check_insn_stack_regs(probes_opcode_t insn,
> + struct arch_probes_insn *asi,
> + const struct decode_header *h,
> + int imm);
> #endif
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 4/4] ARM: kprobes: disallow probing stack consuming instructions
2014-10-25 6:42 [PATCH 0/4] ARM: kprobes: introduces instruction checker Wang Nan
` (2 preceding siblings ...)
2014-10-25 6:42 ` [PATCH 3/4] ARM: kprobes: collects stack consumption for store instructions Wang Nan
@ 2014-10-25 6:42 ` Wang Nan
2014-10-28 12:00 ` Jon Medhurst (Tixy)
3 siblings, 1 reply; 7+ messages in thread
From: Wang Nan @ 2014-10-25 6:42 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>
---
arch/arm/kernel/kprobes-test-arm.c | 16 ++++++++++------
arch/arm/kernel/kprobes.c | 8 ++++++++
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
index 264c064..59f9b25 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,"")
@@ -639,13 +641,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]") \
@@ -669,12 +673,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 618531d..59f5e64 100644
--- a/arch/arm/kernel/kprobes.c
+++ b/arch/arm/kernel/kprobes.c
@@ -124,6 +124,14 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
break;
}
+ /*
+ * Unable to instrument insn like 'str r0, [sp, +/-r1]'.
+ * __und_svc protects 64 bytes stack, so instrumenting insn
+ * likes 'str r0, [sp, #-68]' should be prohibited.
+ */
+ if ((p->ainsn.stack_space < 0) || (p->ainsn.stack_space > 64))
+ return -EINVAL;
+
return 0;
}
--
1.8.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/4] ARM: kprobes: disallow probing stack consuming instructions
2014-10-25 6:42 ` [PATCH 4/4] ARM: kprobes: disallow probing stack consuming instructions Wang Nan
@ 2014-10-28 12:00 ` Jon Medhurst (Tixy)
0 siblings, 0 replies; 7+ messages in thread
From: Jon Medhurst (Tixy) @ 2014-10-28 12:00 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, 2014-10-25 at 14:42 +0800, Wang Nan wrote:
> 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>
[...]
> diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
> index 618531d..59f5e64 100644
> --- a/arch/arm/kernel/kprobes.c
> +++ b/arch/arm/kernel/kprobes.c
> @@ -124,6 +124,14 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
> break;
> }
>
> + /*
> + * Unable to instrument insn like 'str r0, [sp, +/-r1]'.
> + * __und_svc protects 64 bytes stack, so instrumenting insn
> + * likes 'str r0, [sp, #-68]' should be prohibited.
> + */
> + if ((p->ainsn.stack_space < 0) || (p->ainsn.stack_space > 64))
Rather than using the hard coded value 64 it would be safest to create a
#define for the value (in arch/arm/include/asm/kprobes.h) and use that
define in __und_svc too.
Actually, there is already a define there for MAX_STACK_SIZE which is
used as the size of the stack for jprobes and the amount copied by
setjmp_pre_handler and longjmp_break_handler, which looks like it has
the same purpose as we want i.e. allowing space for probed instructions
to push things onto the stack. So we should probably use that define in
this patch and update __und_svc too.
Also, jprobe_return has this comment
* We allocate some slack between the original SP and start of
* our fabricated regs. To be precise we want to have worst case
* covered which is STMFD with all 16 regs so we allocate 2 *
* sizeof(struct_pt_regs)).
*
* This is to prevent any simulated instruction from writing
* over the regs when they are accessing the stack.
which looking at the code should probably more accurately specified as
sizeof(struct_pt_regs)+MAX_STACK_SIZE rather than
2*sizeof(struct_pt_regs)
I've not looked at the jprobes specific code before, so it would be good
to have someone else's option on what I save above about how
MAX_STACK_SIZE is and should be used.
--
Tixy
^ permalink raw reply [flat|nested] 7+ messages in thread