Re: [PATCH v5 09/15] x86: Use an opaque type for functions not callable from C

From: Borislav Petkov <bp@alien8.de>
To: Kees Cook <keescook@chromium.org>
Cc: Sami Tolvanen <samitolvanen@google.com>,
	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>,
	Steven Rostedt <rostedt@goodmis.org>,
	linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev
Subject: Re: [PATCH v5 09/15] x86: Use an opaque type for functions not callable from C
Date: Thu, 14 Oct 2021 19:31:26 +0200	[thread overview]
Message-ID: <YWhpbu/Y6V2p/zlY@zn.tnic> (raw)
In-Reply-To: <202110140904.41B5183E@keescook>

On Thu, Oct 14, 2021 at 09:07:57AM -0700, Kees Cook wrote:
> I don't think it's a super common thing to add, so in this case, yes,
> I think doing it on a case-by-case basis will be fine. 

You don't have a choice - if there's no automation which verifies
whether all the CFI annotation needed is there, people won't know what
to wrap in what macro.

> I'd _much_ prefer keeping the macro, as it explains what's going on,
> which doesn't require a comment at every "extern const u8 foo[]" usage.

You don't have to - it is just an extern.

> It serves as an annotation, etc.

Oh, that I figured.

> And, there's been a lot of discussion on the best way to do this, what
> to name it, etc.

Looking at the changelog, DECLARE_ASM_FUNC_SYMBOL, makes a lot more
sense to me even if it doesn't specify the aspect that it is not called
by C but who cares - it is generic enough.

> This looks to be the best option currently.

Maybe because wrapping some random symbols in a obfuscating macro to
make the next tool happy, is simply the wrong thing to do. I know, I
know, clang CFI needs it because of magical reason X but that doesn't
make it any better. Someday soon we'll have to write a tutorial for
people submitting kernel patches explaining what annotation to add where
and why.

Why can't clang be taught to ignore those symbols:

clang -fsanitize=cfi -fsanitize-cfi-ignore-symbols=<list>

?

Hmm, looking at https://clang.llvm.org/docs/ControlFlowIntegrity.html

there *is* an ignore list:

"Ignorelist

A Sanitizer special case list can be used to relax CFI checks for
certain source files, functions and types using the src, fun and type
entity types. Specific CFI modes can be be specified using [section]
headers.

...

# Ignore all functions with names containing MyFooBar.
fun:*MyFooBar*
..."

So why aren't we doing that instead of those macros?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette