Subject: [PATCH v2 1/3] ARM: kprobes: introduces checker

From: [email-masked] (Masami Hiramatsu)
Date: Thu, 20 Nov 2014 20:57:58 +0900

(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 <[email-masked]>

Thank you,

> 
> Signed-off-by: Wang Nan <[email-masked]>
> ---
>  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;
> 

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt at hitachi.com