From: Peter Zijlstra <peterz@infradead.org>
To: Kees Cook <keescook@chromium.org>
Cc: Ard Biesheuvel <ardb@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Sami Tolvanen <samitolvanen@google.com>, X86 ML <x86@kernel.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: Thu, 28 Oct 2021 22:29:05 +0200 [thread overview]
Message-ID: <20211028202905.GO174703@worktop.programming.kicks-ass.net> (raw)
In-Reply-To: <202110280958.22E5F74@keescook>
On Thu, Oct 28, 2021 at 10:12:32AM -0700, Kees Cook wrote:
> On Thu, Oct 28, 2021 at 01:09:39PM +0200, Peter Zijlstra wrote:
> > On Wed, Oct 27, 2021 at 03:27:59PM -0700, Kees Cook wrote:
> >
> > > Right -- though wouldn't just adding __ro_after_init do the same?
> > >
> > > DEFINE_STATIC_CALL(static_call_name, func_a) __ro_after_init;
> >
> > That breaks modules (and your jump_label patch doing the same is
> > similarly broken).
>
> Well that's no fun. :) I'd like to understand this better so I can fix
> it!
>
> >
> > When a module is loaded that uses the static_call(), it needs to
> > register it's .static_call_sites range with the static_call_key which
> > requires modifying it.
>
> Reading static_call_add_module() leaves me with even more questions. ;)
Yes, that function is highly magical..
> It looks like module static calls need to write to kernel text?
No, they need to modify the static_call_key though.
> I don't
> understand. Is this when a module is using an non-module key for a call
> site? And in that case, this happens:
>
> key |= s_key & STATIC_CALL_SITE_FLAGS;
>
> Where "key" is not in the module?
>
> And the flags can be:
>
> #define STATIC_CALL_SITE_TAIL 1UL /* tail call */
> #define STATIC_CALL_SITE_INIT 2UL /* init section */
>
> But aren't these per-site attributes? Why are they stored per-key?
They are per site, but stored in the key pointer.
So static_call has (and jump_label is nearly identical):
struct static_call_site {
s32 addr;
s32 key;
};
struct static_call_mod {
struct static_call_mod *next;
struct module *mod;
struct static_call_sutes *sites;
};
struct static_call_key {
void *func;
union {
unsigned long type;
struct static_call_mod *mods;
struct static_call_site *sites;
};
};
__SCT_##name() tramplines (no analog with jump_label)
.static_call_sites section
.static_call_tramp_key section (no analog with jump_label)
Where the key holds the current function pointer and a pointer to either
an array of static_call_site or a pointer to a static_call_mod.
Now, a key observation is that all these data structures are word
aligned, which means we have at least 2 lsb bits to play with. For
static_call_key::{mods,sites} the LSB indicates which, 0:mods, 1:sites.
Then the .static_call_sites section is an array of struct
static_call_site sorted by the static_call_key pointer.
The static_call_sites holds relative displacements, but represents:
struct static_call_key *key;
unsigned long call_address;
Now, since code (on x86) is variable length, there are no spare bits in
the code address, but since static_call_key is aligned, we have spare
bits. It is those bits we use to encode TAIL (Bit0) and INIT (Bit1).
If INIT, the address points to an __init section and we shouldn't try
and touch if after those have been freed or bad stuff happens.
If TAIL, it's a tail-call and we get to write a jump instruction instead
of a call instruction.
So, objtool builds .static_call_sites at built time, then at init (or
module load) time we sort the array by static_call_key pointer, such
that we get consequtive ranges per key. We iterate the array and every
time the key pointer changes, we -- already having the key pointer --
set key->sites to the first.
Now, kernel init of static_call happens *really* early and memory
allocation doesn't work yet, which is why we have that {mods,sites}
thing. Therefore, when the first module gets loaded, we need to allocate
a struct static_call_mod for the kernel (mod==NULL) and transfer the
sites pointer to it and change key to a mods pointer.
So one possible solution would be to have a late init (but before RO),
that, re-iterates the sites array and pre-allocates the kernel
static_call_mod structure. That way, static_call_key gets changed to a
mods pointer and wouldn't ever need changing after that, only the
static_call_mod (which isn't RO) gets changed when modules get
added/deleted.
The above is basically identical to jump_labels. However static_call()
have one more trick:
EXPORT_STATIC_CALL_TRAMP()
That exports the trampoline symbol, but not the static_call_key data
structure. The result is that modules can use the static_call(), but
cannot use static_call_update() because they cannot get at the key.
In this case objtool cannot correctly put the static_call_key address in
the static_call_site, what it does instead is store the trampoline
address (there's a 1:1 relation between key and tramplines). And then we
ues the .static_call_tramp_key section to find a mapping from trampoline
to key and rewrite the site to be 'right'. All this happens before
sorting it on key obv.
Hope that clarifies things, instead of making it worse :-)
next prev parent reply other threads:[~2021-10-28 20:29 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
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 [this message]
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=20211028202905.GO174703@worktop.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=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=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).