Re: [PATCH v15 06/25] x86/stacktool: Compile-time stack metadata validation

From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	live-patching@vger.kernel.org, Michal Marek <mmarek@suse.cz>,
	Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andi Kleen <andi@firstfloor.org>, Pedro Alves <palves@redhat.com>,
	Namhyung Kim <namhyung@gmail.com>,
	Bernd Petrovitsch <bernd@petrovitsch.priv.at>,
	Chris J Arges <chris.j.arges@canonical.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jiri Slaby <jslaby@suse.cz>,
	Arnaldo Carvalho de Melo <acme@kernel.org>
Subject: Re: [PATCH v15 06/25] x86/stacktool: Compile-time stack metadata validation
Date: Tue, 12 Jan 2016 09:06:39 -0600	[thread overview]
Message-ID: <20160112150639.GC310@treble.redhat.com> (raw)
In-Reply-To: <20160112144829.GC22699@pd.tnic>

On Tue, Jan 12, 2016 at 03:48:29PM +0100, Borislav Petkov wrote:
> On Fri, Dec 18, 2015 at 06:39:20AM -0600, Josh Poimboeuf wrote:
> > This adds a CONFIG_STACK_VALIDATION option which enables a host tool
> > named stacktool which runs at compile time.  It analyzes every .o file
> > and ensures the validity of its stack metadata.  It enforces a set of
> > rules on asm code and C inline assembly code so that stack traces can be
> > reliable.
> > 
> > For each function, it recursively follows all possible code paths and
> > validates the correct frame pointer state at each instruction.
> > 
> > It also follows code paths involving special sections, like
> > .altinstructions, __jump_table, and __ex_table, which can add
> > alternative execution paths to a given instruction (or set of
> > instructions).  Similarly, it knows how to follow switch statements, for
> > which gcc sometimes uses jump tables.
> 
> Have you run this through checkpatch?
> 
> Some of the warnings make sense, like:
> 
> ERROR: Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past, and may do so again. Linux already includes a copy of the GPL.
> #2496: FILE: tools/stacktool/arch/x86/insn/insn.h:17:
> + * along with this program; if not, write to the Free Software$

This file is a straight copy from the kernel arch/x86/include/asm/insn.h
file, and there's even a Makefile target which warns if there are any
differences between the two.  I figured it's such a minor issue that it
wasn't worth the churn to fix both files.

There are several other warnings from the copied files (insn.h and
list.h) but again I figured it wasn't worth the churn to fix them all.

I did run checkpatch on previous revisions of the patch set, though I
admit I forgot to run it on this one, and so I missed the improperly
formatted commit id reference in the list.h copy commit which you
pointed out.

Otherwise, all the remaining warnings seemed bogus to me.

-- 
Josh