All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>, 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,
	LKML <linux-kernel@vger.kernel.org>,
	clang-built-linux <clang-built-linux@googlegroups.com>
Subject: Re: [PATCH 05/15] x86: Implement function_nocfi
Date: Fri, 16 Apr 2021 15:52:44 -0700	[thread overview]
Message-ID: <CALCETrV6WYx7dt56aCuUYsrrFya==zYR+p-YZnaATptnaO7w2A@mail.gmail.com> (raw)
In-Reply-To: <202104161519.1D37B6D26@keescook>

On Fri, Apr 16, 2021 at 3:28 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Apr 16, 2021 at 03:06:17PM -0700, Andy Lutomirski wrote:
> > On Fri, Apr 16, 2021 at 3:03 PM Borislav Petkov <bp@alien8.de> wrote:
> > >
> > > On Fri, Apr 16, 2021 at 02:49:23PM -0700, Sami Tolvanen wrote:
> > > > __nocfi only disables CFI checking in a function, the compiler still
> > > > changes function addresses to point to the CFI jump table, which is
> > > > why we need function_nocfi().
> > >
> > > So call it __func_addr() or get_function_addr() or so, so that at least
> > > it is clear what this does.
> > >
> >
> > This seems backwards to me.  If I do:
> >
> > extern void foo(some signature);
> >
> > then I would, perhaps naively, expect foo to be the actual symbol that
> > gets called
>
> Yes.
>
> > and for the ABI to be changed to do the CFI checks.
>
> Uh, no? There's no ABI change -- indirect calls are changed to do the
> checking.

Maybe ABI is the wrong word, or maybe I'm not fully clued in.  But, if I do:

extern void call_it(void (*ptr)(void));

and I define call_it in one translation unit and call it from another,
the ABI effectively changed, right?  Because ptr is (depending on the
"canonical" mode) no longer a regular function pointer.

> > char entry_whatever[];
> > wrmsrl(..., (unsigned long)entry_whatever);
>
> This is just casting. It'll still resolve to the jump table entry.

How?  As far as clang is concerned, entry_whatever isn't a function at
all.  What jump table entry?

>
> > or, alternatively,
> >
> > extern void func() __attribute__((nocfi));
>
> __nocfi says func() should not perform checking of correct jump table
> membership for indirect calls.
>
> But we don't want a global marking for a function to be ignored by CFI;
> we don't want functions to escape CFI -- we want specific _users_ to
> either not check CFI for indirect calls (__nocfi) or we want specific
> passed addresses to avoid going through the jump table
> (function_nocfi()).

Maybe I spelled it badly, and I maybe I requested the wrong thing.
Here are actual required use cases.

1. I defined a function in asm.  I want to tell clang that this
function is defined in asm, and for clang to behave accordingly:

.globl func
func:
 ; do stuff

later:

extern void func(void) [something here];

There really should be a way to write this correctly such that it
works regardless of the setting of
-fsanitize-cfi-canonical-jump-tables.  This should not bypass CFI.  It
should *work*, with CFI enforced.  If I read all the various things
you linked correctly, this would be something like __cfi_noncanonical,
and I reserve the right to think that this is a horrible name.

2. I need a raw function pointer, thank you very much.  I would like
to be honest about it, and I don't really want to bypass CFI, but I
need the actual bits in the actual symbol.

translation unit 1 defines func.  Maybe it's C with
-fsanitize-cfi-canonical-jump-tables, maybe it's C with
-fno-sanitize-cfi-canonical-jump-tables or however it's spelled, and
maybe it's plain asm.  Now translation unit 2 does:

2a. Uses a literal symbol, because it's going to modify function text
or poke an MSR or whatever:

wrmsrl(MSR_WHATEVER, func);

clang needs to give us *some* way to have a correct declaration of
func such that we can, without resorting to inline asm kludges, get
the actual bit pattern of the actual symbol.

2b. Maybe optional: convert from function pointer to bit pattern of
actual symbol.

If someone gives me a real, correctly typed C pointer representing a
function pointer, I want a way to find the address of the body of the
function.  Then we can use it for things that aren't *calling* it per
se, e.g. disassembling it.  This is not necessarily a fully formed
thought right now, but I think something along these lines might be
needed.

The reverse of 2b (converting from actual symbol to function pointer)
might be equivalent to 1, or it might not.  I suppose there are some
subtleties here.

Be that as it may, it sounds like right now clang has some issues
interoperating with asm when CFI is enabled.  If so, clang needs to be
improved.

(The unsigned long hack is not necessarily good enough.  I should be able to do:

.global func
func:
 ; C ABI compliant code here

extern void func(void) [attribute as in 1]

unsigned long actual_address = [something clang actually understands](func);

If this thing refuses to work when fed a nonconstant function pointer
because of some genuinely good reason, fine.  But, if 'func' is an
actual literal symbol name, this thing needs to be compile-time
constant expression.

>
> So, instead of a cast, a wrapper is used to bypass instrumentation in
> the very few cases its needed.

NAK.  The compiler needs to cooperate IMO.

>
> (Note that such a wrapper is no-op without CFI enabled.)

> But note that this shouldn't turn into a discussion of "maybe Clang could
> do CFI differently"; this is what Clang has.
>
> https://clang.llvm.org/docs/ControlFlowIntegrity.html

If this is what Clang has, and Clang won't improve, then we can just
not apply these patches...

  reply	other threads:[~2021-04-16 22:53 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-16 20:38 [PATCH 00/15] x86: Add support for Clang CFI Sami Tolvanen
2021-04-16 20:38 ` [PATCH 01/15] objtool: Find a destination for jumps beyond the section end Sami Tolvanen
2021-04-20 18:14   ` Josh Poimboeuf
2021-04-20 20:25     ` Sami Tolvanen
2021-04-20 22:55       ` Josh Poimboeuf
2021-04-20 22:58         ` Nick Desaulniers
2021-04-16 20:38 ` [PATCH 02/15] objtool: Add CONFIG_CFI_CLANG support Sami Tolvanen
2021-04-20 19:47   ` Josh Poimboeuf
2021-04-20 20:45     ` Sami Tolvanen
2021-04-16 20:38 ` [PATCH 03/15] objtool: Add ASM_STACK_FRAME_NON_STANDARD Sami Tolvanen
2021-04-16 20:38 ` [PATCH 04/15] static_call: Use global functions for the self-test Sami Tolvanen
2021-04-16 21:37   ` Thomas Gleixner
2021-04-17  0:16     ` Thomas Gleixner
2021-04-16 20:38 ` [PATCH 05/15] x86: Implement function_nocfi Sami Tolvanen
2021-04-16 21:18   ` Borislav Petkov
2021-04-16 21:49     ` Sami Tolvanen
2021-04-16 22:02       ` Borislav Petkov
2021-04-16 22:06         ` Andy Lutomirski
2021-04-16 22:14           ` Borislav Petkov
2021-04-16 22:20             ` Andy Lutomirski
2021-04-16 22:37               ` Kees Cook
2021-04-16 23:02                 ` Thomas Gleixner
2021-04-17 10:16                   ` Thomas Gleixner
2021-04-19 15:13                     ` Sami Tolvanen
2021-04-16 22:28           ` Kees Cook
2021-04-16 22:52             ` Andy Lutomirski [this message]
2021-04-16 22:58               ` Kees Cook
2021-04-16 23:40               ` Kees Cook
2021-04-17 23:19                 ` Andy Lutomirski
2021-04-17 23:53                   ` Thomas Gleixner
2021-04-18  0:11                     ` Andy Lutomirski
2021-04-18 16:17                       ` Thomas Gleixner
2021-04-18 22:57                         ` Andy Lutomirski
2021-04-19 15:20                           ` Sami Tolvanen
2021-04-19 15:26                       ` David Laight
2021-04-19 17:46                         ` Andy Lutomirski
2021-04-17 14:20             ` David Laight
2021-04-17 15:48               ` Andy Lutomirski
2021-04-19  8:40             ` Rasmus Villemoes
2021-04-19 16:45               ` Joao Moreira
2021-04-19 21:52               ` David Laight
2021-04-16 22:16         ` Kees Cook
2021-04-16 22:13       ` Thomas Gleixner
2021-04-16 20:38 ` [PATCH 06/15] x86: Avoid CFI jump tables in IDT and entry points Sami Tolvanen
2021-04-16 22:26   ` Thomas Gleixner
2021-04-16 23:56     ` Kees Cook
2021-04-17  0:02       ` Thomas Gleixner
2021-04-16 20:38 ` [PATCH 07/15] x86/ftrace: Use function_nocfi in MCOUNT_ADDR Sami Tolvanen
2021-04-16 20:38 ` [PATCH 08/15] x86/extable: Do not mark exception callback as CFI Sami Tolvanen
2021-04-16 20:38 ` [PATCH 09/15] x86/alternatives: Use C int3 selftest but disable KASAN Sami Tolvanen
2021-04-17 11:37   ` Peter Zijlstra
2021-04-19 15:26     ` Sami Tolvanen
2021-04-20  7:19       ` Peter Zijlstra
2021-04-16 20:38 ` [PATCH 10/15] x86/purgatory: Disable CFI Sami Tolvanen
2021-04-16 20:38 ` [PATCH 11/15] x86, relocs: Ignore __typeid__ relocations Sami Tolvanen
2021-04-16 20:38 ` [PATCH 12/15] x86, module: " Sami Tolvanen
2021-04-16 20:38 ` [PATCH 13/15] x86, cpu: Use LTO for cpu.c with CFI Sami Tolvanen
2021-04-16 20:38 ` [PATCH 14/15] x86, kprobes: Fix optprobe_template_func type mismatch Sami Tolvanen
2021-04-16 20:38 ` [PATCH 15/15] x86, build: Allow CONFIG_CFI_CLANG to be selected Sami Tolvanen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALCETrV6WYx7dt56aCuUYsrrFya==zYR+p-YZnaATptnaO7w2A@mail.gmail.com' \
    --to=luto@kernel.org \
    --cc=bp@alien8.de \
    --cc=clang-built-linux@googlegroups.com \
    --cc=jpoimboe@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=peterz@infradead.org \
    --cc=samitolvanen@google.com \
    --cc=sedat.dilek@gmail.com \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH 05/15] x86: Implement function_nocfi' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.