From: Ard Biesheuvel <ardb@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
Sami Tolvanen <samitolvanen@google.com>, X86 ML <x86@kernel.org>,
Kees Cook <keescook@chromium.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
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 Mailing List <linux-kernel@vger.kernel.org>,
llvm@lists.linux.dev
Subject: Re: [PATCH v5 00/15] x86: Add support for Clang CFI
Date: Wed, 27 Oct 2021 15:30:11 +0200 [thread overview]
Message-ID: <CAMj1kXHKh7wv6JqusVnoiQDMm7ApFq2ujzbfWmM9AzLKFehhAA@mail.gmail.com> (raw)
In-Reply-To: <YXlOd1lyKZKAcJfA@hirez.programming.kicks-ass.net>
On Wed, 27 Oct 2021 at 15:05, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Oct 27, 2021 at 02:48:52PM +0200, Peter Zijlstra wrote:
> > On Wed, Oct 27, 2021 at 02:22:27PM +0200, Ard Biesheuvel wrote:
> > > On Wed, 27 Oct 2021 at 14:05, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > > > > Should not this jump-table thingy get converted to an actual function
> > > > > address somewhere around arch_static_call_transform() ? This also seems
> > > > > relevant for arm64 (which already has CLANG_CFI supported) given:
> > > > >
> > > > > https://lkml.kernel.org/r/20211025122102.46089-3-frederic@kernel.org
> > > >
> > > > Ugh, yeah, we'll need to do the function_nocfi() dance somewhere...
> > > >
> > >
> > > Sadly, that only works on symbol names, so we cannot use it to strip
> > > CFI-ness from void *func arguments passed into the static call API,
> > > unfortunately.
> >
> > Right, and while mostly static_call_update() is used, whcih is a macro
> > and could possibly be used to wrap this, we very much rely on
> > __static_call_update() also working without that wrapper and then we're
> > up a creek without no paddles.
>
> Specifically, we support code like:
>
> struct foo {
> void (*func1)(args1);
> void (*func2)(args2);
> }
>
> struct foo global_foo;
>
> ...
>
> DEFINE_STATIC_CALL_NULL(func1, *global_foo.func1);
>
> ...
>
> __init foo_init()
> {
> // whatever code that fills out foo
>
> static_call_update(func1, global_foo.func1);
> }
>
> ...
>
> hot_function()
> {
> ...
> static_cal(func1)(args1);
> ...
> }
>
> cold_function()
> {
> ...
> global_foo->func1(args1);
> ...
> }
>
> And there is no way I can see this working sanely with CFI as presented.
>
> Even though the above uses static_call_update(), we can't no longer use
> function_nocfi() on the @func argument, because it's not a symbol, it's
> already a function pointer.
>
> Nor can we fill global_foo.func1 with function_nocfi() because then
> cold_function() goes sideways.
>
As far as I can tell from playing around with Clang, the stubs can
actually be executed directly, they just jumps to the actual function.
The compiler simply generates a jump table for each prototype that
appears in the code as the target of an indirect jump, and checks
whether the target appears in the list.
E.g., the code below
void foo(void) {}
void bar(int) {}
void baz(int) {}
void (* volatile fn1)(void) = foo;
void (* volatile fn2)(int) = bar;
int main(int argc, char *argv[])
{
fn1();
fn2 = baz;
fn2(-1);
}
produces
0000000000400594 <foo.cfi>:
400594: d65f03c0 ret
0000000000400598 <bar.cfi>:
400598: d65f03c0 ret
000000000040059c <baz.cfi>:
40059c: d65f03c0 ret
00000000004005a0 <main>:
4005a0: a9bf7bfd stp x29, x30, [sp, #-16]!
// First indirect call
4005a4: b0000088 adrp x8, 411000 <__libc_start_main@GLIBC_2.17>
4005a8: f9401508 ldr x8, [x8, #40]
4005ac: 90000009 adrp x9, 400000 <__abi_tag-0x278>
4005b0: 91182129 add x9, x9, #0x608
4005b4: 910003fd mov x29, sp
4005b8: eb09011f cmp x8, x9
4005bc: 54000241 b.ne 400604 <main+0x64> // b.any
4005c0: d63f0100 blr x8
// Assignment of fn2
4005c4: 90000009 adrp x9, 400000 <__abi_tag-0x278>
4005c8: b0000088 adrp x8, 411000 <__libc_start_main@GLIBC_2.17>
4005cc: 91184129 add x9, x9, #0x610
4005d0: f9001909 str x9, [x8, #48]
// Second indirect call
4005d4: f9401908 ldr x8, [x8, #48]
4005d8: 90000009 adrp x9, 400000 <__abi_tag-0x278>
4005dc: 91183129 add x9, x9, #0x60c
4005e0: cb090109 sub x9, x8, x9
4005e4: 93c90929 ror x9, x9, #2
4005e8: f100053f cmp x9, #0x1
4005ec: 540000c8 b.hi 400604 <main+0x64> // b.pmore
4005f0: 12800000 mov w0, #0xffffffff // #-1
4005f4: d63f0100 blr x8
4005f8: 2a1f03e0 mov w0, wzr
4005fc: a8c17bfd ldp x29, x30, [sp], #16
400600: d65f03c0 ret
400604: d4200020 brk #0x1
0000000000400608 <__typeid__ZTSFvvE_global_addr>:
400608: 17ffffe3 b 400594 <foo.cfi>
000000000040060c <__typeid__ZTSFviE_global_addr>:
40060c: 17ffffe3 b 400598 <bar.cfi>
400610: 17ffffe3 b 40059c <baz.cfi>
So it looks like taking the address is fine, although not optimal due
to the additional jump. We could fudge around that by checking the
opcode at the target of the call, or token paste ".cfi" after the
symbol name in the static_call_update() macro, but it doesn't like
like anything is terminally broken tbh.
next prev parent reply other threads:[~2021-10-27 13:30 UTC|newest]
Thread overview: 117+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-13 18:16 [PATCH v5 00/15] x86: Add support for Clang CFI Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 01/15] objtool: Add CONFIG_CFI_CLANG support Sami Tolvanen
2021-10-13 18:59 ` Kees Cook
2021-10-14 0:44 ` Josh Poimboeuf
2021-10-14 10:22 ` Peter Zijlstra
2021-10-14 19:20 ` Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 02/15] objtool: Add ASM_STACK_FRAME_NON_STANDARD Sami Tolvanen
2021-10-13 18:59 ` Kees Cook
2021-10-13 18:16 ` [PATCH v5 03/15] linkage: Add DECLARE_NOT_CALLED_FROM_C Sami Tolvanen
2021-10-13 19:00 ` Kees Cook
2021-10-15 2:51 ` Andy Lutomirski
2021-10-15 15:35 ` Sami Tolvanen
2021-10-15 15:55 ` Thomas Gleixner
2021-10-15 16:22 ` Andy Lutomirski
2021-10-15 16:47 ` Sami Tolvanen
2021-10-15 17:34 ` Andy Lutomirski
2021-10-15 17:57 ` Thomas Gleixner
2021-10-15 18:42 ` Sami Tolvanen
2021-10-15 19:35 ` Andy Lutomirski
2021-10-15 20:37 ` Sami Tolvanen
2021-10-16 21:12 ` Josh Poimboeuf
2021-10-18 17:08 ` Sami Tolvanen
2021-10-15 22:17 ` Thomas Gleixner
2021-10-16 21:16 ` Josh Poimboeuf
2021-10-13 18:16 ` [PATCH v5 04/15] cfi: Add DEFINE_CFI_IMMEDIATE_RETURN_STUB Sami Tolvanen
2021-10-13 19:02 ` Kees Cook
2021-10-13 18:16 ` [PATCH v5 05/15] tracepoint: Exclude tp_stub_func from CFI checking Sami Tolvanen
2021-10-13 19:03 ` Kees Cook
2021-10-13 19:20 ` Steven Rostedt
2021-10-13 18:16 ` [PATCH v5 06/15] ftrace: Use an opaque type for functions not callable from C Sami Tolvanen
2021-10-13 19:04 ` Kees Cook
2021-10-13 19:20 ` Steven Rostedt
2021-10-13 18:16 ` [PATCH v5 07/15] lkdtm: Disable UNSET_SMEP with CFI Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 08/15] lkdtm: Use an opaque type for lkdtm_rodata_do_nothing Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 09/15] x86: Use an opaque type for functions not callable from C Sami Tolvanen
2021-10-14 11:21 ` Borislav Petkov
2021-10-14 16:07 ` Kees Cook
2021-10-14 17:31 ` Borislav Petkov
2021-10-14 18:24 ` Sami Tolvanen
2021-10-14 19:00 ` Nick Desaulniers
2021-10-14 18:47 ` Kees Cook
2021-10-14 18:52 ` Steven Rostedt
2021-10-14 19:06 ` Josh Poimboeuf
2021-10-13 18:16 ` [PATCH v5 10/15] x86/purgatory: Disable CFI Sami Tolvanen
2021-10-13 19:05 ` Kees Cook
2021-10-13 18:16 ` [PATCH v5 11/15] x86, relocs: Ignore __typeid__ relocations Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 12/15] x86, module: " Sami Tolvanen
2021-10-13 18:55 ` Kees Cook
2021-10-13 18:16 ` [PATCH v5 13/15] x86, cpu: Use LTO for cpu.c with CFI Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 14/15] x86, kprobes: Fix optprobe_template_func type mismatch Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 15/15] x86, build: Allow CONFIG_CFI_CLANG to be selected Sami Tolvanen
2021-10-13 18:56 ` Kees Cook
2021-10-13 19:07 ` [PATCH v5 00/15] x86: Add support for Clang CFI Kees Cook
2021-10-19 10:06 ` Alexander Lobakin
2021-10-19 15:40 ` Sami Tolvanen
2021-10-21 10:27 ` Alexander Lobakin
2021-10-26 20:16 ` Peter Zijlstra
2021-10-27 10:02 ` David Laight
2021-10-27 10:17 ` Peter Zijlstra
2021-10-27 12:05 ` Mark Rutland
2021-10-27 12:22 ` Ard Biesheuvel
2021-10-27 12:48 ` Peter Zijlstra
2021-10-27 13:04 ` Peter Zijlstra
2021-10-27 13:30 ` Ard Biesheuvel [this message]
2021-10-27 14:03 ` Peter Zijlstra
2021-10-27 14:18 ` Ard Biesheuvel
2021-10-27 14:36 ` Peter Zijlstra
2021-10-27 15:50 ` Sami Tolvanen
2021-10-27 15:55 ` Ard Biesheuvel
2021-10-29 20:03 ` Peter Zijlstra
2021-10-30 7:47 ` [PATCH] static_call,x86: Robustify trampoline patching Peter Zijlstra
2021-10-30 8:16 ` Peter Zijlstra
2021-11-02 17:35 ` Kees Cook
2021-11-02 18:15 ` Peter Zijlstra
2021-11-15 13:09 ` Rasmus Villemoes
2021-10-30 17:19 ` Ard Biesheuvel
2021-10-30 18:02 ` Peter Zijlstra
2021-10-30 18:55 ` Ard Biesheuvel
2021-10-31 16:24 ` Ard Biesheuvel
2021-10-31 16:39 ` Peter Zijlstra
2021-10-31 16:44 ` Ard Biesheuvel
2021-10-31 20:09 ` Peter Zijlstra
2021-10-31 20:21 ` Ard Biesheuvel
2021-10-31 20:44 ` Peter Zijlstra
2021-10-31 23:36 ` Ard Biesheuvel
2021-11-01 9:01 ` Peter Zijlstra
2021-11-01 9:36 ` David Laight
2021-11-01 14:14 ` Ard Biesheuvel
2021-11-02 12:57 ` Peter Zijlstra
2021-11-02 15:15 ` Peter Zijlstra
2021-11-02 17:44 ` Ard Biesheuvel
2021-11-02 18:14 ` Peter Zijlstra
2021-11-02 18:17 ` Peter Zijlstra
2021-11-02 18:18 ` Ard Biesheuvel
2021-11-02 21:48 ` Peter Zijlstra
2021-11-02 18:10 ` Kees Cook
2021-11-02 21:02 ` Andy Lutomirski
2021-11-02 23:13 ` Kees Cook
2021-11-03 0:20 ` Andy Lutomirski
2021-11-03 8:35 ` Peter Zijlstra
2021-11-03 10:01 ` David Laight
2021-11-03 19:32 ` Andy Lutomirski
2021-11-02 21:19 ` Peter Zijlstra
2021-11-11 12:15 ` [tip: locking/urgent] " tip-bot2 for Peter Zijlstra
2021-10-30 19:07 ` [PATCH v5 00/15] x86: Add support for Clang CFI Sami Tolvanen
2021-10-27 17:11 ` Kees Cook
2021-10-27 21:21 ` Peter Zijlstra
2021-10-27 22:27 ` Kees Cook
2021-10-28 11:09 ` Peter Zijlstra
2021-10-28 17:12 ` Kees Cook
2021-10-28 20:29 ` Peter Zijlstra
2021-11-02 17:26 ` Kees Cook
2021-11-01 4:13 ` Andy Lutomirski
2021-10-27 12:46 ` Peter Zijlstra
2021-10-27 12:55 ` David Laight
2021-10-27 13:17 ` Mark Rutland
2021-10-27 21:31 ` David Laight
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=CAMj1kXHKh7wv6JqusVnoiQDMm7ApFq2ujzbfWmM9AzLKFehhAA@mail.gmail.com \
--to=ardb@kernel.org \
--cc=jpoimboe@redhat.com \
--cc=keescook@chromium.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=mark.rutland@arm.com \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=samitolvanen@google.com \
--cc=sedat.dilek@gmail.com \
--cc=x86@kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).