linux-kernel - Re: [PATCH v2 4/5] powerpc: kprobes: factor out code to emulate instruction into a helper

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1492073293.wa42fra50w.astroid@naverao1-tp.none>
Date:   Thu, 13 Apr 2017 08:50:33 +0000
From:   "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>,
        linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        Ingo Molnar <mingo@...nel.org>,
        Michael Ellerman <mpe@...erman.id.au>
Subject: Re: [PATCH v2 4/5] powerpc: kprobes: factor out code to emulate
 instruction into a helper

Excerpts from Masami Hiramatsu's message of April 13, 2017 10:04:
> On Wed, 12 Apr 2017 16:28:27 +0530
> "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com> wrote:
> 
>> This helper will be used in a subsequent patch to emulate instructions
>> on re-entering the kprobe handler. No functional change.
> 
> In this case, please merge this patch into the next patch which
> actually uses the factored out function unless that changes
> too much.

In hindsight, this patch actually just refactors the code so that the 
helper can be re-used subsequently. Using the helper constitutes a 
separate unrelated change, so I'm keeping this patch as is. I am 
updating the description to convey this better.

- Naveen

> 
> Thank you,
> 
>> 
>> Acked-by: Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@...ux.vnet.ibm.com>
>> ---
>>  arch/powerpc/kernel/kprobes.c | 52 ++++++++++++++++++++++++++-----------------
>>  1 file changed, 31 insertions(+), 21 deletions(-)
>> 
>> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
>> index 0732a0291ace..8b48f7d046bd 100644
>> --- a/arch/powerpc/kernel/kprobes.c
>> +++ b/arch/powerpc/kernel/kprobes.c
>> @@ -207,6 +207,35 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
>>  	regs->link = (unsigned long)kretprobe_trampoline;
>>  }
>>  
>> +int __kprobes try_to_emulate(struct kprobe *p, struct pt_regs *regs)
>> +{
>> +	int ret;
>> +	unsigned int insn = *p->ainsn.insn;
>> +
>> +	/* regs->nip is also adjusted if emulate_step returns 1 */
>> +	ret = emulate_step(regs, insn);
>> +	if (ret > 0) {
>> +		/*
>> +		 * Once this instruction has been boosted
>> +		 * successfully, set the boostable flag
>> +		 */
>> +		if (unlikely(p->ainsn.boostable == 0))
>> +			p->ainsn.boostable = 1;
>> +	} else if (ret < 0) {
>> +		/*
>> +		 * We don't allow kprobes on mtmsr(d)/rfi(d), etc.
>> +		 * So, we should never get here... but, its still
>> +		 * good to catch them, just in case...
>> +		 */
>> +		printk("Can't step on instruction %x\n", insn);
>> +		BUG();
>> +	} else if (ret == 0)
>> +		/* This instruction can't be boosted */
>> +		p->ainsn.boostable = -1;
>> +
>> +	return ret;
>> +}
>> +
>>  int __kprobes kprobe_handler(struct pt_regs *regs)
>>  {
>>  	struct kprobe *p;
>> @@ -302,18 +331,9 @@ int __kprobes kprobe_handler(struct pt_regs *regs)
>>  
>>  ss_probe:
>>  	if (p->ainsn.boostable >= 0) {
>> -		unsigned int insn = *p->ainsn.insn;
>> +		ret = try_to_emulate(p, regs);
>>  
>> -		/* regs->nip is also adjusted if emulate_step returns 1 */
>> -		ret = emulate_step(regs, insn);
>>  		if (ret > 0) {
>> -			/*
>> -			 * Once this instruction has been boosted
>> -			 * successfully, set the boostable flag
>> -			 */
>> -			if (unlikely(p->ainsn.boostable == 0))
>> -				p->ainsn.boostable = 1;
>> -
>>  			if (p->post_handler)
>>  				p->post_handler(p, regs, 0);
>>  
>> @@ -321,17 +341,7 @@ int __kprobes kprobe_handler(struct pt_regs *regs)
>>  			reset_current_kprobe();
>>  			preempt_enable_no_resched();
>>  			return 1;
>> -		} else if (ret < 0) {
>> -			/*
>> -			 * We don't allow kprobes on mtmsr(d)/rfi(d), etc.
>> -			 * So, we should never get here... but, its still
>> -			 * good to catch them, just in case...
>> -			 */
>> -			printk("Can't step on instruction %x\n", insn);
>> -			BUG();
>> -		} else if (ret == 0)
>> -			/* This instruction can't be boosted */
>> -			p->ainsn.boostable = -1;
>> +		}
>>  	}
>>  	prepare_singlestep(p, regs);
>>  	kcb->kprobe_status = KPROBE_HIT_SS;
>> -- 
>> 2.12.1
>> 
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@...nel.org>
> 
>