Re: [PATCH v8 27/30] powerpc/kprobes: Don't allow breakpoints on suffixes

["Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>]

Christophe Leroy wrote:
> 
> 
> Le 06/05/2020 à 05:40, Jordan Niethe a écrit :
>> Do not allow inserting breakpoints on the suffix of a prefix instruction
>> in kprobes.
>> 
>> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
>> ---
>> v8: Add this back from v3
>> ---
>>   arch/powerpc/kernel/kprobes.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>> 
>> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
>> index 33d54b091c70..227510df8c55 100644
>> --- a/arch/powerpc/kernel/kprobes.c
>> +++ b/arch/powerpc/kernel/kprobes.c
>> @@ -106,7 +106,9 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
>>   int arch_prepare_kprobe(struct kprobe *p)
>>   {
>>   	int ret = 0;
>> +	struct kprobe *prev;
>>   	struct ppc_inst insn = ppc_inst_read((struct ppc_inst *)p->addr);
>> +	struct ppc_inst prefix = ppc_inst_read((struct ppc_inst *)(p->addr - 1));
> 
> What if p->addr is the first word of a page and the previous page is 
> not mapped ?

Good catch!
I think we can just skip validation if the instruction is at the 
beginning of a page. I have a few cleanups in this area - I will post a 
patchset soon.

> 
>>   
>>   	if ((unsigned long)p->addr & 0x03) {
>>   		printk("Attempt to register kprobe at an unaligned address\n");
>> @@ -114,6 +116,17 @@ int arch_prepare_kprobe(struct kprobe *p)
>>   	} else if (IS_MTMSRD(insn) || IS_RFID(insn) || IS_RFI(insn)) {
>>   		printk("Cannot register a kprobe on rfi/rfid or mtmsr[d]\n");
>>   		ret = -EINVAL;
>> +	} else if (ppc_inst_prefixed(prefix)) {
> 
> If p->addr - 2 contains a valid prefixed instruction, then p->addr - 1 contains the suffix of that 
> prefixed instruction. Are we sure a suffix can never ever be misinterpreted as the prefix of a 
> prefixed instruction ?

Yes. Per the ISA:
  Bits 0:5 of all prefixes are assigned the primary opcode
  value 0b000001. 0b000001 is not available for use as a
  primary opcode for either word instructions or suffixes
  of prefixed instructions.


- Naveen