linux-kernel - Re: [RFC PATCH v2 11/20] objtool: arm64: Walk instructions and compute CFI for each instruction

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: <767e814c-b3cc-73a1-d283-82dbc0287cde@huawei.com>
Date:   Tue, 24 May 2022 21:45:19 +0800
From:   Chen Zhongjin <chenzhongjin@...wei.com>
To:     <madvenka@...ux.microsoft.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

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.

> +	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.

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.