Re: exercising current aarch64 kprobe support with systemtap

[William Cohen <wcohen@redhat.com>]

On 06/10/2016 09:42 AM, Pratyush Anand wrote:
> On 10/06/2016:01:49:10 AM, David Long wrote:
>> Attached are incremental diffs I hope will fix the latest systemtap
>> failures, without abandoning atomic sequence checking.  I'm trying to avoid
>> the hex constants but I don't think the insn.c functions help in this case.
> 
> It will save us from current problem by checking "stp x29,x30,[sp,...]"
> instruction and returning false if matches. However, we will have to find some
> way to recognize .word instructions.
> 
> * An assembly function may not start with "stp x29,x30,[sp,...]", e.g.
>  __dma_map_area(), _cpu_resume etc. However, it could be least likely that a
>  .word instruction exists before start of assembly function and that too
>  contains a word value which could be misleading.
> 
> * But major issue is, what if someone instruments a kprobe at an address which
>  contains  .word values. Instruction will never hit, so probe function will not
>  be called, but when real code reads that .word value, it reads a wrong value.
> 
> Can GCC provide some compiler option where .word values are located into a
> specific area?
> 
> ~Pratyush

Hi Dave and Pratyush,

Expecting the instruction to the stp x29, x30, [sp,...] would be pretty fragile.  The compiler might not generate that for some very simple function or with certain types of optimization. If the compiler could generate a sentinel word before the start of each function that might be a more robust solution.  Maybe something like a breakpoint instruction or something that clearly would not be in an atomic region.

-Will
> 
>>
>> -dl
>>
> 
>> diff --git a/arch/arm64/kernel/kprobes-arm64.c b/arch/arm64/kernel/kprobes-arm64.c
>> index 28b9c5b..36b4ea5 100644
>> --- a/arch/arm64/kernel/kprobes-arm64.c
>> +++ b/arch/arm64/kernel/kprobes-arm64.c
>> @@ -127,7 +127,9 @@ is_probed_address_atomic(kprobe_opcode_t *scan_start, kprobe_opcode_t *scan_end)
>>  		 * atomic region starts from exclusive load and ends with
>>  		 * exclusive store.
>>  		 */
>> -		if (aarch64_insn_is_store_ex(le32_to_cpu(*scan_start)))
>> +		if ((le32_to_cpu(*scan_start) & 0xffc07fff) == 0xa9807bfd)
>> +			return false;
>> +		else if (aarch64_insn_is_store_ex(le32_to_cpu(*scan_start)))
>>  			return false;
>>  		else if (aarch64_insn_is_load_ex(le32_to_cpu(*scan_start)))
>>  			return true;
>