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]
Date:   Sun, 29 May 2022 10:18:00 -0500
From:   "Madhavan T. Venkataraman" <madvenka@...ux.microsoft.com>
To:     Chen Zhongjin <chenzhongjin@...wei.com>, jpoimboe@...hat.com,
        peterz@...radead.org, mark.rutland@....com, broonie@...nel.org,
        nobuta.keiya@...itsu.com, sjitindarsingh@...il.com,
        catalin.marinas@....com, will@...nel.org,
        jamorris@...ux.microsoft.com, linux-arm-kernel@...ts.infradead.org,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v2 11/20] objtool: arm64: Walk instructions and
 compute CFI for each instruction



On 5/24/22 08:45, Chen Zhongjin wrote:
> Hi,
> 
> On 2022/5/24 8:16, madvenka@...ux.microsoft.com wrote:
>> From: "Madhavan T. Venkataraman" <madvenka@...ux.microsoft.com>
>>
>> Implement arch_initial_func_cfi_state() to initialize the CFI for a
>> function.
>>
>> Add code to fpv_decode() to walk the instructions in every function and
>> compute the CFI information for each instruction.
>>
>> Implement special handling for cases like jump tables.
>>
>> Signed-off-by: Madhavan T. Venkataraman <madvenka@...ux.microsoft.com>
>> ---
>>  tools/objtool/arch/arm64/decode.c |  15 +++
>>  tools/objtool/fpv.c               | 204 ++++++++++++++++++++++++++++++
>>  2 files changed, 219 insertions(+)
> ...
>> +static void update_cfi_state(struct cfi_state *cfi, struct stack_op *op)
>> +{
>> +	struct cfi_reg *cfa = &cfi->cfa;
>> +	struct cfi_reg *regs = cfi->regs;
>> +
>> +	if (op->src.reg == CFI_SP) {
>> +		if (op->dest.reg == CFI_SP)
>> +			cfa->offset -= op->src.offset;
>> +		else
>> +			regs[CFI_FP].offset = -cfa->offset + op->src.offset;
> Seems wrong here, we don't have any op->src.offset for [mov x29, sp] so here we
> get: fp->offset = -cfa->offset. The dumped info also proves this.


See the example below.

> 
>> +	case UNWIND_HINT_TYPE_CALL:
>> +		/* Normal call */
>> +		frame->cfa += orc->sp_offset;
>> +		fp = frame->cfa + orc->fp_offset;
>> +		break;
> Obviously this is not conform to the reliability check because we get
> frame->cfa == fp here.
> 

See the example below:

> IIUC your sp_offset equals to stack length, and fp_offset is offset from next
> x29 to next CFA. So maybe here we should have
> regs[CFI_FP].offset = regs[CFI_SP].offset for [mov x29, sp].
> 
> Anyway, in original objtool sp_offset and fp_offset both represents the offset
> from CFA to REGs. I think it's better not spoiling their original meaning and
> just extending.
> 
> 

I am not spoiling anything.


Let us take an example:

ffff800008010000 <bcm2835_handle_irq>:
ffff800008010000:       d503201f        nop
ffff800008010004:       d503201f        nop
ffff800008010008:       d503233f        paciasp
ffff80000801000c:       a9be7bfd        stp     x29, x30, [sp, #-32]!
ffff800008010010:       910003fd        mov     x29, sp
ffff800008010014:       f9000bf3        str     x19, [sp, #16]


The stack pointer is first moved by -32 and the FP and LR are stored there.
At this point, SP is pointing to the frame. The CFA is:

	CFA = SP + 32

The frame pointer has been stored at the location pointed to by the SP.
So, FP should be:

	FP = CFA - 32

Therefore, at instruction address ffff800008010014:

	frame->cfa = SP + 32;
	frame->fp = frame->cfa - 32 = SP;

So, if a call/interrupt happens after this instruction, the frame pointer computed
from the above data will match with the actual frame pointer.

I have verified this using the DWARF data generated by the compiler. It is correct.
I have also verified that the stack trace through such code passes the reliability
check. That is, it computes the frame pointer correctly which matches with the
actual frame pointer.

Madhavan

Powered by blists - more mailing lists