RE: [PATCH 05/15] x86: Implement function_nocfi

From: David Laight <David.Laight@ACULAB.COM>
To: 'Rasmus Villemoes' <linux@rasmusvillemoes.dk>,
	Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>,
	Sami Tolvanen <samitolvanen@google.com>, X86 ML <x86@kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Nathan Chancellor <nathan@kernel.org>,
	"Nick Desaulniers" <ndesaulniers@google.com>,
	Sedat Dilek <sedat.dilek@gmail.com>,
	"linux-hardening@vger.kernel.org"
	<linux-hardening@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	clang-built-linux <clang-built-linux@googlegroups.com>
Subject: RE: [PATCH 05/15] x86: Implement function_nocfi
Date: Mon, 19 Apr 2021 21:52:48 +0000	[thread overview]
Message-ID: <a40525ea67144351a1baab99739fef52@AcuMS.aculab.com> (raw)
In-Reply-To: <2812c98b-3b5a-7be5-9fd9-2a6260406a45@rasmusvillemoes.dk>

From: Rasmus Villemoes
> Sent: 19 April 2021 09:40
> 
> On 17/04/2021 00.28, Kees Cook wrote:
> > On Fri, Apr 16, 2021 at 03:06:17PM -0700, Andy Lutomirski wrote:
> 
> >> The
> >> foo symbol would point to whatever magic is needed.
> >
> > No, the symbol points to the jump table entry. Direct calls get minimal
> > overhead and indirect calls can add the "is this function in the right
> > table" checking.
> >
> >
> > But note that this shouldn't turn into a discussion of "maybe Clang could
> > do CFI differently"; this is what Clang has.
> 
> Why not? In particular, I'd really like somebody to answer the question
> "why not just store a cookie before each address-taken or
> external-linkage function?".
> 
> (1) direct calls get _no_ overhead, not just "minimal".
> (2) address comparison, intra- or inter-module, Just Works, no need to
> disable one sanity check to get others.
> (3) The overhead and implementation of the signature check is the same
> for inter- and intra- module calls.
> (4) passing (unsigned long)sym into asm code or stashing it in a table
> Just Works.
> 
> How much code would be needed on the clang side to provide a
> --cfi-mode=inline ?
> 
> The only issue, AFAICT, which is also a problem being handled in the
> current proposal, is indirect calls to asm code from C. They either have
> to be instrumented to not do any checking (which requires callers to
> know that the pointer they have might point to an asm-implementation),
> or the asm code could be taught to emit those cookies as well - which
> would provide even better coverage. Something like
> 
> CFI_COOKIE(foo)
> foo:
>   ...
> 
> with CFI_COOKIE expanding to nothing when !CONFIG_CFI, and otherwise to
> (something like) ".quad cfi_cookie__foo" ; with some appropriate Kbuild
> and compiler magic to generate a table of cfi_cookie__* defines from a
> header file with the prototypes.

I'm wondering what 'threat models' CFI is trying to protect from.
Adding code to code doing 'call indirect' can only protect against
calls to 'inappropriate' functions.
You may also be worried about whether functions are being called
from the right place - someone might manage to 'plant' an indirect
jump somewhere - probably more likely than overwriting an address.
But a truly malicious caller will be able to subvert most checks.
And non-malicious errors can be largely eliminated by strengthening
normal compiler type checking.

A simple run-time check would be adding an extra 'hidden' function
parameter that is a hash of the prototype.
Especially if the hash is based on a build-id - so differs
between kernel builds.

Having the caller scan a list of valid targets seems just broken.
You might as well replace the function pointer with an index
into the array of valid targets.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)