linux-kernel - Re: [PATCH] arm64: perf: Report arm pc registers for compat perf

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] [day] [month] [year] [list]

Message-ID: <5DCB8380.3070304@samsung.com>
Date:   Wed, 13 Nov 2019 13:16:00 +0900
From:   Seung-Woo Kim <sw0312.kim@...sung.com>
To:     Mark Rutland <mark.rutland@....com>
CC:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        will@...nel.org, catalin.marinas@....com, sungguk.na@...sung.com
Subject: Re: [PATCH] arm64: perf: Report arm pc registers for compat perf

Hi Mark Rutland,

On 2019년 11월 12일 18:40, Mark Rutland wrote:
> On Tue, Nov 12, 2019 at 10:01:41AM +0900, Seung-Woo Kim wrote:
>> If perf is built as arm 32-bit, it only reads 15 registers as arm
>> 32-bit register map and this breaks dwarf call-chain in compat
>> perf because pc register information is not filled.
>> Report arm pc registers for 32-bit compat perf.
>>
>> Without this, arm 32-bit perf dwarf call-graph shows below
>> verbose message:
>>   unwind: reg 15, val 0
>>   unwind: reg 13, val ffbc6360
>>   unwind: no map for 0
>>
>> Signed-off-by: Seung-Woo Kim <sw0312.kim@...sung.com>
>> ---
>>  arch/arm64/kernel/perf_regs.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/perf_regs.c b/arch/arm64/kernel/perf_regs.c
>> index 0bbac61..d4172e7 100644
>> --- a/arch/arm64/kernel/perf_regs.c
>> +++ b/arch/arm64/kernel/perf_regs.c
>> @@ -24,6 +24,8 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
>>  			return regs->compat_sp;
>>  		if ((u32)idx == PERF_REG_ARM64_LR)
>>  			return regs->compat_lr;
>> +		if ((u32)idx == 15) /* PERF_REG_ARM_PC */
>> +			return regs->pc;
>>  	}
> 
> This doesn't look quite right to me, since perf_regs_value() is
> consuming the arm64 index for all other registers (e.g. the LR, in the
> patch context).
> 
> i.e. this is designed for a native arm64 caller, and the fixup allows it
> to view a compat task's registers as-if it were native.
> 
> How does this work for a native arm64 perf invocation with a compat
> task? I assume it consumers regs->pc, and works as expected?

In native arm64 perf, compat task registers are set as arm64 register
map, and sp, lr, and pc are set properly. But compat_sp is from regs[13]
and compat_lr is from regs[14], so same register values are set for
regs[13]/egs->sp and regs[14]/regs->lr. With this change, it sets
regs[15] same with regs->pc, but the register is not use at least for
arm 32-bit compat binary callchain, so no issue as far as I understood
and tested.

> 
> I suspect we need separate native and compat forms of this function, but
> then it's not entirely clear how this should work -- how does this work
> for a compat perf analysing a native arm64 binary?

I didn't expect native arm64 binary callchain is possible to get from
arm 32-bit perf.

In my test with 32-bit compat perf, it sets perf
event->attr.sample_regs_user as 0xffff, which is matched with 32-bit
arm, but in arm64 perf part, it cannot be accessed. If there is way to
check it, it is possible to set difference register form. Anyway, in the
case, native arm64 register map is still not fully reported to 32-bit
compat perf.


Thanks,
- Seung-Woo Kim

> 
> Thanks,
> Mark.
> 
> 

-- 
Seung-Woo Kim
Samsung Research
--