linux-kernel - Re: [PATCH v5 02/10] x86: Compile-time asm code validation

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:	Wed, 10 Jun 2015 11:15:19 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Josh Poimboeuf <jpoimboe@...hat.com>
Cc:	Michal Marek <mmarek@...e.cz>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Andi Kleen <andi@...stfloor.org>,
	live-patching@...r.kernel.org, X86 ML <x86@...nel.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v5 02/10] x86: Compile-time asm code validation

On Wed, Jun 10, 2015 at 10:53 AM, Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> On Wed, Jun 10, 2015 at 10:21:36AM -0700, Andy Lutomirski wrote:
>> On Jun 10, 2015 5:07 AM, "Josh Poimboeuf" <jpoimboe@...hat.com> wrote:
>> >
>> > Add a new CONFIG_ASM_VALIDATION option which adds an asmvalidate host
>> > tool which runs on every compiled .S file.  Its goal is to enforce sane
>> > rules on all asm code, so that stack debug metadata (frame/back chain
>> > pointers and/or DWARF CFI metadata) can be made reliable.
>> >
>> > It enforces the following rules:
>> >
>> > 1. Each callable function must be annotated with the ELF STT_FUNC type.
>> >    This is typically done using the ENTRY/ENDPROC macros.  If
>> >    asmvalidate finds a return instruction outside of a function, it
>> >    flags an error, since that usually indicates callable code which
>> >    should be annotated accordingly.
>> >
>> > 2. Each callable function must never leave its own bounds (i.e. with a
>> >    jump to outside the function) except when returning.
>>
>> Won't that break with sibling/tail calls?
>
> Yes, asmvalidate will flag a warning for tail calls.
>
>> GCC can generate those, and the ia32_ptregs_common label is an example
>> of such a thing.
>>
>> I'd rather have the script understand tail calls and possibly require
>> that ia32_ptregs_common have a dummy frame pointer save in front
>> before the label if needed.
>
> Why do you prefer tail calls there?  See patch 3 for how I handled that
> for ia32_ptregs_common (I duplicated the code with macros).
>
> I think adding support for tail calls in the tooling would be tricky.
> So I'm just trying to figure out if there's a good reason to keep them.

To save code size by deduplicating common tails.  The code currently
does that, and it would be nice to avoid bloating the code to keep the
validator happy.

I imagine that an automatic CFI annotation adder would walk through
functions one instruction at a time and keep track of the frame state.
If so, then it could verify that common jump targets had identical
state and continue walking through them and annotating.  I think this
would get this case right, and it might be necessary anyway to handle
jumps within functions.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/