| Message ID | 1416291582-74982-2-git-send-email-wangnan0@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2014-11-18 at 14:19 +0800, Wang Nan wrote: > This patch introdces 'checker' to decoding phase, and calls checkers > when instruction decoding. This allows further decoding for specific > instructions. > > v1 -> v2: > - kprobe checker stubs are introduced in this patch. Showing changes between versions, like above, is a good thing to do as it helps patch reviewers, but the changes don't want to appear in the commit message when they are applied to git. So they should be put bellow the first '---' where 'git am' will ignore them. This also applies to some other patches in your series. As for the actual code in this patch, it looks good to me, there's just one nit-picking issue below, but when you fix that, and the commit message to omit the comments on version difference, please also add Reviewed-by: Jon Medhurst <tixy@linaro.org> (I'll be commenting on the other patches shortly, but from my first look I don't expect there to be any significant issues.) > > Signed-off-by: Wang Nan <wangnan0@huawei.com> > --- > 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 | 51 ++++++++++++++++++++++++++++++++++++++++- > arch/arm/kernel/probes.h | 11 ++++++++- > arch/arm/kernel/uprobes.c | 2 +- > 11 files changed, 91 insertions(+), 15 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..f114a3f 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, NULL)) { > 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..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); Now action is a local variable, the above "d->decoder.action" can just be replaced with "action". There are several other places below where this applies too. (I'm sure the compiler will generate the same code either way, but seems tidier to me) > } > > 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; s/d->handler.action/action/ ^^^ > 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, s/d->handler.action/action/ ^^^ After changing the above, the line will be short enough that it doesn't need splitting across two lines so will read: if (!emulate) return actions[action].decoder(insn, asi, h); Finally, there is one more "d->handler.action" immediately after this that can be replaced but it isn't shown in this patch context. > 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;
(2014/11/18 15:19), Wang Nan wrote: > This patch introdces 'checker' to decoding phase, and calls checkers > when instruction decoding. This allows further decoding for specific > instructions. > > v1 -> v2: > - kprobe checker stubs are introduced in this patch. Hmm, could you state this patch just introduce a "stub" call of the checker in the patch description? (not only version comment) Other parts are OK to me. Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> Thank you, > > Signed-off-by: Wang Nan <wangnan0@huawei.com> > --- > 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 | 51 ++++++++++++++++++++++++++++++++++++++++- > arch/arm/kernel/probes.h | 11 ++++++++- > arch/arm/kernel/uprobes.c | 2 +- > 11 files changed, 91 insertions(+), 15 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..f114a3f 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, NULL)) { > 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..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; >
(2014/11/18 15:19), Wang Nan wrote: > diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c > index 6d64420..f114a3f 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, NULL)) { BTW, even if checkers always empty at this point, we should pass the checkers to this function. Thank you,
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..f114a3f 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, NULL)) { 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..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;
This patch introdces 'checker' to decoding phase, and calls checkers when instruction decoding. This allows further decoding for specific instructions. v1 -> v2: - kprobe checker stubs are introduced in this patch. Signed-off-by: Wang Nan <wangnan0@huawei.com> --- 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 | 51 ++++++++++++++++++++++++++++++++++++++++- arch/arm/kernel/probes.h | 11 ++++++++- arch/arm/kernel/uprobes.c | 2 +- 11 files changed, 91 insertions(+), 15 deletions(-)