All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Qing Zhao <qing.zhao@oracle.com>,
	Andrew Pinski <pinskia@gmail.com>,
	Richard Biener <rguenther@suse.de>,
	Joseph Myers <josmyers@redhat.com>, Jan Hubicka <hubicka@ucw.cz>,
	Richard Earnshaw <richard.earnshaw@arm.com>,
	Richard Sandiford <richard.sandiford@arm.com>,
	Marcus Shawcroft <marcus.shawcroft@arm.com>,
	Kyrylo Tkachov <kyrylo.tkachov@arm.com>,
	Kito Cheng <kito.cheng@gmail.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Andrew Waterman <andrew@sifive.com>,
	Jim Wilson <jim.wilson.gcc@gmail.com>,
	Dan Li <ashimida.1990@gmail.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	Ramon de C Valle <rcvalle@google.com>,
	Joao Moreira <joao@overdrivepizza.com>,
	Nathan Chancellor <nathan@kernel.org>,
	Bill Wendling <morbo@google.com>,
	gcc-patches@gcc.gnu.org, linux-hardening@vger.kernel.org
Subject: Re: [PATCH v2 2/7] kcfi: Add core Kernel Control Flow Integrity infrastructure
Date: Mon, 8 Sep 2025 14:55:38 -0700	[thread overview]
Message-ID: <202509081452.5AD50CAA@keescook> (raw)
In-Reply-To: <20250908153258.GE4067720@noisy.programming.kicks-ass.net>

On Mon, Sep 08, 2025 at 05:32:58PM +0200, Peter Zijlstra wrote:
> On Fri, Sep 05, 2025 at 09:19:29AM -0700, Kees Cook wrote:
> > On Fri, Sep 05, 2025 at 10:51:03AM +0200, Peter Zijlstra wrote:
> > > On Thu, Sep 04, 2025 at 05:24:10PM -0700, Kees Cook wrote:
> > > > +- The check-call instruction sequence must be treated a single unit: it
> > > > +  cannot be rearranged or split or optimized. The pattern is that
> > > > +  indirect calls, "call *$target", get converted into:
> > > > +
> > > > +    mov $target_expression, %target ; only present if the expression was
> > > > +                                    ; not already %target register
> > > > +    load -$offset(%target), %tmp    ; load the typeid hash at target
> > > > +    cmp $hash, %tmp                 ; compare expected typeid with loaded
> > > > +    je .Lcheck_passed               ; jump to the indirect call
> > > > +  .Lkcfi_trap$N:                    ; label of trap insn
> > > > +    trap                            ; trap on failure, but arranged so
> > > > +                                    ; "permissive mode" falls through
> > > > +  .Lkcfi_call$N:                    ; label of call insn
> > > > +    call *%target                   ; actual indirect call
> > > > +
> > > > +  This pattern of call immediately after trap provides for the
> > > > +  "permissive" checking mode automatically: the trap gets handled,
> > > > +  a warning emitted, and then execution continues after the trap to
> > > > +  the call.
> > > 
> > > I know it is far too late to do anything here. But I've recently dug
> > > through a bunch of optimization manual and the like and that Jcc is
> > > about as bad as it gets :/
> > > 
> > > The old optimization manual states that forward jumps are assumed
> > > not-taken; while backward jumps are assumed taken.
> > > 
> > > The new wisdom is that any Jcc must be assumed not-taken; that is, the
> > > fallthrough case has the best throughput.
> > 
> > I would expect the cmp to be the slowest part of this sequence, and I
> > figured the both the trap and the call to be speculation barriers? I'm
> > not sure, though. Is changing the sequence actually useful?
> 
> The load can miss, in which case it is definitely the most expensive
> thing around.
> 
> > > Here we have a forward branch which is assumed taken :-(
> > 
> > The constraints we have are:
> > 
> > - Linux x86 KCFI trap handler decodes the instructions from the trap
> >   backwards, but it uses exact offsets (-12 and -6).
> > - Control flow following the trap must make the call (for warn-only mode)
> > 
> > If we change this, we'd need to make the insn decoder smarter to likey
> > look at the insn AFTER the trap ("is it a direct jump?")
> > 
> > And then use this, which is ugly, but matches second constraint:
> > 
> > 	cmp $hash %tmp
> > 	jne .Ltrap
> > .Lcall:
> > 	call *%target
> > 	jmp .Ldone
> > .Ltrap:
> > 	ud2
> > 	jmp .Lcall
> > .Ldone:
> 
> Ah, you can do something like:
> 
> 	cmp $hash, %tmp
> 	jne +3
> 	nopl -42(%rax)
> 	call *%target
> 
> which is only 2 bytes longer. Notably, that nopl is 4 bytes and the 4th
> byte is 0xd6 (aka UDB). This is an effective UDcc instruction based
> around a forward non-taken branch.

Oh right, I forgot about the nop encodings.

> But yeah, I don't know if it is worth changing this. Its just that I've
> been staring at these things far too much of late :-)

To do this we'd need to change the Linux trap handler and Clang's
implementation, so yeah, I'm inclined to just leave it as-is until we
have a stronger reason to change it.

-- 
Kees Cook

  reply	other threads:[~2025-09-08 21:55 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-05  0:24 [PATCH v2 0/7] Introduce Kernel Control Flow Integrity ABI [PR107048] Kees Cook
2025-09-05  0:24 ` [PATCH v2 1/7] mangle: Introduce C typeinfo mangling API Kees Cook
2025-09-05  0:50   ` Andrew Pinski
2025-09-05  1:09     ` Kees Cook
2025-09-05  0:24 ` [PATCH v2 2/7] kcfi: Add core Kernel Control Flow Integrity infrastructure Kees Cook
2025-09-05  8:51   ` Peter Zijlstra
2025-09-05 16:19     ` Kees Cook
2025-09-08 15:32       ` Peter Zijlstra
2025-09-08 21:55         ` Kees Cook [this message]
2025-09-09 18:49   ` Qing Zhao
2025-09-11  3:05     ` Kees Cook
2025-09-11  7:29       ` Peter Zijlstra
2025-09-12  6:20         ` Kees Cook
2025-09-11 15:04       ` Qing Zhao
2025-09-12  7:32         ` Kees Cook
2025-09-12 14:01           ` Qing Zhao
2025-09-13  6:29             ` Kees Cook
2025-09-05  0:24 ` [PATCH v2 3/7] x86: Add x86_64 Kernel Control Flow Integrity implementation Kees Cook
2025-09-05  0:24 ` [PATCH v2 4/7] aarch64: Add AArch64 " Kees Cook
2025-09-05  0:24 ` [PATCH v2 5/7] arm: Add ARM 32-bit " Kees Cook
2025-09-11  7:49   ` Ard Biesheuvel
2025-09-12  9:03     ` Kees Cook
2025-09-12  9:08       ` Kees Cook
2025-09-12  9:43         ` Ard Biesheuvel
2025-09-12 19:01           ` Kees Cook
2025-09-05  0:24 ` [PATCH v2 6/7] riscv: Add RISC-V " Kees Cook
2025-09-16  3:40   ` Jeff Law
2025-09-16  6:04     ` Kees Cook
2025-10-01  0:56       ` Jeff Law
2025-09-05  0:24 ` [PATCH v2 7/7] kcfi: Add regression test suite Kees Cook
2025-09-05  7:06   ` Jakub Jelinek
2025-09-05 17:15     ` Kees Cook

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=202509081452.5AD50CAA@keescook \
    --to=kees@kernel.org \
    --cc=andrew@sifive.com \
    --cc=ashimida.1990@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=jim.wilson.gcc@gmail.com \
    --cc=joao@overdrivepizza.com \
    --cc=josmyers@redhat.com \
    --cc=kito.cheng@gmail.com \
    --cc=kyrylo.tkachov@arm.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=marcus.shawcroft@arm.com \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=palmer@dabbelt.com \
    --cc=peterz@infradead.org \
    --cc=pinskia@gmail.com \
    --cc=qing.zhao@oracle.com \
    --cc=rcvalle@google.com \
    --cc=rguenther@suse.de \
    --cc=richard.earnshaw@arm.com \
    --cc=richard.sandiford@arm.com \
    --cc=samitolvanen@google.com \
    /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 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.