linux-kernel - Re: [RFC 2/6] objtool: arm64: Add required implementation for supporting the aarch64 architecture in objtool.

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: <20190424162324.36xn7aepuggyvbzi@treble>
Date:   Wed, 24 Apr 2019 11:23:24 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Raphael Gault <Raphael.Gault@....com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        Catalin Marinas <Catalin.Marinas@....com>,
        Will Deacon <Will.Deacon@....com>,
        Julien Thierry <Julien.Thierry@....com>
Subject: Re: [RFC 2/6] objtool: arm64: Add required implementation for
 supporting the aarch64 architecture in objtool.

On Wed, Apr 24, 2019 at 04:16:26PM +0000, Raphael Gault wrote:
> On 4/23/19 9:18 PM, Josh Poimboeuf wrote:
> > On Tue, Apr 09, 2019 at 02:52:39PM +0100, Raphael Gault wrote:
> >> Provide implementation for the arch-dependent functions that are called by the main check
> >> function of objtool.
> >> The ORC unwinder is not yet supported by the arm64 architecture so we only provide a dummy
> >> interface for now.
> >> The decoding of the instruction is split into classes and subclasses as described into
> >> the Instruction Encoding in the ArmV8.5 Architecture Reference Manual.
> >
> > Where did the code for the decoder come from?  Was it written from
> > scratch?
> >
> 
> This decoder was indeed written from scratch based on the armv8 ARM. The
> automatic generation idea hasn't really been discussed yet.

Ok.  That's a lot of code.  Hopefully ARM folks can review it closely.

> >> +#ifndef __ASSEMBLY__
> >> +/*
> >> + * This struct is more or less a vastly simplified version of the DWARF Call
> >> + * Frame Information standard.  It contains only the necessary parts of DWARF
> >> + * CFI, simplified for ease of access by the in-kernel unwinder.  It tells the
> >> + * unwinder how to find the previous SP and BP (and sometimes entry regs) on
> >> + * the stack for a given code address.  Each instance of the struct corresponds
> >> + * to one or more code locations.
> >> + */
> >> +struct orc_entry {
> >> +s16sp_offset;
> >> +s16bp_offset;
> >> +unsignedsp_reg:4;
> >> +unsignedbp_reg:4;
> >> +unsignedtype:2;
> >> +unsignedend:1;
> >> +} __packed;
> >> +
> >> +/*
> >> + * This struct is used by asm and inline asm code to manually annotate the
> >> + * location of registers on the stack for the ORC unwinder.
> >> + *
> >> + * Type can be either ORC_TYPE_* or UNWIND_HINT_TYPE_*.
> >> + */
> >> +struct unwind_hint {
> >> +u32ip;
> >> +s16sp_offset;
> >> +u8sp_reg;
> >> +u8type;
> >> +u8end;
> >> +};
> >> +#endif /* __ASSEMBLY__ */
> >> +
> >> +#endif /* _ORC_TYPES_H */
> >
> > It seems odd to have the above header file in arm64 code, since it
> > doesn't implement ORC.  Is it really needed?
> >
> 
> The unwind_hint part can safely be removed. However the orc_entry seems
> to be needed since the struct instruction comports a struct orc_entry
> field. I have chosen to let it here as well but maybe a better approach
> is possible.

Ideally we can figure out a way to decouple 'struct instruction' from
'struct orc_entry'.  But yes, I think your approach is fine for now.

Though I think using an arch-independent header file would be better, to
avoid creating duplicated (dead) code.

-- 
Josh